Export Config.Log and make possible to configure logger through context (controller-runtime way)

fgiloux · fgiloux · commit daea40bd143a · 2022-03-28T16:36:15.000+02:00
Signed-off-by: Frederic Giloux &lt;fgiloux@redhat.com&gt;
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -73,4 +73,4 @@ jobs:
         fetch-depth: 0
 
     - name: Run go-apidiff
-      uses: joelanford/go-apidiff@master
+      uses: joelanford/go-apidiff@main
diff --git a/prune/maxage.go b/prune/maxage.go
@@ -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 {
@@ -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])
 		}
diff --git a/prune/maxcount.go b/prune/maxcount.go
@@ -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])
 
diff --git a/prune/prune.go b/prune/prune.go
@@ -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
@@ -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
@@ -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 {
@@ -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
@@ -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")
 		}
@@ -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
 }
@@ -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 {
+	var log logr.Logger
+	if cfg.Log != (logr.Logger{}) {
+		log = cfg.Log
+	} else {
+		log = ctrllog.FromContext(ctx)
+	}
+	return log.WithValues(keysAndValues...)
+}
diff --git a/prune/remove.go b/prune/remove.go
@@ -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
 			}
diff --git a/prune/resource_test.go b/prune/resource_test.go
@@ -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",
@@ -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() {
@@ -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{
@@ -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")
 	}

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ func (config Config) removeResources(ctx context.Context, resources []ResourceIn`
`31`	`31`	`r := resources[i]`
`32`	`32`
`33`	`33`	`if config.PreDeleteHook != nil {`
`34`		`- err = config.PreDeleteHook(config, r)`
	`34`	`+ err = config.PreDeleteHook(ctx, config, r)`
`35`	`35`	`if err != nil {`
`36`	`36`	`return err`
`37`	`37`	`}`