Fixes #99: Remove log from Config and align with the library strategy.

operator-framework · Jan 25, 2022 · c065097 · c065097
1 parent 4cbd0e1
commit c065097
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 24 deletions.
diff --git a/prune/maxage.go b/prune/maxage.go
@@ -23,7 +23,7 @@ import (
 // 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)
+	log.V(1).Info("maxAge running", "setting", config.Strategy.MaxAgeSetting)
 
 	maxAgeDuration, e := time.ParseDuration(config.Strategy.MaxAgeSetting)
 	if e != nil {
@@ -33,9 +33,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
@@ -24,15 +24,15 @@ import (
 // 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))
+	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
@@ -19,12 +19,14 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
+var log = logf.Log.WithName("prune")
+
 // ResourceStatus describes the Kubernetes resource status we are evaluating
 type ResourceStatus string
 
@@ -63,20 +65,19 @@ type PreDelete func(cfg Config, something ResourceInfo) error
 // determines what will get pruned
 type Config struct {
 	Clientset      kubernetes.Interface      // kube client used by pruning
-	LabelSelector  string                    //selector resources to prune
-	DryRun         bool                      //true only performs a check, not removals
-	Resources      []schema.GroupVersionKind //pods, jobs are supported
-	Namespaces     []string                  //empty means all namespaces
-	Strategy       StrategyConfig            //strategy for pruning, either age or max
-	CustomStrategy StrategyFunc              //custom strategy
-	PreDeleteHook  PreDelete                 //called before resource is deleteds
-	log            logr.Logger
+	LabelSelector  string                    // selector resources to prune
+	DryRun         bool                      // true only performs a check, not removals
+	Resources      []schema.GroupVersionKind // pods, jobs are supported
+	Namespaces     []string                  // empty means all namespaces
+	Strategy       StrategyConfig            // strategy for pruning, either age or max
+	CustomStrategy StrategyFunc              // custom strategy
+	PreDeleteHook  PreDelete                 // called before resource is deleted
 }
 
 // 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.V(1).Info("Execute Prune")
 
 	err := config.validate()
 	if err != nil {
@@ -92,13 +93,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
@@ -123,7 +124,7 @@ func (config Config) Execute(ctx context.Context) error {
 		}
 	}
 
-	config.log.V(1).Info("Prune completed")
+	log.V(1).Info("Prune completed")
 
 	return nil
 }

diff --git a/prune/resource_test.go b/prune/resource_test.go
@@ -27,7 +27,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 	testclient "k8s.io/client-go/kubernetes/fake"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 var _ = Describe("Prune", func() {
@@ -41,7 +40,6 @@ var _ = Describe("Prune", func() {
 			client = testclient.NewSimpleClientset()
 			ctx = context.Background()
 			cfg = Config{
-				log:           logf.Log.WithName("prune"),
 				DryRun:        false,
 				Clientset:     client,
 				LabelSelector: "app=churro",
@@ -98,7 +96,6 @@ var _ = Describe("Prune", func() {
 		)
 		BeforeEach(func() {
 			cfg = Config{}
-			cfg.log = logf.Log.WithName("prune")
 			ctx = context.Background()
 		})
 		It("should return an error when LabelSelector is not set", func() {
@@ -130,7 +127,6 @@ var _ = Describe("Prune", func() {
 			ctx = context.Background()
 			jobcfg = Config{
 				DryRun:        false,
-				log:           logf.Log.WithName("prune"),
 				Clientset:     jobclient,
 				LabelSelector: "app=churro",
 				Resources: []schema.GroupVersionKind{
@@ -324,8 +320,8 @@ func myhook(cfg Config, x ResourceInfo) error {
 }
 
 // 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
+// example, the strategy doesn't really do anything other than counting
+// 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)