Skip to content

Closes #99: Make logging configurable. #100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 30, 2022
Merged

Closes #99: Make logging configurable. #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,4 @@ jobs:
fetch-depth: 0

- name: Run go-apidiff
uses: joelanford/go-apidiff@master
uses: joelanford/go-apidiff@main
9 changes: 5 additions & 4 deletions prune/maxage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ import (
// maxAge looks for and prunes resources, currently jobs and pods,
// that exceed a user specified age (e.g. 3d), resources to be removed
// are returned
func pruneByMaxAge(_ context.Context, config Config, resources []ResourceInfo) (resourcesToRemove []ResourceInfo, err error) {
config.log.V(1).Info("maxAge running", "setting", config.Strategy.MaxAgeSetting)
func pruneByMaxAge(ctx context.Context, config Config, resources []ResourceInfo) (resourcesToRemove []ResourceInfo, err error) {
log := Logger(ctx, config)
log.V(1).Info("maxAge running", "setting", config.Strategy.MaxAgeSetting)

maxAgeDuration, e := time.ParseDuration(config.Strategy.MaxAgeSetting)
if e != nil {
Expand All @@ -33,9 +34,9 @@ func pruneByMaxAge(_ context.Context, config Config, resources []ResourceInfo) (
maxAgeTime := time.Now().Add(-maxAgeDuration)

for i := 0; i < len(resources); i++ {
config.log.V(1).Info("age of pod ", "age", time.Since(resources[i].StartTime), "maxage", maxAgeTime)
log.V(1).Info("age of pod ", "age", time.Since(resources[i].StartTime), "maxage", maxAgeTime)
if resources[i].StartTime.Before(maxAgeTime) {
config.log.V(1).Info("pruning ", "kind", resources[i].GVK, "name", resources[i].Name)
log.V(1).Info("pruning ", "kind", resources[i].GVK, "name", resources[i].Name)

resourcesToRemove = append(resourcesToRemove, resources[i])
}
Expand Down
7 changes: 4 additions & 3 deletions prune/maxcount.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,17 @@ import (
// pruneByMaxCount looks for and prunes resources, currently jobs and pods,
// that exceed a user specified count (e.g. 3), the oldest resources
// are pruned, resources to remove are returned
func pruneByMaxCount(_ context.Context, config Config, resources []ResourceInfo) (resourcesToRemove []ResourceInfo, err error) {
config.log.V(1).Info("pruneByMaxCount running ", "max count", config.Strategy.MaxCountSetting, "resource count", len(resources))
func pruneByMaxCount(ctx context.Context, config Config, resources []ResourceInfo) (resourcesToRemove []ResourceInfo, err error) {
log := Logger(ctx, config)
log.V(1).Info("pruneByMaxCount running ", "max count", config.Strategy.MaxCountSetting, "resource count", len(resources))
if config.Strategy.MaxCountSetting < 0 {
return resourcesToRemove, fmt.Errorf("max count setting less than zero")
}

if len(resources) > config.Strategy.MaxCountSetting {
removeCount := len(resources) - config.Strategy.MaxCountSetting
for i := len(resources) - 1; i >= 0; i-- {
config.log.V(1).Info("pruning pod ", "pod name", resources[i].Name, "age", time.Since(resources[i].StartTime))
log.V(1).Info("pruning pod ", "pod name", resources[i].Name, "age", time.Since(resources[i].StartTime))

resourcesToRemove = append(resourcesToRemove, resources[i])

Expand Down
34 changes: 25 additions & 9 deletions prune/prune.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/kubernetes"

ctrllog "sigs.k8s.io/controller-runtime/pkg/log"
)

// ResourceStatus describes the Kubernetes resource status we are evaluating
Expand Down Expand Up @@ -54,10 +56,10 @@ type StrategyConfig struct {

// StrategyFunc function allows a means to specify
// custom prune strategies
type StrategyFunc func(cfg Config, resources []ResourceInfo) ([]ResourceInfo, error)
type StrategyFunc func(ctx context.Context, cfg Config, resources []ResourceInfo) ([]ResourceInfo, error)

// PreDelete function is called before a resource is pruned
type PreDelete func(cfg Config, something ResourceInfo) error
type PreDelete func(ctx context.Context, cfg Config, something ResourceInfo) error

// Config defines a pruning configuration and ultimately
// determines what will get pruned
Expand All @@ -70,13 +72,13 @@ type Config struct {
Strategy StrategyConfig //strategy for pruning, either age or max
CustomStrategy StrategyFunc //custom strategy
PreDeleteHook PreDelete //called before resource is deleteds
log logr.Logger
Log logr.Logger //optional: to overwrite the logger set at context level
}

// Execute causes the pruning work to be executed based on its configuration
func (config Config) Execute(ctx context.Context) error {

config.log.V(1).Info("Execute Prune")
log := Logger(ctx, config)
log.V(1).Info("Execute Prune")

err := config.validate()
if err != nil {
Expand All @@ -92,13 +94,13 @@ func (config Config) Execute(ctx context.Context) error {
if err != nil {
return err
}
config.log.V(1).Info("pods ", "count", len(resourceList))
log.V(1).Info("pods ", "count", len(resourceList))
} else if config.Resources[i].Kind == JobKind {
resourceList, err = config.getCompletedJobs(ctx)
if err != nil {
return err
}
config.log.V(1).Info("jobs ", "count", len(resourceList))
log.V(1).Info("jobs ", "count", len(resourceList))
}

var resourcesToRemove []ResourceInfo
Expand All @@ -109,7 +111,7 @@ func (config Config) Execute(ctx context.Context) error {
case MaxCountStrategy:
resourcesToRemove, err = pruneByMaxCount(ctx, config, resourceList)
case CustomStrategy:
resourcesToRemove, err = config.CustomStrategy(config, resourceList)
resourcesToRemove, err = config.CustomStrategy(ctx, config, resourceList)
default:
return fmt.Errorf("unknown strategy")
}
Expand All @@ -123,7 +125,7 @@ func (config Config) Execute(ctx context.Context) error {
}
}

config.log.V(1).Info("Prune completed")
log.V(1).Info("Prune completed")

return nil
}
Expand Down Expand Up @@ -181,3 +183,17 @@ func (config Config) validate() (err error) {
}
return nil
}

// Logger returns a logger from the context using logr method or Config.Log if none is found
// controller-runtime automatically provides a logger in context.Context during Reconcile calls.
// Note that there is no compile time check whether a logger can be retrieved by either way.
// keysAndValues allow to add fields to the logs, cf logr documentation.
func Logger(ctx context.Context, cfg Config, keysAndValues ...interface{}) logr.Logger {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this function need to be exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is exported in controller-runtime and the user can leverage it for custom code that gets injected through PreDeleteHook and CustomStrategy. I am passing the context to them for the purpose.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Makes sense. Thanks for the explanation!

var log logr.Logger
if cfg.Log != (logr.Logger{}) {
log = cfg.Log
} else {
log = ctrllog.FromContext(ctx)
}
return log.WithValues(keysAndValues...)
}
2 changes: 1 addition & 1 deletion prune/remove.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (config Config) removeResources(ctx context.Context, resources []ResourceIn
r := resources[i]

if config.PreDeleteHook != nil {
err = config.PreDeleteHook(config, r)
err = config.PreDeleteHook(ctx, config, r)
if err != nil {
return err
}
Expand Down
16 changes: 9 additions & 7 deletions prune/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var _ = Describe("Prune", func() {
client = testclient.NewSimpleClientset()
ctx = context.Background()
cfg = Config{
log: logf.Log.WithName("prune"),
Log: logf.Log.WithName("prune"),
DryRun: false,
Clientset: client,
LabelSelector: "app=churro",
Expand Down Expand Up @@ -98,7 +98,7 @@ var _ = Describe("Prune", func() {
)
BeforeEach(func() {
cfg = Config{}
cfg.log = logf.Log.WithName("prune")
cfg.Log = logf.Log.WithName("prune")
ctx = context.Background()
})
It("should return an error when LabelSelector is not set", func() {
Expand Down Expand Up @@ -130,7 +130,7 @@ var _ = Describe("Prune", func() {
ctx = context.Background()
jobcfg = Config{
DryRun: false,
log: logf.Log.WithName("prune"),
Log: logf.Log.WithName("prune"),
Clientset: jobclient,
LabelSelector: "app=churro",
Resources: []schema.GroupVersionKind{
Expand Down Expand Up @@ -318,17 +318,19 @@ func createTestPods(client kubernetes.Interface) (err error) {
return nil
}

func myhook(cfg Config, x ResourceInfo) error {
fmt.Println("myhook is called ")
func myhook(ctx context.Context, cfg Config, x ResourceInfo) error {
log := Logger(ctx, cfg)
log.V(1).Info("myhook is called")
return nil
}

// myStrategy shows how you can write your own strategy, in this
// example, the strategy doesn't really do another other than count
// the number of resources, returning a list of resources to delete in
// this case zero.
func myStrategy(cfg Config, resources []ResourceInfo) (resourcesToRemove []ResourceInfo, err error) {
fmt.Printf("myStrategy is called with resources %v config %v\n", resources, cfg)
func myStrategy(ctx context.Context, cfg Config, resources []ResourceInfo) (resourcesToRemove []ResourceInfo, err error) {
log := Logger(ctx, cfg)
log.V(1).Info("myStrategy is called", "resources", resources, "config", cfg)
if len(resources) != 3 {
return resourcesToRemove, fmt.Errorf("count of resources did not equal our expectation")
}
Expand Down