From 3e98659a51f3b52dc2e31a6f1e9ef07b28e21bb1 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Thu, 14 Apr 2022 16:37:22 -0400 Subject: [PATCH 01/12] update prune package improve the existing prune package fixes #101 Signed-off-by: Bryce Palmer --- .github/workflows/ci.yml | 4 +- go.mod | 6 +- go.sum | 9 +- prune/maxage.go | 46 --- prune/maxcount.go | 48 --- prune/prunables.go | 55 +++ prune/prune.go | 317 +++++++++++------- prune/prune_test.go | 699 +++++++++++++++++++++++++++++++++++++++ prune/registry.go | 67 ++++ prune/remove.go | 57 ---- prune/resource_test.go | 338 ------------------- prune/resources.go | 106 ------ 12 files changed, 1023 insertions(+), 729 deletions(-) delete mode 100644 prune/maxage.go delete mode 100644 prune/maxcount.go create mode 100644 prune/prunables.go create mode 100644 prune/prune_test.go create mode 100644 prune/registry.go delete mode 100644 prune/remove.go delete mode 100644 prune/resource_test.go delete mode 100644 prune/resources.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index acb5ad0..799f1bc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,7 @@ jobs: - name: Set up Go 1.x uses: actions/setup-go@v2 with: - go-version: ^1.17 + go-version: 1.17.x id: go - name: Check out code into the Go module directory @@ -50,7 +50,7 @@ jobs: uses: actions/checkout@v2 - name: Run golangci-lint - uses: golangci/golangci-lint-action@v2 + uses: golangci/golangci-lint-action@v3 with: version: v1.29 args: --timeout 5m diff --git a/go.mod b/go.mod index 837eb82..eb1e9fc 100644 --- a/go.mod +++ b/go.mod @@ -38,10 +38,10 @@ require ( github.com/prometheus/common v0.28.0 // indirect github.com/prometheus/procfs v0.6.0 // indirect github.com/spf13/pflag v1.0.5 // indirect - golang.org/x/net v0.0.0-20210825183410-e898025ed96a // indirect + golang.org/x/net v0.0.0-20220225172249-27dd8689420f // indirect golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f // indirect - golang.org/x/sys v0.0.0-20211029165221-6e7872819dc8 // indirect - golang.org/x/term v0.0.0-20210615171337-6886f2dfbf5b // indirect + golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e // indirect + golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect golang.org/x/text v0.3.7 // indirect golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect diff --git a/go.sum b/go.sum index 264e0b8..6cc0bae 100644 --- a/go.sum +++ b/go.sum @@ -665,8 +665,9 @@ golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96b golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT/9KSuxbyM7479/AVlXFRxuMCk= golang.org/x/net v0.0.0-20210525063256-abc453219eb5/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/net v0.0.0-20210825183410-e898025ed96a h1:bRuuGXV8wwSdGTB+CtJf+FjgO1APK1CoO39T4BN/XBw= golang.org/x/net v0.0.0-20210825183410-e898025ed96a/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= +golang.org/x/net v0.0.0-20220225172249-27dd8689420f h1:oA4XRj0qtSt8Yo1Zms0CUlsT3KG69V2UGQWPBxujDmc= +golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -764,13 +765,15 @@ golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210831042530-f4d43177bf5e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20211029165221-6e7872819dc8 h1:M69LAlWZCshgp0QSzyDcSsSIejIEeuaCVpmwcKwyLMk= golang.org/x/sys v0.0.0-20211029165221-6e7872819dc8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e h1:fLOSk5Q00efkSvAm+4xcoXD+RRmLmmulPn5I3Y9F2EM= +golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.0.0-20210615171337-6886f2dfbf5b h1:9zKuko04nR4gjZ4+DNjHqRlAJqbJETHwiNKDqTfOjfE= golang.org/x/term v0.0.0-20210615171337-6886f2dfbf5b/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY= +golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/prune/maxage.go b/prune/maxage.go deleted file mode 100644 index 4c47d84..0000000 --- a/prune/maxage.go +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2021 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package prune - -import ( - "context" - "time" -) - -// 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(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 { - return resourcesToRemove, e - } - - maxAgeTime := time.Now().Add(-maxAgeDuration) - - for i := 0; i < len(resources); i++ { - log.V(1).Info("age of pod ", "age", time.Since(resources[i].StartTime), "maxage", maxAgeTime) - if resources[i].StartTime.Before(maxAgeTime) { - log.V(1).Info("pruning ", "kind", resources[i].GVK, "name", resources[i].Name) - - resourcesToRemove = append(resourcesToRemove, resources[i]) - } - } - - return resourcesToRemove, nil -} diff --git a/prune/maxcount.go b/prune/maxcount.go deleted file mode 100644 index 303d219..0000000 --- a/prune/maxcount.go +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2021 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package prune - -import ( - "context" - "fmt" - "time" -) - -// 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(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-- { - log.V(1).Info("pruning pod ", "pod name", resources[i].Name, "age", time.Since(resources[i].StartTime)) - - resourcesToRemove = append(resourcesToRemove, resources[i]) - - removeCount-- - if removeCount == 0 { - break - } - } - } - - return resourcesToRemove, nil -} diff --git a/prune/prunables.go b/prune/prunables.go new file mode 100644 index 0000000..1fb4ce3 --- /dev/null +++ b/prune/prunables.go @@ -0,0 +1,55 @@ +// Copyright 2021 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prune + +import ( + "sigs.k8s.io/controller-runtime/pkg/client" + + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" +) + +// Some default pruneable functions + +// DefaultPodIsPrunable is a default IsPrunableFunc to be used specifically with Pod resources. +// This can be overridden by registering your own IsPrunableFunc via the RegisterIsPrunableFunc method +func DefaultPodIsPrunable(obj client.Object) error { + pod := obj.(*corev1.Pod) + if pod.Status.Phase != corev1.PodSucceeded { + return &Unprunable{ + Obj: &obj, + Reason: "Pod has not succeeded", + } + } + + return nil +} + +// DefaultJobIsPrunable is a default IsPrunableFunc to be used specifically with Job resources. +// This can be overridden by registering your own IsPrunableFunc via the RegisterIsPrunableFunc method +func DefaultJobIsPrunable(obj client.Object) error { + + job := obj.(*batchv1.Job) + + // If the job has completed we can remove it + if job.Status.CompletionTime == nil { + return &Unprunable{ + Obj: &obj, + Reason: "Job has not completed", + } + } + + return nil +} diff --git a/prune/prune.go b/prune/prune.go index 62133cd..6593ef5 100644 --- a/prune/prune.go +++ b/prune/prune.go @@ -16,184 +16,249 @@ package prune import ( "context" + "errors" "fmt" - "time" "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/kubernetes" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "sigs.k8s.io/controller-runtime/pkg/client" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" -) -// ResourceStatus describes the Kubernetes resource status we are evaluating -type ResourceStatus string - -// Strategy describes the pruning strategy we want to employ -type Strategy string - -const ( - // CustomStrategy maximum age of a resource that is desired, Duration - CustomStrategy Strategy = "Custom" - // MaxAgeStrategy maximum age of a resource that is desired, Duration - MaxAgeStrategy Strategy = "MaxAge" - // MaxCountStrategy maximum number of a resource that is desired, int - MaxCountStrategy Strategy = "MaxCount" - // JobKind equates to a Kube Job resource kind - JobKind string = "Job" - // PodKind equates to a Kube Pod resource kind - PodKind string = "Pod" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" ) -// StrategyConfig holds settings unique to each pruning mode -type StrategyConfig struct { - Mode Strategy - MaxAgeSetting string - MaxCountSetting int - CustomSettings map[string]interface{} +/* + ------------------------------------------- + New Auto Pruning API Implementation + ------------------------------------------- +*/ + +// Pruner is an object that runs a prune job. +type Pruner struct { + Registry + + // Client is the k8s client that will be used + Client client.Client + + // DryRun indicates whether or not we should actually perform pruning or just return the list of pruneable objects + // true = Just check, don't prune + // false (default) = Prune + DryRun bool + + // GVK is the type of objects to prune. + // It defaults to Pod + GVK schema.GroupVersionKind + + // Strategy is the function used to determine a list of resources that are pruneable + Strategy StrategyFunc + + // Labels is a map of the labels to use for label matching when looking for resources + Labels map[string]string + + // Namespace is the namespace to use when looking for resources + Namespace string + + // Logger is the logger to use when running pruning functionality + Logger logr.Logger } -// StrategyFunc function allows a means to specify -// custom prune strategies -type StrategyFunc func(ctx context.Context, cfg Config, resources []ResourceInfo) ([]ResourceInfo, error) - -// PreDelete function is called before a resource is pruned -type PreDelete func(ctx context.Context, cfg Config, something ResourceInfo) error - -// Config defines a pruning configuration and ultimately -// 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 //optional: to overwrite the logger set at context level +// Unprunable indicates that it is not allowed to prune a specific object. +type Unprunable struct { + Obj *client.Object + Reason string } -// Execute causes the pruning work to be executed based on its configuration -func (config Config) Execute(ctx context.Context) error { - log := Logger(ctx, config) - log.V(1).Info("Execute Prune") +// Error returns a string representation of an `Unprunable` error. +func (e *Unprunable) Error() string { + return fmt.Sprintf("unable to prune %s: %s", client.ObjectKeyFromObject(*e.Obj), e.Reason) +} - err := config.validate() - if err != nil { - return err - } - - for i := 0; i < len(config.Resources); i++ { - var resourceList []ResourceInfo - var err error - - if config.Resources[i].Kind == PodKind { - resourceList, err = config.getSucceededPods(ctx) - if err != nil { - return err - } - 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 - } - log.V(1).Info("jobs ", "count", len(resourceList)) - } +// StrategyFunc takes a list of resources and returns the subset to prune. +type StrategyFunc func(ctx context.Context, objs []client.Object) ([]client.ObjectKey, error) - var resourcesToRemove []ResourceInfo - - switch config.Strategy.Mode { - case MaxAgeStrategy: - resourcesToRemove, err = pruneByMaxAge(ctx, config, resourceList) - case MaxCountStrategy: - resourcesToRemove, err = pruneByMaxCount(ctx, config, resourceList) - case CustomStrategy: - resourcesToRemove, err = config.CustomStrategy(ctx, config, resourceList) - default: - return fmt.Errorf("unknown strategy") - } - if err != nil { - return err - } +// IsPrunableFunc is a function that checks the data of an object to see whether or not it is safe to prune it. +// It should return `nil` if it is safe to prune, `Unprunable` if it is unsafe, or another error. +// It should safely assert the object is the expected type, otherwise it might panic. +type IsPrunableFunc func(obj client.Object) error - err = config.removeResources(ctx, resourcesToRemove) - if err != nil { - return err - } +// PrunerOption configures the pruner. +type PrunerOption func(p *Pruner) + +// SetRegistry can be used to set the Registry field when configuring a Pruner +func SetRegistry(registry Registry) PrunerOption { + return func(p *Pruner) { + p.Registry = registry } +} - log.V(1).Info("Prune completed") +// DryRun can be used to set the DryRun field to true when configuring a Pruner +func DryRun() PrunerOption { + return func(p *Pruner) { + p.DryRun = true + } +} - return nil +// SetStrategy can be used to set the Strategy field when configuring a Pruner +func SetStrategy(strategy StrategyFunc) PrunerOption { + return func(p *Pruner) { + p.Strategy = strategy + } } -// containsString checks if a string is present in a slice -func containsString(s []string, str string) bool { - for _, v := range s { - if v == str { - return true - } +// Namespace can be used to set the Namespace field when configuring a Pruner +func Namespace(namespace string) PrunerOption { + return func(p *Pruner) { + p.Namespace = namespace } +} - return false +// SetLogger can be used to set the Logger field when configuring a Pruner +func SetLogger(logger logr.Logger) PrunerOption { + return func(p *Pruner) { + p.Logger = logger + } } -// containsName checks if a string is present in a ResourceInfo slice -func containsName(s []ResourceInfo, str string) bool { - for _, v := range s { - if v.Name == str { - return true - } +// Labels can be used to set the Labels field when configuring a Pruner +func Labels(labels map[string]string) PrunerOption { + return func(p *Pruner) { + p.Labels = labels } +} - return false +// GVK can be used to set the GVK field when configuring a Pruner +func GVK(gvk schema.GroupVersionKind) PrunerOption { + return func(p *Pruner) { + p.GVK = gvk + } } -func (config Config) validate() (err error) { - if config.CustomStrategy == nil && config.Strategy.Mode == CustomStrategy { - return fmt.Errorf("custom strategies require a strategy function to be specified") +// NewPruner returns a pruner that uses the given strategy to prune objects. +func NewPruner(prunerClient client.Client, opts ...PrunerOption) Pruner { + podGVK := corev1.SchemeGroupVersion.WithKind("Pod") + + jobGVK := batchv1.SchemeGroupVersion.WithKind("Job") + + pruner := Pruner{ + Registry: defaultRegistry, + Client: prunerClient, + DryRun: false, + Logger: Logger(context.Background(), Pruner{}), + GVK: podGVK, } - if len(config.Namespaces) == 0 { - return fmt.Errorf("namespaces are required") + // Populate the default IsPrunableFunc(s) + RegisterIsPrunableFunc(podGVK, DefaultPodIsPrunable) + + RegisterIsPrunableFunc(jobGVK, DefaultJobIsPrunable) + + for _, opt := range opts { + opt(&pruner) } - if containsString(config.Namespaces, "") { - return fmt.Errorf("empty namespace value not supported") + return pruner +} + +// Prune runs the pruner. +func (p Pruner) Prune(ctx context.Context) ([]client.ObjectKey, error) { + var objs []client.Object + p.Logger.Info("Starting the pruning process...") + listOpts := client.ListOptions{ + LabelSelector: labels.Set(p.Labels).AsSelector(), + Namespace: p.Namespace, } - _, err = labels.Parse(config.LabelSelector) - if err != nil { - return err + var unstructuredObjs unstructured.UnstructuredList + unstructuredObjs.SetGroupVersionKind(p.GVK) + if err := p.Client.List(ctx, &unstructuredObjs, &listOpts); err != nil { + p.Logger.Error(err, "failed to get a list of resources for pruning", "Labels", p.Labels, "Namespace", p.Namespace) + return nil, fmt.Errorf("failed to get list of objects -- ERROR -- %s", err) } - if config.Strategy.Mode == MaxAgeStrategy { - _, err = time.ParseDuration(config.Strategy.MaxAgeSetting) + for _, unsObj := range unstructuredObjs.Items { + obj, err := convert(p.Client, p.GVK, &unsObj) if err != nil { - return err + return nil, err } + + if err := p.IsPrunable(obj); isUnprunable(err) { + continue + } else if err != nil { + return nil, err + } + + objs = append(objs, obj) + } + + objsToPrune, err := p.Strategy(ctx, objs) + if err != nil { + p.Logger.Error(err, "failed to get a list of resources to prune from Strategy") + return nil, fmt.Errorf("failed when running Strategy -- ERROR -- %s", err) + } + + if p.DryRun { + // print out objects + return objsToPrune, nil } - if config.Strategy.Mode == MaxCountStrategy { - if config.Strategy.MaxCountSetting < 0 { - return fmt.Errorf("max count is required to be greater than or equal to 0") + + // Prune the resources + for _, obj := range objsToPrune { + // Prune + prunableObj := &unstructured.Unstructured{} + prunableObj.SetName(obj.Name) + prunableObj.SetNamespace(obj.Namespace) + prunableObj.SetGroupVersionKind(p.GVK) + + if err = p.Client.Delete(ctx, prunableObj); err != nil { + p.Logger.Error(err, "failed to prune resource", "Resource", obj) + return nil, fmt.Errorf("failed to prune object -- ERROR -- %s", err) } } - return nil + + return objsToPrune, nil } +/* + ------------------------------------------- + New Auto Pruning API Implementation + ------------------------------------------- +*/ + // 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 { +func Logger(ctx context.Context, pruner Pruner, keysAndValues ...interface{}) logr.Logger { var log logr.Logger - if cfg.Log != (logr.Logger{}) { - log = cfg.Log + if pruner.Logger != (logr.Logger{}) { + log = pruner.Logger } else { log = ctrllog.FromContext(ctx) } return log.WithValues(keysAndValues...) } + +func isUnprunable(target error) bool { + var unprunable *Unprunable + return errors.As(target, &unprunable) +} + +func convert(c client.Client, gvk schema.GroupVersionKind, obj client.Object) (client.Object, error) { + obj2, err := c.Scheme().New(gvk) + if err != nil { + return nil, err + } + objConverted := obj2.(client.Object) + if err := c.Scheme().Convert(obj, objConverted, nil); err != nil { + return nil, err + } + + objConverted.GetObjectKind().SetGroupVersionKind(gvk) + + return objConverted, nil +} diff --git a/prune/prune_test.go b/prune/prune_test.go new file mode 100644 index 0000000..68f4dea --- /dev/null +++ b/prune/prune_test.go @@ -0,0 +1,699 @@ +// Copyright 2021 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prune + +import ( + "context" + "errors" + "fmt" + + "github.com/go-logr/logr" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/scheme" + + batchv1 "k8s.io/api/batch/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + crFake "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const namespace = "default" +const app = "churro" + +var _ = Describe("Prune", func() { + var ( + fakeClient client.Client + fakeObj client.Object + prunerConfig PrunerOption + podGVK schema.GroupVersionKind + jobGVK schema.GroupVersionKind + ) + BeforeEach(func() { + testScheme, err := createSchemes() + + Expect(err).Should(BeNil()) + + fakeClient = crFake.NewClientBuilder().WithScheme(testScheme).Build() + + fakeObj = &corev1.Pod{} + + // Create our function to configure our pruner + prunerConfig = func(p *Pruner) { + + // Create the labels we want to select with + labels := make(map[string]string) + labels["app"] = app + + // Set our strategy + p.Strategy = myStrategy + + p.Labels = labels + + p.Namespace = namespace + p.Client = fakeClient + } + + podGVK = corev1.SchemeGroupVersion.WithKind("Pod") + + jobGVK = batchv1.SchemeGroupVersion.WithKind("Job") + + }) + + Describe("Unprunable", func() { + Describe("Error()", func() { + It("Should Return a String Representation of Unprunable", func() { + unpruneable := Unprunable{ + Obj: &fakeObj, + Reason: "TestReason", + } + + Expect(unpruneable.Error()).To(Equal(fmt.Sprintf("unable to prune %s: %s", client.ObjectKeyFromObject(fakeObj), unpruneable.Reason))) + }) + }) + }) + + Describe("Registry", func() { + Describe("NewRegistry()", func() { + It("Should Return a New Registry Object", func() { + registry := NewRegistry() + + Expect(registry).ShouldNot(BeNil()) + }) + }) + + Describe("RegisterIsPrunableFunc()", func() { + It("Should Add an Entry to Registry Prunables Map", func() { + registry := NewRegistry() + + Expect(registry).ShouldNot(BeNil()) + + registry.RegisterIsPrunableFunc(podGVK, myIsPrunable) + }) + }) + + Describe("IsPrunable()", func() { + It("Should Return 'nil' if object GVK is not found in Prunables Map", func() { + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "group", + Version: "v1", + Kind: "NotReal", + }) + + Expect(IsPrunable(obj)).Should(BeNil()) + }) + }) + + }) + Describe("Pruner", func() { + Describe("NewPruner()", func() { + It("Should Return a New Pruner Object", func() { + pruner := NewPruner(fakeClient) + + Expect(pruner).ShouldNot(BeNil()) + }) + + It("Should Return a New Pruner Object with Custom Configuration", func() { + registry := NewRegistry() + namespace := "namespace" + labels := map[string]string{"app": "churro"} + logger := &logr.Logger{} + pruner := NewPruner(fakeClient, + SetRegistry(*registry), + Namespace(namespace), + Labels(labels), + DryRun(), + SetStrategy(nil), + SetLogger(*logger), + GVK(jobGVK)) + + Expect(pruner).ShouldNot(BeNil()) + Expect(&pruner.Registry).Should(Equal(registry)) + Expect(pruner.Namespace).Should(Equal(namespace)) + Expect(pruner.Labels).Should(Equal(labels)) + Expect(&pruner.Logger).Should(Equal(logger)) + Expect(pruner.Strategy).Should(BeNil()) + Expect(pruner.DryRun).Should(BeTrue()) + Expect(pruner.GVK).Should(Equal(jobGVK)) + Expect(pruner.Client).Should(Equal(fakeClient)) + }) + + }) + + Describe("Prune()", func() { + Context("Does not return an Error", func() { + It("Should Prune Pods with Default IsPrunableFunc", func() { + // Create the test resources - in this case Pods + err := createTestPods(fakeClient) + + Expect(err).Should(BeNil()) + + // Make sure the pod resources are properly created + pods := &unstructured.UnstructuredList{} + pods.SetGroupVersionKind(podGVK) + err = fakeClient.List(context.Background(), pods) + + Expect(err).Should(BeNil()) + Expect(len(pods.Items)).Should(Equal(3)) + + pruner := NewPruner(fakeClient, prunerConfig) + + Expect(pruner).ShouldNot(BeNil()) + + prunedObjects, err := pruner.Prune(context.Background()) + + Expect(err).Should(BeNil()) + Expect(len(prunedObjects)).Should(Equal(2)) + // Get a list of the Pods to make sure we have pruned the ones we expected + err = fakeClient.List(context.Background(), pods) + + Expect(err).Should(BeNil()) + Expect(len(pods.Items)).Should(Equal(1)) + }) + + It("Should Prune Jobs with Default IsPrunableFunc", func() { + // Create the test resources - in this case Jobs + err := createTestJobs(fakeClient) + + Expect(err).Should(BeNil()) + + // Make sure the job resources are properly created + jobs := &unstructured.UnstructuredList{} + jobs.SetGroupVersionKind(jobGVK) + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(3)) + + pruner := NewPruner(fakeClient, prunerConfig, GVK(jobGVK)) + + Expect(pruner).ShouldNot(BeNil()) + + prunedObjects, err := pruner.Prune(context.Background()) + + Expect(err).Should(BeNil()) + Expect(len(prunedObjects)).Should(Equal(2)) + + // Get a list of the job to make sure we have pruned the ones we expected + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(1)) + }) + + It("Should Remove Resource When Using a Custom IsPrunableFunc", func() { + // Create the test resources - in this case Jobs + err := createTestJobs(fakeClient) + + Expect(err).Should(BeNil()) + + // Make sure the job resources are properly created + jobs := &unstructured.UnstructuredList{} + jobs.SetGroupVersionKind(jobGVK) + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(3)) + + pruner := NewPruner(fakeClient, prunerConfig, GVK(jobGVK)) + + Expect(pruner).ShouldNot(BeNil()) + + // Register our custom IsPrunableFunc + RegisterIsPrunableFunc(jobGVK, myIsPrunable) + + prunedObjects, err := pruner.Prune(context.Background()) + + Expect(err).Should(BeNil()) + Expect(len(prunedObjects)).Should(Equal(2)) + + // Get a list of the jobs to make sure we have pruned the ones we expected + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(1)) + }) + + It("Should Not Prune Resources when DryRun is set to true", func() { + // Create the test resources - in this case Pods + err := createTestPods(fakeClient) + + Expect(err).Should(BeNil()) + + // Make sure the pod resources are properly created + pods := &unstructured.UnstructuredList{} + pods.SetGroupVersionKind(podGVK) + err = fakeClient.List(context.Background(), pods) + + Expect(err).Should(BeNil()) + Expect(len(pods.Items)).Should(Equal(3)) + + pruner := NewPruner(fakeClient, prunerConfig) + + Expect(pruner).ShouldNot(BeNil()) + + pruner.DryRun = true + + prunedObjects, err := pruner.Prune(context.Background()) + + Expect(err).Should(BeNil()) + Expect(len(prunedObjects)).Should(Equal(2)) + + // Get a list of the Pods to make sure we haven't pruned any + err = fakeClient.List(context.Background(), pods) + + Expect(err).Should(BeNil()) + Expect(len(pods.Items)).Should(Equal(3)) + }) + + It("Should Skip Pruning a Resource If IsPrunable Returns an Error of Type Unprunable", func() { + // Create the test resources - in this case Jobs + err := createTestJobs(fakeClient) + + Expect(err).Should(BeNil()) + + // Make sure the job resources are properly created + jobs := &unstructured.UnstructuredList{} + jobs.SetGroupVersionKind(jobGVK) + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(3)) + + pruner := NewPruner(fakeClient, prunerConfig, GVK(jobGVK)) + + Expect(pruner).ShouldNot(BeNil()) + + // IsPrunableFunc that throws Unprunable error + errorPrunableFunc := func(obj client.Object) error { + return &Unprunable{ + Obj: &obj, + Reason: "TEST", + } + } + + // Register our custom IsPrunableFunc + RegisterIsPrunableFunc(jobGVK, errorPrunableFunc) + + prunedObjects, err := pruner.Prune(context.Background()) + + Expect(err).Should(BeNil()) + Expect(len(prunedObjects)).Should(Equal(0)) + + // Get a list of the jobs to make sure we have pruned the ones we expected + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(3)) + }) + + }) + Context("Returns an Error", func() { + It("Should Return an Error if IsPrunableFunc Returns an Error That is not of Type Unprunable", func() { + // Create the test resources - in this case Jobs + err := createTestJobs(fakeClient) + + Expect(err).Should(BeNil()) + + // Make sure the job resources are properly created + jobs := &unstructured.UnstructuredList{} + jobs.SetGroupVersionKind(jobGVK) + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(3)) + + pruner := NewPruner(fakeClient, prunerConfig, GVK(jobGVK)) + + Expect(pruner).ShouldNot(BeNil()) + + // IsPrunableFunc that throws non Unprunable error + errorPrunableFunc := func(obj client.Object) error { + return fmt.Errorf("TEST") + } + + // Register our custom IsPrunableFunc + RegisterIsPrunableFunc(jobGVK, errorPrunableFunc) + + prunedObjects, err := pruner.Prune(context.Background()) + + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).Should(Equal("TEST")) + Expect(len(prunedObjects)).Should(Equal(0)) + + // Get a list of the jobs to make sure we have pruned the ones we expected + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(3)) + }) + + It("Should Return An Error If Strategy Function Returns An Error", func() { + // Create the test resources - in this case Jobs + err := createTestJobs(fakeClient) + + Expect(err).Should(BeNil()) + + // Make sure the job resources are properly created + jobs := &unstructured.UnstructuredList{} + jobs.SetGroupVersionKind(jobGVK) + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(3)) + + pruner := NewPruner(fakeClient, prunerConfig) + + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + + // Register our custom IsPrunableFunc + RegisterIsPrunableFunc(jobGVK, myIsPrunable) + + // Override pruner strategy with one that will return an error + pruner.Strategy = func(ctx context.Context, objs []client.Object) ([]client.ObjectKey, error) { + return nil, fmt.Errorf("TESTERROR") + } + + prunedObjects, err := pruner.Prune(context.Background()) + + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).Should(Equal("failed when running Strategy -- ERROR -- TESTERROR")) + Expect(prunedObjects).Should(BeNil()) + + // Get a list of the jobs to make sure we have pruned the ones we expected + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(3)) + }) + + It("Should Return an Error if it can not Prune a Resource", func() { + // Create the test resources - in this case Jobs + err := createTestJobs(fakeClient) + + Expect(err).Should(BeNil()) + + // Make sure the job resources are properly created + jobs := &unstructured.UnstructuredList{} + jobs.SetGroupVersionKind(jobGVK) + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(3)) + + pruner := NewPruner(fakeClient, prunerConfig, GVK(jobGVK)) + + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + + // IsPrunableFunc that returns nil but also deletes the object + // so that it will throw an error when attempting to remove the object + prunableFunc := func(obj client.Object) error { + _ = fakeClient.Delete(context.TODO(), obj, &client.DeleteOptions{}) + return nil + } + + // Register our custom IsPrunableFunc + RegisterIsPrunableFunc(jobGVK, prunableFunc) + + prunedObjects, err := pruner.Prune(context.Background()) + + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).Should(ContainSubstring("failed to prune object -- ERROR -- jobs.batch \"churro1\" not found")) + Expect(len(prunedObjects)).Should(Equal(0)) + + // Get a list of the jobs to make sure we have pruned the ones we expected + err = fakeClient.List(context.Background(), jobs) + + Expect(err).Should(BeNil()) + Expect(len(jobs.Items)).Should(Equal(0)) + }) + + }) + }) + }) + + Context("DefaultPodIsPrunable", func() { + It("Should Return 'nil' When Criteria Is Met", func() { + // Create a Pod Object + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: app, + Namespace: namespace, + Labels: map[string]string{"app": app}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + }, + } + pod.SetGroupVersionKind(podGVK) + + // Run it through DefaultPodIsPrunable + err := DefaultPodIsPrunable(pod) + + // Check that return is 'nil' + Expect(err).Should(BeNil()) + }) + + It("Should Panic When client.Object is not of type 'Pod'", func() { + // Create an Unstrutcured with GVK where Kind is not 'Pod' + notPod := &unstructured.Unstructured{} + + defer expectPanic() + + // Run it through DefaultPodIsPrunable + _ = DefaultPodIsPrunable(notPod) + }) + + It("Should Return An Error When Kind Is 'Pod' But Phase Is Not 'Succeeded'", func() { + // Create a Pod Object + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: app, + Namespace: namespace, + Labels: map[string]string{"app": app}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + }, + } + pod.SetGroupVersionKind(podGVK) + + // Run it through DefaultPodIsPrunable + err := DefaultPodIsPrunable(pod) + + // Check that return is error + Expect(err).ShouldNot(BeNil()) + var expectErr *Unprunable + Expect(errors.As(err, &expectErr)).Should(BeTrue()) + Expect(expectErr.Reason).Should(Equal("Pod has not succeeded")) + Expect(expectErr.Obj).ShouldNot(BeNil()) + Expect(err.Error()).Should(Equal(fmt.Sprintf("unable to prune %s: Pod has not succeeded", client.ObjectKeyFromObject(pod)))) + }) + }) + + Context("DefaultJobIsPrunable", func() { + It("Should Return 'nil' When Criteria Is Met", func() { + // Create a Job Object + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: app, + Namespace: namespace, + Labels: map[string]string{"app": app}, + }, + Status: batchv1.JobStatus{ + CompletionTime: &metav1.Time{Time: metav1.Now().Time}, + }, + } + job.SetGroupVersionKind(jobGVK) + + // Run it through DefaultJobIsPrunable + err := DefaultJobIsPrunable(job) + + // Check that return is 'nil' + Expect(err).Should(BeNil()) + }) + + It("Should Return An Error When Kind Is Not 'Job'", func() { + // Create an Unstrutcured with GVK where Kind is not 'Job' + notJob := &unstructured.Unstructured{} + + defer expectPanic() + + // Run it through DefaultJobIsPrunable + _ = DefaultJobIsPrunable(notJob) + }) + + It("Should Return An Error When Kind Is 'Job' But 'CompletionTime' is 'nil'", func() { + // Create a Job Object + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: app, + Namespace: namespace, + Labels: map[string]string{"app": app}, + }, + Status: batchv1.JobStatus{ + CompletionTime: nil, + }, + } + job.SetGroupVersionKind(jobGVK) + + // Run it through DefaultJobIsPrunable + err := DefaultJobIsPrunable(job) + + // Check that return is error + Expect(err).ShouldNot(BeNil()) + var expectErr *Unprunable + Expect(errors.As(err, &expectErr)).Should(BeTrue()) + Expect(expectErr.Reason).Should(Equal("Job has not completed")) + Expect(expectErr.Obj).ShouldNot(BeNil()) + Expect(err.Error()).Should(ContainSubstring(fmt.Sprintf("unable to prune %s: Job has not completed", client.ObjectKeyFromObject(job)))) + }) + }) + +}) + +// create 3 pods and 3 jobs with different start times (now, 2 days old, 4 days old) +func createTestPods(client client.Client) (err error) { + // some defaults + ns := namespace + appLabel := app + + // Due to some weirdness in the way the fake client is set up we need to create our + // Kubernetes objects via the unstructured.Unstructured method + for i := 0; i < 3; i++ { + pod := &unstructured.Unstructured{} + pod.SetUnstructuredContent(map[string]interface{}{ + "apiVersion": "core/v1", + "kind": "Pod", + "metadata": map[string]interface{}{ + "name": fmt.Sprintf("churro%d", i), + "namespace": ns, + "labels": map[string]interface{}{ + "app": appLabel, + }, + }, + "status": map[string]interface{}{ + "phase": "Succeeded", + }, + }) + pod.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Pod")) + err = client.Create(context.Background(), pod) + + if err != nil { + return err + } + } + + return nil +} + +// create 3 pods and 3 jobs with different start times (now, 2 days old, 4 days old) +func createTestJobs(client client.Client) (err error) { + // some defaults + ns := namespace + appLabel := app + + // Due to some weirdness in the way the fake client is set up we need to create our + // Kubernetes objects via the unstructured.Unstructured method + for i := 0; i < 3; i++ { + job := &unstructured.Unstructured{} + job.SetUnstructuredContent(map[string]interface{}{ + "apiVersion": "batch/v1", + "kind": "Job", + "metadata": map[string]interface{}{ + "name": fmt.Sprintf("churro%d", i), + "namespace": ns, + "labels": map[string]interface{}{ + "app": appLabel, + }, + }, + "status": map[string]interface{}{ + "completionTime": metav1.Now(), + }, + }) + job.SetGroupVersionKind(batchv1.SchemeGroupVersion.WithKind("Job")) + err = client.Create(context.Background(), job) + + if err != nil { + return err + } + } + + return nil +} + +// createSchemes is a helper function to set up the schemes needed to run +// our tests utilizing controller-runtime's fake client +func createSchemes() (*runtime.Scheme, error) { + + corev1SchemeBuilder := &scheme.Builder{GroupVersion: corev1.SchemeGroupVersion} + corev1SchemeBuilder.Register(&corev1.Pod{}, &corev1.PodList{}) + + batchv1SchemeBuilder := &scheme.Builder{GroupVersion: batchv1.SchemeGroupVersion} + batchv1SchemeBuilder.Register(&batchv1.Job{}, &batchv1.JobList{}) + + outScheme := runtime.NewScheme() + + err := corev1SchemeBuilder.AddToScheme(outScheme) + if err != nil { + return nil, err + } + + err = batchv1SchemeBuilder.AddToScheme(outScheme) + if err != nil { + return nil, err + } + + return outScheme, nil +} + +// myStrategy shows how you can write your own strategy +// In this example it simply removes a resource if it has +// the name 'churro1' or 'churro2' +func myStrategy(ctx context.Context, objs []client.Object) ([]client.ObjectKey, error) { + var objsToRemove []client.ObjectKey + + for _, obj := range objs { + // If the object has name churro1 or churro2 get rid of it + if obj.GetName() == "churro1" || obj.GetName() == "churro2" { + objsToRemove = append(objsToRemove, client.ObjectKeyFromObject(obj)) + } + } + + return objsToRemove, nil +} + +// expectPanic is a helper function for testing functions that are expected to panic +// when used it should be used with a defer statement before the function +// that is expected to panic is called +func expectPanic() { + r := recover() + + Expect(r).ShouldNot(BeNil()) +} + +// myIsPrunable shows how you can write your own IsPrunableFunc +// In this example it simply removes all resources +func myIsPrunable(obj client.Object) error { + return nil +} diff --git a/prune/registry.go b/prune/registry.go new file mode 100644 index 0000000..18c9a88 --- /dev/null +++ b/prune/registry.go @@ -0,0 +1,67 @@ +// Copyright 2021 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prune + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// Registry is used to register a mapping of GroupVersionKind to an IsPrunableFunc +type Registry struct { + // prunables is a map of GVK to an IsPrunableFunc + prunables map[schema.GroupVersionKind]IsPrunableFunc +} + +// NewRegistry creates a new Registry +func NewRegistry() *Registry { + return new(Registry) +} + +// DefaultRegistry is a default Registry configuration +func DefaultRegistry() *Registry { + return &defaultRegistry +} + +var defaultRegistry Registry + +// RegisterIsPrunableFunc registers a function to check whether it is safe to prune a resource of a certain type. +func (r *Registry) RegisterIsPrunableFunc(gvk schema.GroupVersionKind, isPrunable IsPrunableFunc) { + if r.prunables == nil { + r.prunables = make(map[schema.GroupVersionKind]IsPrunableFunc) + } + + r.prunables[gvk] = isPrunable +} + +// IsPrunable checks if an object is prunable +func (r *Registry) IsPrunable(obj client.Object) error { + isPrunable, ok := r.prunables[obj.GetObjectKind().GroupVersionKind()] + if !ok { + return nil + } + + return isPrunable(obj) +} + +// RegisterIsPrunableFunc registers a function to check whether it is safe to prune a resource of a certain type. +func RegisterIsPrunableFunc(gvk schema.GroupVersionKind, isPrunable IsPrunableFunc) { + DefaultRegistry().RegisterIsPrunableFunc(gvk, isPrunable) +} + +// IsPrunable checks if an object is prunable +func IsPrunable(obj client.Object) error { + return DefaultRegistry().IsPrunable(obj) +} diff --git a/prune/remove.go b/prune/remove.go deleted file mode 100644 index c0826b6..0000000 --- a/prune/remove.go +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2021 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package prune - -import ( - "context" - "fmt" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func (config Config) removeResources(ctx context.Context, resources []ResourceInfo) (err error) { - - if config.DryRun { - return nil - } - - for i := 0; i < len(resources); i++ { - r := resources[i] - - if config.PreDeleteHook != nil { - err = config.PreDeleteHook(ctx, config, r) - if err != nil { - return err - } - } - - switch resources[i].GVK.Kind { - case PodKind: - err := config.Clientset.CoreV1().Pods(r.Namespace).Delete(ctx, r.Name, metav1.DeleteOptions{}) - if err != nil { - return err - } - case JobKind: - err := config.Clientset.BatchV1().Jobs(r.Namespace).Delete(ctx, r.Name, metav1.DeleteOptions{}) - if err != nil { - return err - } - default: - return fmt.Errorf("unsupported resource kind") - } - } - - return nil -} diff --git a/prune/resource_test.go b/prune/resource_test.go deleted file mode 100644 index a5e9f5a..0000000 --- a/prune/resource_test.go +++ /dev/null @@ -1,338 +0,0 @@ -// Copyright 2021 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package prune - -import ( - "context" - "fmt" - "time" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - batchv1 "k8s.io/api/batch/v1" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "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() { - Describe("test pods", func() { - var ( - client kubernetes.Interface - cfg Config - ctx context.Context - ) - BeforeEach(func() { - client = testclient.NewSimpleClientset() - ctx = context.Background() - cfg = Config{ - Log: logf.Log.WithName("prune"), - DryRun: false, - Clientset: client, - LabelSelector: "app=churro", - Resources: []schema.GroupVersionKind{ - {Group: "", Version: "", Kind: PodKind}, - }, - Namespaces: []string{"default"}, - Strategy: StrategyConfig{ - Mode: MaxCountStrategy, - MaxCountSetting: 1, - }, - PreDeleteHook: myhook, - } - - _ = createTestPods(client) - }) - It("test pod maxCount strategy", func() { - err := cfg.Execute(ctx) - Expect(err).Should(BeNil()) - var pods []ResourceInfo - pods, err = cfg.getSucceededPods(ctx) - Expect(err).Should(BeNil()) - Expect(len(pods)).To(Equal(1)) - Expect(containsName(pods, "churro1")).To(Equal(true)) - }) - It("test pod maxAge strategy", func() { - cfg.Strategy.Mode = MaxAgeStrategy - cfg.Strategy.MaxAgeSetting = "3h" - err := cfg.Execute(ctx) - Expect(err).Should(BeNil()) - var pods []ResourceInfo - pods, err = cfg.getSucceededPods(ctx) - Expect(err).Should(BeNil()) - Expect(containsName(pods, "churro1")).To(Equal(true)) - Expect(containsName(pods, "churro2")).To(Equal(true)) - }) - It("test pod custom strategy", func() { - cfg.Strategy.Mode = CustomStrategy - cfg.Strategy.CustomSettings = make(map[string]interface{}) - cfg.CustomStrategy = myStrategy - err := cfg.Execute(ctx) - Expect(err).Should(BeNil()) - var pods []ResourceInfo - pods, err = cfg.getSucceededPods(ctx) - Expect(err).Should(BeNil()) - Expect(len(pods)).To(Equal(3)) - }) - }) - - Describe("config validation", func() { - var ( - ctx context.Context - cfg Config - ) - BeforeEach(func() { - cfg = Config{} - cfg.Log = logf.Log.WithName("prune") - ctx = context.Background() - }) - It("should return an error when LabelSelector is not set", func() { - err := cfg.Execute(ctx) - Expect(err).ShouldNot(BeNil()) - }) - It("should return an error is Namespaces is empty", func() { - cfg.LabelSelector = "app=churro" - err := cfg.Execute(ctx) - Expect(err).ShouldNot(BeNil()) - }) - It("should return an error when labels dont parse", func() { - cfg.Namespaces = []string{"one"} - cfg.LabelSelector = "-" - err := cfg.Execute(ctx) - Expect(err).ShouldNot(BeNil()) - }) - }) - - Describe("test jobs", func() { - var ( - jobclient kubernetes.Interface - jobcfg Config - ctx context.Context - ) - BeforeEach(func() { - jobclient = testclient.NewSimpleClientset() - - ctx = context.Background() - jobcfg = Config{ - DryRun: false, - Log: logf.Log.WithName("prune"), - Clientset: jobclient, - LabelSelector: "app=churro", - Resources: []schema.GroupVersionKind{ - {Group: "", Version: "", Kind: JobKind}, - }, - Namespaces: []string{"default"}, - Strategy: StrategyConfig{ - Mode: MaxCountStrategy, - MaxCountSetting: 1, - }, - PreDeleteHook: myhook, - } - - _ = createTestJobs(jobclient) - }) - It("test job maxAge strategy", func() { - jobcfg.Strategy.Mode = MaxAgeStrategy - jobcfg.Strategy.MaxAgeSetting = "3h" - err := jobcfg.Execute(ctx) - Expect(err).Should(BeNil()) - var jobs []ResourceInfo - jobs, err = jobcfg.getCompletedJobs(ctx) - Expect(err).Should(BeNil()) - Expect(containsName(jobs, "churro1")).To(Equal(true)) - Expect(containsName(jobs, "churro2")).To(Equal(true)) - }) - It("test job maxCount strategy", func() { - err := jobcfg.Execute(ctx) - Expect(err).Should(BeNil()) - var jobs []ResourceInfo - jobs, err = jobcfg.getCompletedJobs(ctx) - Expect(err).Should(BeNil()) - Expect(len(jobs)).To(Equal(1)) - Expect(containsName(jobs, "churro1")).To(Equal(true)) - }) - It("test job custom strategy", func() { - jobcfg.Strategy.Mode = CustomStrategy - jobcfg.Strategy.CustomSettings = make(map[string]interface{}) - jobcfg.CustomStrategy = myStrategy - err := jobcfg.Execute(ctx) - Expect(err).Should(BeNil()) - var jobs []ResourceInfo - jobs, err = jobcfg.getCompletedJobs(ctx) - Expect(err).Should(BeNil()) - Expect(len(jobs)).To(Equal(3)) - }) - }) -}) - -// create 3 jobs with different start times (now, 2 days old, 4 days old) -func createTestJobs(client kubernetes.Interface) (err error) { - // some defaults - ns := "default" - labels := make(map[string]string) - labels["app"] = "churro" - - // delete any existing jobs - _ = client.BatchV1().Jobs(ns).Delete(context.TODO(), "churro1", metav1.DeleteOptions{}) - _ = client.BatchV1().Jobs(ns).Delete(context.TODO(), "churro2", metav1.DeleteOptions{}) - _ = client.BatchV1().Jobs(ns).Delete(context.TODO(), "churro3", metav1.DeleteOptions{}) - - // create 3 jobs with different CompletionTime - now := time.Now() //initial start time - startTime := metav1.NewTime(now) - j1 := &batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - Name: "churro1", - Namespace: ns, - Labels: labels, - }, - Status: batchv1.JobStatus{ - CompletionTime: &startTime, - }, - } - _, err = client.BatchV1().Jobs(ns).Create(context.TODO(), j1, metav1.CreateOptions{}) - if err != nil { - return err - } - - twoHoursPriorToNow := now.Add(time.Hour * time.Duration(-2)) - // create start time 2 hours before now - startTime = metav1.NewTime(twoHoursPriorToNow) - j2 := &batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - Name: "churro2", - Namespace: ns, - Labels: labels, - }, - Status: batchv1.JobStatus{ - CompletionTime: &startTime, - }, - } - _, err = client.BatchV1().Jobs(ns).Create(context.TODO(), j2, metav1.CreateOptions{}) - if err != nil { - return err - } - // create start time 4 hours before now - fourHoursPriorToNow := now.Add(time.Hour * time.Duration(-4)) - startTime = metav1.NewTime(fourHoursPriorToNow) - j3 := &batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - Name: "churro3", - Namespace: ns, - Labels: labels, - }, - Status: batchv1.JobStatus{ - CompletionTime: &startTime, - }, - } - _, err = client.BatchV1().Jobs(ns).Create(context.TODO(), j3, metav1.CreateOptions{}) - if err != nil { - return err - } - return nil -} - -// create 3 pods and 3 jobs with different start times (now, 2 days old, 4 days old) -func createTestPods(client kubernetes.Interface) (err error) { - // some defaults - ns := "default" - labels := make(map[string]string) - labels["app"] = "churro" - - // delete any existing pods - _ = client.CoreV1().Pods(ns).Delete(context.TODO(), "churro1", metav1.DeleteOptions{}) - _ = client.CoreV1().Pods(ns).Delete(context.TODO(), "churro2", metav1.DeleteOptions{}) - _ = client.CoreV1().Pods(ns).Delete(context.TODO(), "churro3", metav1.DeleteOptions{}) - - // create 3 pods with different StartTimes - now := time.Now() //initial start time - startTime := metav1.NewTime(now) - p1 := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "churro1", - Namespace: ns, - Labels: labels, - }, - Status: v1.PodStatus{ - Phase: v1.PodSucceeded, - StartTime: &startTime, - }, - } - _, err = client.CoreV1().Pods(ns).Create(context.TODO(), p1, metav1.CreateOptions{}) - if err != nil { - return err - } - - twoHoursPriorToNow := now.Add(time.Hour * time.Duration(-2)) - // create start time 2 hours before now - startTime = metav1.NewTime(twoHoursPriorToNow) - p2 := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "churro2", - Namespace: ns, - Labels: labels, - }, - Status: v1.PodStatus{ - Phase: v1.PodSucceeded, - StartTime: &startTime, - }, - } - _, err = client.CoreV1().Pods(ns).Create(context.TODO(), p2, metav1.CreateOptions{}) - if err != nil { - return err - } - // create start time 4 hours before now - fourHoursPriorToNow := now.Add(time.Hour * time.Duration(-4)) - startTime = metav1.NewTime(fourHoursPriorToNow) - p3 := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "churro3", - Namespace: ns, - Labels: labels, - }, - Status: v1.PodStatus{ - Phase: v1.PodSucceeded, - StartTime: &startTime, - }, - } - _, err = client.CoreV1().Pods(ns).Create(context.TODO(), p3, metav1.CreateOptions{}) - if err != nil { - return err - } - - return nil -} - -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(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") - } - return resourcesToRemove, nil -} diff --git a/prune/resources.go b/prune/resources.go deleted file mode 100644 index f3e7cbf..0000000 --- a/prune/resources.go +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright 2021 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package prune - -import ( - "context" - "sort" - "time" - - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" -) - -// ResourceInfo describes the Kube resources that we are about to consider -// when pruning resources -type ResourceInfo struct { - Name string - GVK schema.GroupVersionKind - Namespace string - StartTime time.Time -} - -func (config Config) getSucceededPods(ctx context.Context) (resources []ResourceInfo, err error) { - - listOptions := metav1.ListOptions{LabelSelector: config.LabelSelector} - for n := 0; n < len(config.Namespaces); n++ { - pods, err := config.Clientset.CoreV1().Pods(config.Namespaces[n]).List(ctx, listOptions) - if err != nil { - return resources, err - } - - for i := 0; i < len(pods.Items); i++ { - p := pods.Items[i] - switch p.Status.Phase { - case v1.PodRunning: - case v1.PodPending: - case v1.PodFailed: - case v1.PodUnknown: - case v1.PodSucceeded: - // currently we only care to prune succeeded pods - resources = append(resources, ResourceInfo{ - Name: p.Name, - GVK: schema.GroupVersionKind{ - Kind: PodKind, - }, - Namespace: config.Namespaces[n], - StartTime: p.Status.StartTime.Time, - }) - default: - } - } - } - - // sort by StartTime, earliest first order - sort.Slice(resources, func(i, j int) bool { - return resources[i].StartTime.After(resources[j].StartTime) - }) - - return resources, nil -} - -func (config Config) getCompletedJobs(ctx context.Context) (resources []ResourceInfo, err error) { - - listOptions := metav1.ListOptions{LabelSelector: config.LabelSelector} - - for n := 0; n < len(config.Namespaces); n++ { - jobs, err := config.Clientset.BatchV1().Jobs(config.Namespaces[n]).List(ctx, listOptions) - if err != nil { - return resources, err - } - for i := 0; i < len(jobs.Items); i++ { - j := jobs.Items[i] - if j.Status.CompletionTime != nil { - // currently we only care to prune succeeded pods - resources = append(resources, ResourceInfo{ - Name: j.Name, - GVK: schema.GroupVersionKind{ - Kind: JobKind, - }, - Namespace: config.Namespaces[n], - StartTime: j.Status.CompletionTime.Time, - }) - } - } - } - - // sort by StartTime, earliest first order - sort.Slice(resources, func(i, j int) bool { - return resources[i].StartTime.After(resources[j].StartTime) - }) - - return resources, nil -} From 3f60543cf9ce9403d249d252ce3cf1bd9a42d34d Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Wed, 20 Apr 2022 16:11:17 -0400 Subject: [PATCH 02/12] address review comments (partial) Signed-off-by: Bryce Palmer --- prune/prunables.go | 7 +--- prune/prune.go | 94 +++++++++++++-------------------------------- prune/prune_test.go | 46 ++++++++++------------ prune/registry.go | 5 --- 4 files changed, 50 insertions(+), 102 deletions(-) diff --git a/prune/prunables.go b/prune/prunables.go index 1fb4ce3..7137448 100644 --- a/prune/prunables.go +++ b/prune/prunables.go @@ -21,9 +21,8 @@ import ( corev1 "k8s.io/api/core/v1" ) -// Some default pruneable functions - // DefaultPodIsPrunable is a default IsPrunableFunc to be used specifically with Pod resources. +// It marks a Pod resource as prunable if it's Status.Phase is "Succeeded" // This can be overridden by registering your own IsPrunableFunc via the RegisterIsPrunableFunc method func DefaultPodIsPrunable(obj client.Object) error { pod := obj.(*corev1.Pod) @@ -38,12 +37,10 @@ func DefaultPodIsPrunable(obj client.Object) error { } // DefaultJobIsPrunable is a default IsPrunableFunc to be used specifically with Job resources. +// It marks a Job resource as prunable if it's Status.CompletionTime value is not `nil`, indicating that the Job has completed // This can be overridden by registering your own IsPrunableFunc via the RegisterIsPrunableFunc method func DefaultJobIsPrunable(obj client.Object) error { - job := obj.(*batchv1.Job) - - // If the job has completed we can remove it if job.Status.CompletionTime == nil { return &Unprunable{ Obj: &obj, diff --git a/prune/prune.go b/prune/prune.go index 6593ef5..f9c9573 100644 --- a/prune/prune.go +++ b/prune/prune.go @@ -32,24 +32,20 @@ import ( corev1 "k8s.io/api/core/v1" ) -/* - ------------------------------------------- - New Auto Pruning API Implementation - ------------------------------------------- -*/ +func init() { + RegisterIsPrunableFunc(corev1.SchemeGroupVersion.WithKind("Pod"), DefaultPodIsPrunable) + + RegisterIsPrunableFunc(batchv1.SchemeGroupVersion.WithKind("Job"), DefaultJobIsPrunable) +} // Pruner is an object that runs a prune job. type Pruner struct { Registry - // Client is the k8s client that will be used + // Client is the controller-runtime client that will be used + // To perform a dry run, use the controller-runtime DryRunClient Client client.Client - // DryRun indicates whether or not we should actually perform pruning or just return the list of pruneable objects - // true = Just check, don't prune - // false (default) = Prune - DryRun bool - // GVK is the type of objects to prune. // It defaults to Pod GVK schema.GroupVersionKind @@ -79,7 +75,7 @@ func (e *Unprunable) Error() string { } // StrategyFunc takes a list of resources and returns the subset to prune. -type StrategyFunc func(ctx context.Context, objs []client.Object) ([]client.ObjectKey, error) +type StrategyFunc func(ctx context.Context, objs []client.Object) ([]client.Object, error) // IsPrunableFunc is a function that checks the data of an object to see whether or not it is safe to prune it. // It should return `nil` if it is safe to prune, `Unprunable` if it is unsafe, or another error. @@ -89,50 +85,43 @@ type IsPrunableFunc func(obj client.Object) error // PrunerOption configures the pruner. type PrunerOption func(p *Pruner) -// SetRegistry can be used to set the Registry field when configuring a Pruner -func SetRegistry(registry Registry) PrunerOption { +// WithRegistry can be used to set the Registry field when configuring a Pruner +func WithRegistry(registry Registry) PrunerOption { return func(p *Pruner) { p.Registry = registry } } -// DryRun can be used to set the DryRun field to true when configuring a Pruner -func DryRun() PrunerOption { - return func(p *Pruner) { - p.DryRun = true - } -} - -// SetStrategy can be used to set the Strategy field when configuring a Pruner -func SetStrategy(strategy StrategyFunc) PrunerOption { +// WithStrategy can be used to set the Strategy field when configuring a Pruner +func WithStrategy(strategy StrategyFunc) PrunerOption { return func(p *Pruner) { p.Strategy = strategy } } -// Namespace can be used to set the Namespace field when configuring a Pruner -func Namespace(namespace string) PrunerOption { +// WithNamespace can be used to set the Namespace field when configuring a Pruner +func WithNamespace(namespace string) PrunerOption { return func(p *Pruner) { p.Namespace = namespace } } -// SetLogger can be used to set the Logger field when configuring a Pruner -func SetLogger(logger logr.Logger) PrunerOption { +// WithLogger can be used to set the Logger field when configuring a Pruner +func WithLogger(logger logr.Logger) PrunerOption { return func(p *Pruner) { p.Logger = logger } } -// Labels can be used to set the Labels field when configuring a Pruner -func Labels(labels map[string]string) PrunerOption { +// WithLabels can be used to set the Labels field when configuring a Pruner +func WithLabels(labels map[string]string) PrunerOption { return func(p *Pruner) { p.Labels = labels } } -// GVK can be used to set the GVK field when configuring a Pruner -func GVK(gvk schema.GroupVersionKind) PrunerOption { +// WithGVK can be used to set the GVK field when configuring a Pruner +func WithGVK(gvk schema.GroupVersionKind) PrunerOption { return func(p *Pruner) { p.GVK = gvk } @@ -140,23 +129,13 @@ func GVK(gvk schema.GroupVersionKind) PrunerOption { // NewPruner returns a pruner that uses the given strategy to prune objects. func NewPruner(prunerClient client.Client, opts ...PrunerOption) Pruner { - podGVK := corev1.SchemeGroupVersion.WithKind("Pod") - - jobGVK := batchv1.SchemeGroupVersion.WithKind("Job") - pruner := Pruner{ Registry: defaultRegistry, Client: prunerClient, - DryRun: false, Logger: Logger(context.Background(), Pruner{}), - GVK: podGVK, + GVK: corev1.SchemeGroupVersion.WithKind("Pod"), } - // Populate the default IsPrunableFunc(s) - RegisterIsPrunableFunc(podGVK, DefaultPodIsPrunable) - - RegisterIsPrunableFunc(jobGVK, DefaultJobIsPrunable) - for _, opt := range opts { opt(&pruner) } @@ -165,7 +144,7 @@ func NewPruner(prunerClient client.Client, opts ...PrunerOption) Pruner { } // Prune runs the pruner. -func (p Pruner) Prune(ctx context.Context) ([]client.ObjectKey, error) { +func (p Pruner) Prune(ctx context.Context) ([]client.Object, error) { var objs []client.Object p.Logger.Info("Starting the pruning process...") listOpts := client.ListOptions{ @@ -176,8 +155,7 @@ func (p Pruner) Prune(ctx context.Context) ([]client.ObjectKey, error) { var unstructuredObjs unstructured.UnstructuredList unstructuredObjs.SetGroupVersionKind(p.GVK) if err := p.Client.List(ctx, &unstructuredObjs, &listOpts); err != nil { - p.Logger.Error(err, "failed to get a list of resources for pruning", "Labels", p.Labels, "Namespace", p.Namespace) - return nil, fmt.Errorf("failed to get list of objects -- ERROR -- %s", err) + return nil, fmt.Errorf("error getting a list of resources: %w", err) } for _, unsObj := range unstructuredObjs.Items { @@ -187,6 +165,7 @@ func (p Pruner) Prune(ctx context.Context) ([]client.ObjectKey, error) { } if err := p.IsPrunable(obj); isUnprunable(err) { + p.Logger.Info(fmt.Errorf("object is unprunable: %w", err).Error()) continue } else if err != nil { return nil, err @@ -197,38 +176,19 @@ func (p Pruner) Prune(ctx context.Context) ([]client.ObjectKey, error) { objsToPrune, err := p.Strategy(ctx, objs) if err != nil { - p.Logger.Error(err, "failed to get a list of resources to prune from Strategy") - return nil, fmt.Errorf("failed when running Strategy -- ERROR -- %s", err) - } - - if p.DryRun { - // print out objects - return objsToPrune, nil + return nil, fmt.Errorf("error determining prunable objects: %w", err) } // Prune the resources for _, obj := range objsToPrune { - // Prune - prunableObj := &unstructured.Unstructured{} - prunableObj.SetName(obj.Name) - prunableObj.SetNamespace(obj.Namespace) - prunableObj.SetGroupVersionKind(p.GVK) - - if err = p.Client.Delete(ctx, prunableObj); err != nil { - p.Logger.Error(err, "failed to prune resource", "Resource", obj) - return nil, fmt.Errorf("failed to prune object -- ERROR -- %s", err) + if err = p.Client.Delete(ctx, obj); err != nil { + return nil, fmt.Errorf("error pruning object: %w", err) } } return objsToPrune, nil } -/* - ------------------------------------------- - New Auto Pruning API Implementation - ------------------------------------------- -*/ - // 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. diff --git a/prune/prune_test.go b/prune/prune_test.go index 68f4dea..9e24974 100644 --- a/prune/prune_test.go +++ b/prune/prune_test.go @@ -68,7 +68,6 @@ var _ = Describe("Prune", func() { p.Labels = labels p.Namespace = namespace - p.Client = fakeClient } podGVK = corev1.SchemeGroupVersion.WithKind("Pod") @@ -118,7 +117,7 @@ var _ = Describe("Prune", func() { Kind: "NotReal", }) - Expect(IsPrunable(obj)).Should(BeNil()) + Expect(NewRegistry().IsPrunable(obj)).Should(BeNil()) }) }) @@ -137,13 +136,12 @@ var _ = Describe("Prune", func() { labels := map[string]string{"app": "churro"} logger := &logr.Logger{} pruner := NewPruner(fakeClient, - SetRegistry(*registry), - Namespace(namespace), - Labels(labels), - DryRun(), - SetStrategy(nil), - SetLogger(*logger), - GVK(jobGVK)) + WithRegistry(*registry), + WithNamespace(namespace), + WithLabels(labels), + WithStrategy(nil), + WithLogger(*logger), + WithGVK(jobGVK)) Expect(pruner).ShouldNot(BeNil()) Expect(&pruner.Registry).Should(Equal(registry)) @@ -151,7 +149,6 @@ var _ = Describe("Prune", func() { Expect(pruner.Labels).Should(Equal(labels)) Expect(&pruner.Logger).Should(Equal(logger)) Expect(pruner.Strategy).Should(BeNil()) - Expect(pruner.DryRun).Should(BeTrue()) Expect(pruner.GVK).Should(Equal(jobGVK)) Expect(pruner.Client).Should(Equal(fakeClient)) }) @@ -203,7 +200,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig, GVK(jobGVK)) + pruner := NewPruner(fakeClient, prunerConfig, WithGVK(jobGVK)) Expect(pruner).ShouldNot(BeNil()) @@ -233,7 +230,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig, GVK(jobGVK)) + pruner := NewPruner(fakeClient, prunerConfig, WithGVK(jobGVK)) Expect(pruner).ShouldNot(BeNil()) @@ -252,7 +249,7 @@ var _ = Describe("Prune", func() { Expect(len(jobs.Items)).Should(Equal(1)) }) - It("Should Not Prune Resources when DryRun is set to true", func() { + It("Should Not Prune Resources when using a DryRunClient", func() { // Create the test resources - in this case Pods err := createTestPods(fakeClient) @@ -266,12 +263,11 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(pods.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig) + dryRunClient := client.NewDryRunClient(fakeClient) + pruner := NewPruner(dryRunClient, prunerConfig) Expect(pruner).ShouldNot(BeNil()) - pruner.DryRun = true - prunedObjects, err := pruner.Prune(context.Background()) Expect(err).Should(BeNil()) @@ -298,7 +294,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig, GVK(jobGVK)) + pruner := NewPruner(fakeClient, prunerConfig, WithGVK(jobGVK)) Expect(pruner).ShouldNot(BeNil()) @@ -341,7 +337,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig, GVK(jobGVK)) + pruner := NewPruner(fakeClient, prunerConfig, WithGVK(jobGVK)) Expect(pruner).ShouldNot(BeNil()) @@ -389,14 +385,14 @@ var _ = Describe("Prune", func() { RegisterIsPrunableFunc(jobGVK, myIsPrunable) // Override pruner strategy with one that will return an error - pruner.Strategy = func(ctx context.Context, objs []client.Object) ([]client.ObjectKey, error) { + pruner.Strategy = func(ctx context.Context, objs []client.Object) ([]client.Object, error) { return nil, fmt.Errorf("TESTERROR") } prunedObjects, err := pruner.Prune(context.Background()) Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(Equal("failed when running Strategy -- ERROR -- TESTERROR")) + Expect(err.Error()).Should(Equal("error determining prunable objects: TESTERROR")) Expect(prunedObjects).Should(BeNil()) // Get a list of the jobs to make sure we have pruned the ones we expected @@ -420,7 +416,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig, GVK(jobGVK)) + pruner := NewPruner(fakeClient, prunerConfig, WithGVK(jobGVK)) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) @@ -438,7 +434,7 @@ var _ = Describe("Prune", func() { prunedObjects, err := pruner.Prune(context.Background()) Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(ContainSubstring("failed to prune object -- ERROR -- jobs.batch \"churro1\" not found")) + Expect(err.Error()).Should(ContainSubstring("error pruning object: jobs.batch \"churro1\" not found")) Expect(len(prunedObjects)).Should(Equal(0)) // Get a list of the jobs to make sure we have pruned the ones we expected @@ -670,13 +666,13 @@ func createSchemes() (*runtime.Scheme, error) { // myStrategy shows how you can write your own strategy // In this example it simply removes a resource if it has // the name 'churro1' or 'churro2' -func myStrategy(ctx context.Context, objs []client.Object) ([]client.ObjectKey, error) { - var objsToRemove []client.ObjectKey +func myStrategy(ctx context.Context, objs []client.Object) ([]client.Object, error) { + var objsToRemove []client.Object for _, obj := range objs { // If the object has name churro1 or churro2 get rid of it if obj.GetName() == "churro1" || obj.GetName() == "churro2" { - objsToRemove = append(objsToRemove, client.ObjectKeyFromObject(obj)) + objsToRemove = append(objsToRemove, obj) } } diff --git a/prune/registry.go b/prune/registry.go index 18c9a88..3ce4c18 100644 --- a/prune/registry.go +++ b/prune/registry.go @@ -60,8 +60,3 @@ func (r *Registry) IsPrunable(obj client.Object) error { func RegisterIsPrunableFunc(gvk schema.GroupVersionKind, isPrunable IsPrunableFunc) { DefaultRegistry().RegisterIsPrunableFunc(gvk, isPrunable) } - -// IsPrunable checks if an object is prunable -func IsPrunable(obj client.Object) error { - return DefaultRegistry().IsPrunable(obj) -} From 67688741005b777c097f0c3952c3d6c010d15db9 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Thu, 21 Apr 2022 11:12:37 -0400 Subject: [PATCH 03/12] updates based on review comments Signed-off-by: Bryce Palmer --- prune/prunables.go | 5 +- prune/prune.go | 113 ++++++++++++----------- prune/prune_test.go | 216 ++++++++++++++++++++------------------------ prune/registry.go | 5 +- 4 files changed, 164 insertions(+), 175 deletions(-) diff --git a/prune/prunables.go b/prune/prunables.go index 7137448..a01b990 100644 --- a/prune/prunables.go +++ b/prune/prunables.go @@ -15,6 +15,7 @@ package prune import ( + "github.com/go-logr/logr" "sigs.k8s.io/controller-runtime/pkg/client" batchv1 "k8s.io/api/batch/v1" @@ -24,7 +25,7 @@ import ( // DefaultPodIsPrunable is a default IsPrunableFunc to be used specifically with Pod resources. // It marks a Pod resource as prunable if it's Status.Phase is "Succeeded" // This can be overridden by registering your own IsPrunableFunc via the RegisterIsPrunableFunc method -func DefaultPodIsPrunable(obj client.Object) error { +func DefaultPodIsPrunable(obj client.Object, logger logr.Logger) error { pod := obj.(*corev1.Pod) if pod.Status.Phase != corev1.PodSucceeded { return &Unprunable{ @@ -39,7 +40,7 @@ func DefaultPodIsPrunable(obj client.Object) error { // DefaultJobIsPrunable is a default IsPrunableFunc to be used specifically with Job resources. // It marks a Job resource as prunable if it's Status.CompletionTime value is not `nil`, indicating that the Job has completed // This can be overridden by registering your own IsPrunableFunc via the RegisterIsPrunableFunc method -func DefaultJobIsPrunable(obj client.Object) error { +func DefaultJobIsPrunable(obj client.Object, logger logr.Logger) error { job := obj.(*batchv1.Job) if job.Status.CompletionTime == nil { return &Unprunable{ diff --git a/prune/prune.go b/prune/prune.go index f9c9573..eca9a9b 100644 --- a/prune/prune.go +++ b/prune/prune.go @@ -40,27 +40,27 @@ func init() { // Pruner is an object that runs a prune job. type Pruner struct { - Registry + registry Registry - // Client is the controller-runtime client that will be used + // client is the controller-runtime client that will be used // To perform a dry run, use the controller-runtime DryRunClient - Client client.Client + client client.Client - // GVK is the type of objects to prune. + // gvk is the type of objects to prune. // It defaults to Pod - GVK schema.GroupVersionKind + gvk schema.GroupVersionKind - // Strategy is the function used to determine a list of resources that are pruneable - Strategy StrategyFunc + // strategy is the function used to determine a list of resources that are pruneable + strategy StrategyFunc - // Labels is a map of the labels to use for label matching when looking for resources - Labels map[string]string + // labels is a map of the labels to use for label matching when looking for resources + labels map[string]string - // Namespace is the namespace to use when looking for resources - Namespace string + // namespace is the namespace to use when looking for resources + namespace string - // Logger is the logger to use when running pruning functionality - Logger logr.Logger + // logger is the logger to use when running pruning functionality + logger logr.Logger } // Unprunable indicates that it is not allowed to prune a specific object. @@ -80,92 +80,95 @@ type StrategyFunc func(ctx context.Context, objs []client.Object) ([]client.Obje // IsPrunableFunc is a function that checks the data of an object to see whether or not it is safe to prune it. // It should return `nil` if it is safe to prune, `Unprunable` if it is unsafe, or another error. // It should safely assert the object is the expected type, otherwise it might panic. -type IsPrunableFunc func(obj client.Object) error +type IsPrunableFunc func(obj client.Object, logger logr.Logger) error // PrunerOption configures the pruner. type PrunerOption func(p *Pruner) -// WithRegistry can be used to set the Registry field when configuring a Pruner -func WithRegistry(registry Registry) PrunerOption { - return func(p *Pruner) { - p.Registry = registry - } -} - -// WithStrategy can be used to set the Strategy field when configuring a Pruner -func WithStrategy(strategy StrategyFunc) PrunerOption { - return func(p *Pruner) { - p.Strategy = strategy - } -} - // WithNamespace can be used to set the Namespace field when configuring a Pruner func WithNamespace(namespace string) PrunerOption { return func(p *Pruner) { - p.Namespace = namespace + p.namespace = namespace } } // WithLogger can be used to set the Logger field when configuring a Pruner func WithLogger(logger logr.Logger) PrunerOption { return func(p *Pruner) { - p.Logger = logger + p.logger = logger } } // WithLabels can be used to set the Labels field when configuring a Pruner func WithLabels(labels map[string]string) PrunerOption { return func(p *Pruner) { - p.Labels = labels + p.labels = labels } } -// WithGVK can be used to set the GVK field when configuring a Pruner -func WithGVK(gvk schema.GroupVersionKind) PrunerOption { - return func(p *Pruner) { - p.GVK = gvk - } +// GVK returns the schema.GroupVersionKind that the Pruner has set +func (p Pruner) GVK() schema.GroupVersionKind { + return p.gvk +} + +// Labels returns the labels that the Pruner is using to find resources to prune +func (p Pruner) Labels() map[string]string { + return p.labels } -// NewPruner returns a pruner that uses the given strategy to prune objects. -func NewPruner(prunerClient client.Client, opts ...PrunerOption) Pruner { +// Namespace returns the namespace that the Pruner is using to find resources to prune +func (p Pruner) Namespace() string { + return p.namespace +} + +// NewPruner returns a pruner that uses the given strategy to prune objects that have the given GVK +func NewPruner(prunerClient client.Client, gvk schema.GroupVersionKind, strategy StrategyFunc, opts ...PrunerOption) (Pruner, error) { + // check for nil explicit parameters + // if gvk.Group == "" then it maps to the 'core' Group + if prunerClient == nil || + (gvk.Version == "" || gvk.Kind == "") || + strategy == nil { + return Pruner{}, fmt.Errorf("error creating a new Pruner: explicit parameters cannot be nil or contain empty values") + } + pruner := Pruner{ - Registry: defaultRegistry, - Client: prunerClient, - Logger: Logger(context.Background(), Pruner{}), - GVK: corev1.SchemeGroupVersion.WithKind("Pod"), + registry: defaultRegistry, + client: prunerClient, + logger: Logger(context.Background(), Pruner{}), + gvk: gvk, + strategy: strategy, } for _, opt := range opts { opt(&pruner) } - return pruner + return pruner, nil } // Prune runs the pruner. func (p Pruner) Prune(ctx context.Context) ([]client.Object, error) { var objs []client.Object - p.Logger.Info("Starting the pruning process...") + p.logger.Info("Starting the pruning process...") listOpts := client.ListOptions{ - LabelSelector: labels.Set(p.Labels).AsSelector(), - Namespace: p.Namespace, + LabelSelector: labels.Set(p.labels).AsSelector(), + Namespace: p.namespace, } var unstructuredObjs unstructured.UnstructuredList - unstructuredObjs.SetGroupVersionKind(p.GVK) - if err := p.Client.List(ctx, &unstructuredObjs, &listOpts); err != nil { + unstructuredObjs.SetGroupVersionKind(p.gvk) + if err := p.client.List(ctx, &unstructuredObjs, &listOpts); err != nil { return nil, fmt.Errorf("error getting a list of resources: %w", err) } for _, unsObj := range unstructuredObjs.Items { - obj, err := convert(p.Client, p.GVK, &unsObj) + obj, err := convert(p.client, p.gvk, &unsObj) if err != nil { return nil, err } - if err := p.IsPrunable(obj); isUnprunable(err) { - p.Logger.Info(fmt.Errorf("object is unprunable: %w", err).Error()) + if err := p.registry.IsPrunable(obj, p.logger); isUnprunable(err) { + p.logger.Info(fmt.Errorf("object is unprunable: %w", err).Error()) continue } else if err != nil { return nil, err @@ -174,14 +177,14 @@ func (p Pruner) Prune(ctx context.Context) ([]client.Object, error) { objs = append(objs, obj) } - objsToPrune, err := p.Strategy(ctx, objs) + objsToPrune, err := p.strategy(ctx, objs) if err != nil { return nil, fmt.Errorf("error determining prunable objects: %w", err) } // Prune the resources for _, obj := range objsToPrune { - if err = p.Client.Delete(ctx, obj); err != nil { + if err = p.client.Delete(ctx, obj); err != nil { return nil, fmt.Errorf("error pruning object: %w", err) } } @@ -195,8 +198,8 @@ func (p Pruner) Prune(ctx context.Context) ([]client.Object, error) { // keysAndValues allow to add fields to the logs, cf logr documentation. func Logger(ctx context.Context, pruner Pruner, keysAndValues ...interface{}) logr.Logger { var log logr.Logger - if pruner.Logger != (logr.Logger{}) { - log = pruner.Logger + if pruner.logger != (logr.Logger{}) { + log = pruner.logger } else { log = ctrllog.FromContext(ctx) } diff --git a/prune/prune_test.go b/prune/prune_test.go index 9e24974..b63679d 100644 --- a/prune/prune_test.go +++ b/prune/prune_test.go @@ -48,32 +48,23 @@ var _ = Describe("Prune", func() { ) BeforeEach(func() { testScheme, err := createSchemes() - Expect(err).Should(BeNil()) fakeClient = crFake.NewClientBuilder().WithScheme(testScheme).Build() - fakeObj = &corev1.Pod{} // Create our function to configure our pruner prunerConfig = func(p *Pruner) { - // Create the labels we want to select with labels := make(map[string]string) labels["app"] = app - // Set our strategy - p.Strategy = myStrategy - - p.Labels = labels - - p.Namespace = namespace + p.labels = labels + p.namespace = namespace } podGVK = corev1.SchemeGroupVersion.WithKind("Pod") - jobGVK = batchv1.SchemeGroupVersion.WithKind("Job") - }) Describe("Unprunable", func() { @@ -83,7 +74,6 @@ var _ = Describe("Prune", func() { Obj: &fakeObj, Reason: "TestReason", } - Expect(unpruneable.Error()).To(Equal(fmt.Sprintf("unable to prune %s: %s", client.ObjectKeyFromObject(fakeObj), unpruneable.Reason))) }) }) @@ -93,7 +83,6 @@ var _ = Describe("Prune", func() { Describe("NewRegistry()", func() { It("Should Return a New Registry Object", func() { registry := NewRegistry() - Expect(registry).ShouldNot(BeNil()) }) }) @@ -101,10 +90,10 @@ var _ = Describe("Prune", func() { Describe("RegisterIsPrunableFunc()", func() { It("Should Add an Entry to Registry Prunables Map", func() { registry := NewRegistry() - Expect(registry).ShouldNot(BeNil()) registry.RegisterIsPrunableFunc(podGVK, myIsPrunable) + Expect(registry.prunables).Should(HaveKey(podGVK)) }) }) @@ -117,42 +106,80 @@ var _ = Describe("Prune", func() { Kind: "NotReal", }) - Expect(NewRegistry().IsPrunable(obj)).Should(BeNil()) + Expect(NewRegistry().IsPrunable(obj, logr.Logger{})).Should(BeNil()) }) }) }) Describe("Pruner", func() { Describe("NewPruner()", func() { - It("Should Return a New Pruner Object", func() { - pruner := NewPruner(fakeClient) + Context("Successful", func() { + It("Should Return a New Pruner Object", func() { + pruner, err := NewPruner(fakeClient, podGVK, myStrategy) + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + }) - Expect(pruner).ShouldNot(BeNil()) - }) + It("Should Return a New Pruner Object with Custom Configuration", func() { + namespace := "namespace" + labels := map[string]string{"app": "churro"} + logger := &logr.Logger{} + pruner, err := NewPruner(fakeClient, + jobGVK, + myStrategy, + WithNamespace(namespace), + WithLabels(labels), + WithLogger(*logger)) - It("Should Return a New Pruner Object with Custom Configuration", func() { - registry := NewRegistry() - namespace := "namespace" - labels := map[string]string{"app": "churro"} - logger := &logr.Logger{} - pruner := NewPruner(fakeClient, - WithRegistry(*registry), - WithNamespace(namespace), - WithLabels(labels), - WithStrategy(nil), - WithLogger(*logger), - WithGVK(jobGVK)) - - Expect(pruner).ShouldNot(BeNil()) - Expect(&pruner.Registry).Should(Equal(registry)) - Expect(pruner.Namespace).Should(Equal(namespace)) - Expect(pruner.Labels).Should(Equal(labels)) - Expect(&pruner.Logger).Should(Equal(logger)) - Expect(pruner.Strategy).Should(BeNil()) - Expect(pruner.GVK).Should(Equal(jobGVK)) - Expect(pruner.Client).Should(Equal(fakeClient)) + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + Expect(&pruner.registry).Should(Equal(DefaultRegistry())) + Expect(pruner.namespace).Should(Equal(namespace)) + Expect(pruner.labels).Should(Equal(labels)) + Expect(&pruner.logger).Should(Equal(logger)) + Expect(pruner.strategy).ShouldNot(BeNil()) + Expect(pruner.gvk).Should(Equal(jobGVK)) + Expect(pruner.client).Should(Equal(fakeClient)) + }) }) + Context("Errors", func() { + errorString := "error creating a new Pruner: explicit parameters cannot be nil or contain empty values" + + It("Should Error if client.Client Parameter is nil", func() { + pruner, err := NewPruner(nil, podGVK, myStrategy) + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).Should(Equal(errorString)) + Expect(pruner).ShouldNot(BeNil()) + }) + + It("Should Error if schema.GroupVersionKind Parameter fields have empty values", func() { + // empty GVK struct + pruner, err := NewPruner(fakeClient, schema.GroupVersionKind{}, myStrategy) + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).Should(Equal(errorString)) + Expect(pruner).ShouldNot(BeNil()) + + // empty Version + pruner, err = NewPruner(fakeClient, schema.GroupVersionKind{Group: "group", Kind: "kind"}, myStrategy) + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).Should(Equal(errorString)) + Expect(pruner).ShouldNot(BeNil()) + + // empty Kind + pruner, err = NewPruner(fakeClient, schema.GroupVersionKind{Group: "group", Version: "version"}, myStrategy) + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).Should(Equal(errorString)) + Expect(pruner).ShouldNot(BeNil()) + }) + + It("Should Error if StrategyFunc parameter is nil", func() { + pruner, err := NewPruner(fakeClient, podGVK, nil) + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).Should(Equal(errorString)) + Expect(pruner).ShouldNot(BeNil()) + }) + }) }) Describe("Prune()", func() { @@ -160,28 +187,25 @@ var _ = Describe("Prune", func() { It("Should Prune Pods with Default IsPrunableFunc", func() { // Create the test resources - in this case Pods err := createTestPods(fakeClient) - Expect(err).Should(BeNil()) // Make sure the pod resources are properly created pods := &unstructured.UnstructuredList{} pods.SetGroupVersionKind(podGVK) err = fakeClient.List(context.Background(), pods) - Expect(err).Should(BeNil()) Expect(len(pods.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig) - + pruner, err := NewPruner(fakeClient, podGVK, myStrategy, prunerConfig) + Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).Should(BeNil()) Expect(len(prunedObjects)).Should(Equal(2)) + // Get a list of the Pods to make sure we have pruned the ones we expected err = fakeClient.List(context.Background(), pods) - Expect(err).Should(BeNil()) Expect(len(pods.Items)).Should(Equal(1)) }) @@ -189,29 +213,25 @@ var _ = Describe("Prune", func() { It("Should Prune Jobs with Default IsPrunableFunc", func() { // Create the test resources - in this case Jobs err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig, WithGVK(jobGVK)) - + pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, prunerConfig) + Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).Should(BeNil()) Expect(len(prunedObjects)).Should(Equal(2)) // Get a list of the job to make sure we have pruned the ones we expected err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(1)) }) @@ -219,32 +239,28 @@ var _ = Describe("Prune", func() { It("Should Remove Resource When Using a Custom IsPrunableFunc", func() { // Create the test resources - in this case Jobs err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig, WithGVK(jobGVK)) - + pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, prunerConfig) + Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) // Register our custom IsPrunableFunc RegisterIsPrunableFunc(jobGVK, myIsPrunable) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).Should(BeNil()) Expect(len(prunedObjects)).Should(Equal(2)) // Get a list of the jobs to make sure we have pruned the ones we expected err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(1)) }) @@ -252,30 +268,26 @@ var _ = Describe("Prune", func() { It("Should Not Prune Resources when using a DryRunClient", func() { // Create the test resources - in this case Pods err := createTestPods(fakeClient) - Expect(err).Should(BeNil()) // Make sure the pod resources are properly created pods := &unstructured.UnstructuredList{} pods.SetGroupVersionKind(podGVK) err = fakeClient.List(context.Background(), pods) - Expect(err).Should(BeNil()) Expect(len(pods.Items)).Should(Equal(3)) dryRunClient := client.NewDryRunClient(fakeClient) - pruner := NewPruner(dryRunClient, prunerConfig) - + pruner, err := NewPruner(dryRunClient, podGVK, myStrategy, prunerConfig) + Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).Should(BeNil()) Expect(len(prunedObjects)).Should(Equal(2)) // Get a list of the Pods to make sure we haven't pruned any err = fakeClient.List(context.Background(), pods) - Expect(err).Should(BeNil()) Expect(len(pods.Items)).Should(Equal(3)) }) @@ -283,23 +295,21 @@ var _ = Describe("Prune", func() { It("Should Skip Pruning a Resource If IsPrunable Returns an Error of Type Unprunable", func() { // Create the test resources - in this case Jobs err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig, WithGVK(jobGVK)) - + pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, prunerConfig) + Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) // IsPrunableFunc that throws Unprunable error - errorPrunableFunc := func(obj client.Object) error { + errorPrunableFunc := func(obj client.Object, logger logr.Logger) error { return &Unprunable{ Obj: &obj, Reason: "TEST", @@ -310,13 +320,11 @@ var _ = Describe("Prune", func() { RegisterIsPrunableFunc(jobGVK, errorPrunableFunc) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).Should(BeNil()) Expect(len(prunedObjects)).Should(Equal(0)) // Get a list of the jobs to make sure we have pruned the ones we expected err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) }) @@ -326,23 +334,21 @@ var _ = Describe("Prune", func() { It("Should Return an Error if IsPrunableFunc Returns an Error That is not of Type Unprunable", func() { // Create the test resources - in this case Jobs err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig, WithGVK(jobGVK)) - + pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, prunerConfig) + Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) // IsPrunableFunc that throws non Unprunable error - errorPrunableFunc := func(obj client.Object) error { + errorPrunableFunc := func(obj client.Object, logger logr.Logger) error { return fmt.Errorf("TEST") } @@ -350,14 +356,12 @@ var _ = Describe("Prune", func() { RegisterIsPrunableFunc(jobGVK, errorPrunableFunc) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).ShouldNot(BeNil()) Expect(err.Error()).Should(Equal("TEST")) Expect(len(prunedObjects)).Should(Equal(0)) // Get a list of the jobs to make sure we have pruned the ones we expected err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) }) @@ -365,39 +369,34 @@ var _ = Describe("Prune", func() { It("Should Return An Error If Strategy Function Returns An Error", func() { // Create the test resources - in this case Jobs err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig) + // strategy that will return an error + prunerStrategy := func(ctx context.Context, objs []client.Object) ([]client.Object, error) { + return nil, fmt.Errorf("TESTERROR") + } + pruner, err := NewPruner(fakeClient, jobGVK, prunerStrategy, prunerConfig) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) // Register our custom IsPrunableFunc RegisterIsPrunableFunc(jobGVK, myIsPrunable) - // Override pruner strategy with one that will return an error - pruner.Strategy = func(ctx context.Context, objs []client.Object) ([]client.Object, error) { - return nil, fmt.Errorf("TESTERROR") - } - prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).ShouldNot(BeNil()) Expect(err.Error()).Should(Equal("error determining prunable objects: TESTERROR")) Expect(prunedObjects).Should(BeNil()) // Get a list of the jobs to make sure we have pruned the ones we expected err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) }) @@ -405,25 +404,22 @@ var _ = Describe("Prune", func() { It("Should Return an Error if it can not Prune a Resource", func() { // Create the test resources - in this case Jobs err := createTestJobs(fakeClient) - Expect(err).Should(BeNil()) // Make sure the job resources are properly created jobs := &unstructured.UnstructuredList{} jobs.SetGroupVersionKind(jobGVK) err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner := NewPruner(fakeClient, prunerConfig, WithGVK(jobGVK)) - + pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, prunerConfig) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) // IsPrunableFunc that returns nil but also deletes the object // so that it will throw an error when attempting to remove the object - prunableFunc := func(obj client.Object) error { + prunableFunc := func(obj client.Object, logger logr.Logger) error { _ = fakeClient.Delete(context.TODO(), obj, &client.DeleteOptions{}) return nil } @@ -432,14 +428,12 @@ var _ = Describe("Prune", func() { RegisterIsPrunableFunc(jobGVK, prunableFunc) prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).ShouldNot(BeNil()) Expect(err.Error()).Should(ContainSubstring("error pruning object: jobs.batch \"churro1\" not found")) Expect(len(prunedObjects)).Should(Equal(0)) // Get a list of the jobs to make sure we have pruned the ones we expected err = fakeClient.List(context.Background(), jobs) - Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(0)) }) @@ -464,9 +458,7 @@ var _ = Describe("Prune", func() { pod.SetGroupVersionKind(podGVK) // Run it through DefaultPodIsPrunable - err := DefaultPodIsPrunable(pod) - - // Check that return is 'nil' + err := DefaultPodIsPrunable(pod, logr.Logger{}) Expect(err).Should(BeNil()) }) @@ -477,7 +469,7 @@ var _ = Describe("Prune", func() { defer expectPanic() // Run it through DefaultPodIsPrunable - _ = DefaultPodIsPrunable(notPod) + _ = DefaultPodIsPrunable(notPod, logr.Logger{}) }) It("Should Return An Error When Kind Is 'Pod' But Phase Is Not 'Succeeded'", func() { @@ -495,9 +487,7 @@ var _ = Describe("Prune", func() { pod.SetGroupVersionKind(podGVK) // Run it through DefaultPodIsPrunable - err := DefaultPodIsPrunable(pod) - - // Check that return is error + err := DefaultPodIsPrunable(pod, logr.Logger{}) Expect(err).ShouldNot(BeNil()) var expectErr *Unprunable Expect(errors.As(err, &expectErr)).Should(BeTrue()) @@ -523,9 +513,7 @@ var _ = Describe("Prune", func() { job.SetGroupVersionKind(jobGVK) // Run it through DefaultJobIsPrunable - err := DefaultJobIsPrunable(job) - - // Check that return is 'nil' + err := DefaultJobIsPrunable(job, logr.Logger{}) Expect(err).Should(BeNil()) }) @@ -536,7 +524,7 @@ var _ = Describe("Prune", func() { defer expectPanic() // Run it through DefaultJobIsPrunable - _ = DefaultJobIsPrunable(notJob) + _ = DefaultJobIsPrunable(notJob, logr.Logger{}) }) It("Should Return An Error When Kind Is 'Job' But 'CompletionTime' is 'nil'", func() { @@ -554,9 +542,7 @@ var _ = Describe("Prune", func() { job.SetGroupVersionKind(jobGVK) // Run it through DefaultJobIsPrunable - err := DefaultJobIsPrunable(job) - - // Check that return is error + err := DefaultJobIsPrunable(job, logr.Logger{}) Expect(err).ShouldNot(BeNil()) var expectErr *Unprunable Expect(errors.As(err, &expectErr)).Should(BeTrue()) @@ -569,7 +555,7 @@ var _ = Describe("Prune", func() { }) // create 3 pods and 3 jobs with different start times (now, 2 days old, 4 days old) -func createTestPods(client client.Client) (err error) { +func createTestPods(client client.Client) error { // some defaults ns := namespace appLabel := app @@ -593,8 +579,8 @@ func createTestPods(client client.Client) (err error) { }, }) pod.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Pod")) - err = client.Create(context.Background(), pod) + err := client.Create(context.Background(), pod) if err != nil { return err } @@ -604,7 +590,7 @@ func createTestPods(client client.Client) (err error) { } // create 3 pods and 3 jobs with different start times (now, 2 days old, 4 days old) -func createTestJobs(client client.Client) (err error) { +func createTestJobs(client client.Client) error { // some defaults ns := namespace appLabel := app @@ -628,8 +614,8 @@ func createTestJobs(client client.Client) (err error) { }, }) job.SetGroupVersionKind(batchv1.SchemeGroupVersion.WithKind("Job")) - err = client.Create(context.Background(), job) + err := client.Create(context.Background(), job) if err != nil { return err } @@ -641,7 +627,6 @@ func createTestJobs(client client.Client) (err error) { // createSchemes is a helper function to set up the schemes needed to run // our tests utilizing controller-runtime's fake client func createSchemes() (*runtime.Scheme, error) { - corev1SchemeBuilder := &scheme.Builder{GroupVersion: corev1.SchemeGroupVersion} corev1SchemeBuilder.Register(&corev1.Pod{}, &corev1.PodList{}) @@ -684,12 +669,11 @@ func myStrategy(ctx context.Context, objs []client.Object) ([]client.Object, err // that is expected to panic is called func expectPanic() { r := recover() - Expect(r).ShouldNot(BeNil()) } // myIsPrunable shows how you can write your own IsPrunableFunc // In this example it simply removes all resources -func myIsPrunable(obj client.Object) error { +func myIsPrunable(obj client.Object, logger logr.Logger) error { return nil } diff --git a/prune/registry.go b/prune/registry.go index 3ce4c18..0a20ebf 100644 --- a/prune/registry.go +++ b/prune/registry.go @@ -15,6 +15,7 @@ package prune import ( + "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -47,13 +48,13 @@ func (r *Registry) RegisterIsPrunableFunc(gvk schema.GroupVersionKind, isPrunabl } // IsPrunable checks if an object is prunable -func (r *Registry) IsPrunable(obj client.Object) error { +func (r *Registry) IsPrunable(obj client.Object, logger logr.Logger) error { isPrunable, ok := r.prunables[obj.GetObjectKind().GroupVersionKind()] if !ok { return nil } - return isPrunable(obj) + return isPrunable(obj, logger) } // RegisterIsPrunableFunc registers a function to check whether it is safe to prune a resource of a certain type. From b850c0b44389ba7c21c857ef98806625963ff884 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Thu, 21 Apr 2022 11:18:38 -0400 Subject: [PATCH 04/12] update tests for linting checks Signed-off-by: Bryce Palmer --- prune/prune_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prune/prune_test.go b/prune/prune_test.go index b63679d..f1d7574 100644 --- a/prune/prune_test.go +++ b/prune/prune_test.go @@ -184,7 +184,7 @@ var _ = Describe("Prune", func() { Describe("Prune()", func() { Context("Does not return an Error", func() { - It("Should Prune Pods with Default IsPrunableFunc", func() { + It("Should Prune Pods with Default IsPrunableFunc", func() { //nolint:dupl // Create the test resources - in this case Pods err := createTestPods(fakeClient) Expect(err).Should(BeNil()) @@ -210,7 +210,7 @@ var _ = Describe("Prune", func() { Expect(len(pods.Items)).Should(Equal(1)) }) - It("Should Prune Jobs with Default IsPrunableFunc", func() { + It("Should Prune Jobs with Default IsPrunableFunc", func() { //nolint:dupl // Create the test resources - in this case Jobs err := createTestJobs(fakeClient) Expect(err).Should(BeNil()) From c39650ed734909576895d8d1d152926f2f0ec196 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Fri, 22 Apr 2022 13:23:02 -0400 Subject: [PATCH 05/12] update tests Signed-off-by: Bryce Palmer --- prune/prune_test.go | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/prune/prune_test.go b/prune/prune_test.go index f1d7574..ca42d8a 100644 --- a/prune/prune_test.go +++ b/prune/prune_test.go @@ -38,13 +38,14 @@ import ( const namespace = "default" const app = "churro" +var appLabels = map[string]string{"app": app} + var _ = Describe("Prune", func() { var ( - fakeClient client.Client - fakeObj client.Object - prunerConfig PrunerOption - podGVK schema.GroupVersionKind - jobGVK schema.GroupVersionKind + fakeClient client.Client + fakeObj client.Object + podGVK schema.GroupVersionKind + jobGVK schema.GroupVersionKind ) BeforeEach(func() { testScheme, err := createSchemes() @@ -53,16 +54,6 @@ var _ = Describe("Prune", func() { fakeClient = crFake.NewClientBuilder().WithScheme(testScheme).Build() fakeObj = &corev1.Pod{} - // Create our function to configure our pruner - prunerConfig = func(p *Pruner) { - // Create the labels we want to select with - labels := make(map[string]string) - labels["app"] = app - - p.labels = labels - p.namespace = namespace - } - podGVK = corev1.SchemeGroupVersion.WithKind("Pod") jobGVK = batchv1.SchemeGroupVersion.WithKind("Job") }) @@ -196,7 +187,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(pods.Items)).Should(Equal(3)) - pruner, err := NewPruner(fakeClient, podGVK, myStrategy, prunerConfig) + pruner, err := NewPruner(fakeClient, podGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) @@ -222,7 +213,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, prunerConfig) + pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) @@ -248,7 +239,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, prunerConfig) + pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) @@ -278,7 +269,7 @@ var _ = Describe("Prune", func() { Expect(len(pods.Items)).Should(Equal(3)) dryRunClient := client.NewDryRunClient(fakeClient) - pruner, err := NewPruner(dryRunClient, podGVK, myStrategy, prunerConfig) + pruner, err := NewPruner(dryRunClient, podGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) @@ -304,7 +295,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, prunerConfig) + pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) @@ -343,7 +334,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, prunerConfig) + pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) @@ -383,7 +374,7 @@ var _ = Describe("Prune", func() { return nil, fmt.Errorf("TESTERROR") } - pruner, err := NewPruner(fakeClient, jobGVK, prunerStrategy, prunerConfig) + pruner, err := NewPruner(fakeClient, jobGVK, prunerStrategy, WithLabels(appLabels), WithNamespace(namespace)) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) @@ -413,7 +404,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, prunerConfig) + pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) From 88fb3e05a17fe5e24f02a42451fd77c66b69e3f4 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Fri, 22 Apr 2022 14:12:50 -0400 Subject: [PATCH 06/12] update tests and nil verification Signed-off-by: Bryce Palmer --- prune/prune.go | 9 +-- prune/prune_test.go | 139 ++++++++++++++++++++++++-------------------- 2 files changed, 78 insertions(+), 70 deletions(-) diff --git a/prune/prune.go b/prune/prune.go index eca9a9b..d5b2584 100644 --- a/prune/prune.go +++ b/prune/prune.go @@ -123,12 +123,9 @@ func (p Pruner) Namespace() string { // NewPruner returns a pruner that uses the given strategy to prune objects that have the given GVK func NewPruner(prunerClient client.Client, gvk schema.GroupVersionKind, strategy StrategyFunc, opts ...PrunerOption) (Pruner, error) { - // check for nil explicit parameters - // if gvk.Group == "" then it maps to the 'core' Group - if prunerClient == nil || - (gvk.Version == "" || gvk.Kind == "") || - strategy == nil { - return Pruner{}, fmt.Errorf("error creating a new Pruner: explicit parameters cannot be nil or contain empty values") + + if gvk.Empty() { + return Pruner{}, fmt.Errorf("error when creating a new Pruner: gvk parameter can not be empty") } pruner := Pruner{ diff --git a/prune/prune_test.go b/prune/prune_test.go index ca42d8a..4cee607 100644 --- a/prune/prune_test.go +++ b/prune/prune_test.go @@ -104,72 +104,40 @@ var _ = Describe("Prune", func() { }) Describe("Pruner", func() { Describe("NewPruner()", func() { - Context("Successful", func() { - It("Should Return a New Pruner Object", func() { - pruner, err := NewPruner(fakeClient, podGVK, myStrategy) - Expect(err).Should(BeNil()) - Expect(pruner).ShouldNot(BeNil()) - }) - - It("Should Return a New Pruner Object with Custom Configuration", func() { - namespace := "namespace" - labels := map[string]string{"app": "churro"} - logger := &logr.Logger{} - pruner, err := NewPruner(fakeClient, - jobGVK, - myStrategy, - WithNamespace(namespace), - WithLabels(labels), - WithLogger(*logger)) - - Expect(err).Should(BeNil()) - Expect(pruner).ShouldNot(BeNil()) - Expect(&pruner.registry).Should(Equal(DefaultRegistry())) - Expect(pruner.namespace).Should(Equal(namespace)) - Expect(pruner.labels).Should(Equal(labels)) - Expect(&pruner.logger).Should(Equal(logger)) - Expect(pruner.strategy).ShouldNot(BeNil()) - Expect(pruner.gvk).Should(Equal(jobGVK)) - Expect(pruner.client).Should(Equal(fakeClient)) - }) + It("Should Return a New Pruner Object", func() { + pruner, err := NewPruner(fakeClient, podGVK, myStrategy) + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) }) - Context("Errors", func() { - errorString := "error creating a new Pruner: explicit parameters cannot be nil or contain empty values" - - It("Should Error if client.Client Parameter is nil", func() { - pruner, err := NewPruner(nil, podGVK, myStrategy) - Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(Equal(errorString)) - Expect(pruner).ShouldNot(BeNil()) - }) - - It("Should Error if schema.GroupVersionKind Parameter fields have empty values", func() { - // empty GVK struct - pruner, err := NewPruner(fakeClient, schema.GroupVersionKind{}, myStrategy) - Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(Equal(errorString)) - Expect(pruner).ShouldNot(BeNil()) - - // empty Version - pruner, err = NewPruner(fakeClient, schema.GroupVersionKind{Group: "group", Kind: "kind"}, myStrategy) - Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(Equal(errorString)) - Expect(pruner).ShouldNot(BeNil()) - - // empty Kind - pruner, err = NewPruner(fakeClient, schema.GroupVersionKind{Group: "group", Version: "version"}, myStrategy) - Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(Equal(errorString)) - Expect(pruner).ShouldNot(BeNil()) - }) + It("Should Return a New Pruner Object with Custom Configuration", func() { + namespace := "namespace" + labels := map[string]string{"app": "churro"} + logger := &logr.Logger{} + pruner, err := NewPruner(fakeClient, + jobGVK, + myStrategy, + WithNamespace(namespace), + WithLabels(labels), + WithLogger(*logger)) + + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + Expect(&pruner.registry).Should(Equal(DefaultRegistry())) + Expect(pruner.namespace).Should(Equal(namespace)) + Expect(pruner.labels).Should(Equal(labels)) + Expect(&pruner.logger).Should(Equal(logger)) + Expect(pruner.strategy).ShouldNot(BeNil()) + Expect(pruner.gvk).Should(Equal(jobGVK)) + Expect(pruner.client).Should(Equal(fakeClient)) + }) - It("Should Error if StrategyFunc parameter is nil", func() { - pruner, err := NewPruner(fakeClient, podGVK, nil) - Expect(err).ShouldNot(BeNil()) - Expect(err.Error()).Should(Equal(errorString)) - Expect(pruner).ShouldNot(BeNil()) - }) + It("Should Error if schema.GroupVersionKind Parameter is empty", func() { + // empty GVK struct + pruner, err := NewPruner(fakeClient, schema.GroupVersionKind{}, myStrategy) + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).Should(Equal("error when creating a new Pruner: gvk parameter can not be empty")) + Expect(pruner).ShouldNot(BeNil()) }) }) @@ -268,7 +236,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(pods.Items)).Should(Equal(3)) - dryRunClient := client.NewDryRunClient(fakeClient) + dryRunClient := newDryRunClient(fakeClient) pruner, err := NewPruner(dryRunClient, podGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) @@ -543,8 +511,51 @@ var _ = Describe("Prune", func() { }) }) + Describe("GVK()", func() { + It("Should return the GVK field in the Pruner", func() { + pruner, err := NewPruner(fakeClient, podGVK, myStrategy) + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + Expect(pruner.GVK()).Should(Equal(podGVK)) + }) + }) + + Describe("Labels()", func() { + It("Should return the Labels field in the Pruner", func() { + pruner, err := NewPruner(fakeClient, podGVK, myStrategy, WithLabels(appLabels)) + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + Expect(pruner.Labels()).Should(Equal(appLabels)) + }) + }) + + Describe("Namespace()", func() { + It("Should return the Namespace field in the Pruner", func() { + pruner, err := NewPruner(fakeClient, podGVK, myStrategy, WithNamespace(namespace)) + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + Expect(pruner.Namespace()).Should(Equal(namespace)) + }) + }) }) +// TODO(everettraven): Remove once https://github.com/kubernetes-sigs/controller-runtime/pull/1873 is released +//--- +type dryRunClient struct { + client.Client +} + +func newDryRunClient(baseClient client.Client) client.Client { + return dryRunClient{client.NewDryRunClient(baseClient)} +} + +// Delete implements a dry run delete, that is currently broken in the latest release. +func (c dryRunClient) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + return nil +} + +//--- + // create 3 pods and 3 jobs with different start times (now, 2 days old, 4 days old) func createTestPods(client client.Client) error { // some defaults From 264c78d7aa3a87b4191cff6e09f0a7d1b94cb08e Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Fri, 22 Apr 2022 15:43:23 -0400 Subject: [PATCH 07/12] Add common strategy functions Signed-off-by: Bryce Palmer --- prune/prune_test.go | 132 +++++++++++++++++++++++++++++++------------- 1 file changed, 94 insertions(+), 38 deletions(-) diff --git a/prune/prune_test.go b/prune/prune_test.go index 4cee607..8a8776d 100644 --- a/prune/prune_test.go +++ b/prune/prune_test.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + "time" "github.com/go-logr/logr" . "github.com/onsi/ginkgo" @@ -143,7 +144,16 @@ var _ = Describe("Prune", func() { Describe("Prune()", func() { Context("Does not return an Error", func() { - It("Should Prune Pods with Default IsPrunableFunc", func() { //nolint:dupl + testPruneWithDefaultIsPrunableFunc := func(gvk schema.GroupVersionKind) { + pruner, err := NewPruner(fakeClient, gvk, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + + prunedObjects, err := pruner.Prune(context.Background()) + Expect(err).Should(BeNil()) + Expect(len(prunedObjects)).Should(Equal(2)) + } + It("Should Prune Pods with Default IsPrunableFunc", func() { // Create the test resources - in this case Pods err := createTestPods(fakeClient) Expect(err).Should(BeNil()) @@ -155,13 +165,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(pods.Items)).Should(Equal(3)) - pruner, err := NewPruner(fakeClient, podGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) - Expect(err).Should(BeNil()) - Expect(pruner).ShouldNot(BeNil()) - - prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).Should(BeNil()) - Expect(len(prunedObjects)).Should(Equal(2)) + testPruneWithDefaultIsPrunableFunc(podGVK) // Get a list of the Pods to make sure we have pruned the ones we expected err = fakeClient.List(context.Background(), pods) @@ -169,7 +173,7 @@ var _ = Describe("Prune", func() { Expect(len(pods.Items)).Should(Equal(1)) }) - It("Should Prune Jobs with Default IsPrunableFunc", func() { //nolint:dupl + It("Should Prune Jobs with Default IsPrunableFunc", func() { // Create the test resources - in this case Jobs err := createTestJobs(fakeClient) Expect(err).Should(BeNil()) @@ -181,13 +185,7 @@ var _ = Describe("Prune", func() { Expect(err).Should(BeNil()) Expect(len(jobs.Items)).Should(Equal(3)) - pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, WithLabels(appLabels), WithNamespace(namespace)) - Expect(err).Should(BeNil()) - Expect(pruner).ShouldNot(BeNil()) - - prunedObjects, err := pruner.Prune(context.Background()) - Expect(err).Should(BeNil()) - Expect(len(prunedObjects)).Should(Equal(2)) + testPruneWithDefaultIsPrunableFunc(jobGVK) // Get a list of the job to make sure we have pruned the ones we expected err = fakeClient.List(context.Background(), jobs) @@ -399,6 +397,33 @@ var _ = Describe("Prune", func() { }) }) + + Describe("GVK()", func() { + It("Should return the GVK field in the Pruner", func() { + pruner, err := NewPruner(fakeClient, podGVK, myStrategy) + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + Expect(pruner.GVK()).Should(Equal(podGVK)) + }) + }) + + Describe("Labels()", func() { + It("Should return the Labels field in the Pruner", func() { + pruner, err := NewPruner(fakeClient, podGVK, myStrategy, WithLabels(appLabels)) + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + Expect(pruner.Labels()).Should(Equal(appLabels)) + }) + }) + + Describe("Namespace()", func() { + It("Should return the Namespace field in the Pruner", func() { + pruner, err := NewPruner(fakeClient, podGVK, myStrategy, WithNamespace(namespace)) + Expect(err).Should(BeNil()) + Expect(pruner).ShouldNot(BeNil()) + Expect(pruner.Namespace()).Should(Equal(namespace)) + }) + }) }) Context("DefaultPodIsPrunable", func() { @@ -511,32 +536,38 @@ var _ = Describe("Prune", func() { }) }) - Describe("GVK()", func() { - It("Should return the GVK field in the Pruner", func() { - pruner, err := NewPruner(fakeClient, podGVK, myStrategy) + Context("NewPruneByCountStrategy", func() { + resources := createDatedResources() + It("Should return the 3 oldest resources", func() { + resourcesToRemove, err := NewPruneByCountStrategy(2)(context.Background(), resources) Expect(err).Should(BeNil()) - Expect(pruner).ShouldNot(BeNil()) - Expect(pruner.GVK()).Should(Equal(podGVK)) + Expect(resourcesToRemove).Should(Equal(resources[2:])) }) - }) - Describe("Labels()", func() { - It("Should return the Labels field in the Pruner", func() { - pruner, err := NewPruner(fakeClient, podGVK, myStrategy, WithLabels(appLabels)) + It("Should return nil", func() { + resourcesToRemove, err := NewPruneByCountStrategy(5)(context.Background(), resources) Expect(err).Should(BeNil()) - Expect(pruner).ShouldNot(BeNil()) - Expect(pruner.Labels()).Should(Equal(appLabels)) + Expect(resourcesToRemove).Should(BeNil()) }) }) - Describe("Namespace()", func() { - It("Should return the Namespace field in the Pruner", func() { - pruner, err := NewPruner(fakeClient, podGVK, myStrategy, WithNamespace(namespace)) + Context("NewPruneByDateStrategy", func() { + resources := createDatedResources() + It("Should return 2 resources", func() { + date := time.Now().Add(time.Hour * time.Duration(2)) + resourcesToRemove, err := NewPruneByDateStrategy(date)(context.Background(), resources) Expect(err).Should(BeNil()) - Expect(pruner).ShouldNot(BeNil()) - Expect(pruner.Namespace()).Should(Equal(namespace)) + Expect(len(resourcesToRemove)).Should(Equal(2)) + }) + + It("Should return 0 resources", func() { + date := time.Now().Add(time.Hour * time.Duration(24)) + resourcesToRemove, err := NewPruneByDateStrategy(date)(context.Background(), resources) + Expect(err).Should(BeNil()) + Expect(len(resourcesToRemove)).Should(Equal(0)) }) }) + }) // TODO(everettraven): Remove once https://github.com/kubernetes-sigs/controller-runtime/pull/1873 is released @@ -593,10 +624,6 @@ func createTestPods(client client.Client) error { // create 3 pods and 3 jobs with different start times (now, 2 days old, 4 days old) func createTestJobs(client client.Client) error { - // some defaults - ns := namespace - appLabel := app - // Due to some weirdness in the way the fake client is set up we need to create our // Kubernetes objects via the unstructured.Unstructured method for i := 0; i < 3; i++ { @@ -606,9 +633,9 @@ func createTestJobs(client client.Client) error { "kind": "Job", "metadata": map[string]interface{}{ "name": fmt.Sprintf("churro%d", i), - "namespace": ns, + "namespace": namespace, "labels": map[string]interface{}{ - "app": appLabel, + "app": app, }, }, "status": map[string]interface{}{ @@ -626,6 +653,35 @@ func createTestJobs(client client.Client) error { return nil } +// createDatedResources is a helper function to get an array of client.Object that have +// different CreationTimestamps to test the common strategy functions +func createDatedResources() []client.Object { + var jobs []client.Object + for i := 0; i < 5; i++ { + job := &unstructured.Unstructured{} + job.SetUnstructuredContent(map[string]interface{}{ + "apiVersion": "batch/v1", + "kind": "Job", + "metadata": map[string]interface{}{ + "name": fmt.Sprintf("churro%d", i), + "namespace": namespace, + "labels": map[string]interface{}{ + "app": app, + }, + }, + "status": map[string]interface{}{ + "completionTime": metav1.Now(), + }, + }) + job.SetGroupVersionKind(batchv1.SchemeGroupVersion.WithKind("Job")) + job.SetCreationTimestamp(metav1.NewTime(time.Now().Add(time.Hour * time.Duration(i)))) + + jobs = append(jobs, job) + } + + return jobs +} + // createSchemes is a helper function to set up the schemes needed to run // our tests utilizing controller-runtime's fake client func createSchemes() (*runtime.Scheme, error) { From 3bf051ac2a5a2b6a9141c8d1c43c661e0d1e2257 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Fri, 22 Apr 2022 15:44:14 -0400 Subject: [PATCH 08/12] Add common strategies Signed-off-by: Bryce Palmer --- prune/strategies.go | 62 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 prune/strategies.go diff --git a/prune/strategies.go b/prune/strategies.go new file mode 100644 index 0000000..0e25890 --- /dev/null +++ b/prune/strategies.go @@ -0,0 +1,62 @@ +// Copyright 2021 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prune + +import ( + "context" + "sort" + "time" + + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// NewPruneByCountStrategy returns a StrategyFunc that will return a list of +// resources to prune based on a maximum count of resources allowed. +// If the max count of resources is exceeded, the oldest resources are prioritized for pruning +func NewPruneByCountStrategy(count int) StrategyFunc { + return func(ctx context.Context, objs []client.Object) ([]client.Object, error) { + + if len(objs) <= count { + return nil, nil + } + + // sort objects by creation date + sortedObjs := objs + + sort.SliceStable(sortedObjs, func(i, j int) bool { + iTimestamp := sortedObjs[i].GetCreationTimestamp() + jTimestamp := sortedObjs[j].GetCreationTimestamp() + return iTimestamp.Before(&jTimestamp) + }) + + return sortedObjs[count:], nil + } +} + +// NewPruneByDateStrategy returns a StrategyFunc that will return a list of +// resources to prune where the resource CreationTimestamp is after the given time.Time. +func NewPruneByDateStrategy(date time.Time) StrategyFunc { + return func(ctx context.Context, objs []client.Object) ([]client.Object, error) { + var objsToPrune []client.Object + + for _, obj := range objs { + if obj.GetCreationTimestamp().After(date) { + objsToPrune = append(objsToPrune, obj) + } + } + + return objsToPrune, nil + } +} From 230e74e5b2422fe95863c28117c52bbeef99915c Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Mon, 25 Apr 2022 11:13:10 -0400 Subject: [PATCH 09/12] update sorting to save some CPU cycles Signed-off-by: Bryce Palmer --- prune/strategies.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prune/strategies.go b/prune/strategies.go index 0e25890..79f65be 100644 --- a/prune/strategies.go +++ b/prune/strategies.go @@ -35,7 +35,7 @@ func NewPruneByCountStrategy(count int) StrategyFunc { // sort objects by creation date sortedObjs := objs - sort.SliceStable(sortedObjs, func(i, j int) bool { + sort.Slice(sortedObjs, func(i, j int) bool { iTimestamp := sortedObjs[i].GetCreationTimestamp() jTimestamp := sortedObjs[j].GetCreationTimestamp() return iTimestamp.Before(&jTimestamp) From be7adfb415a270d4b5483d58e3d3a0abfc157934 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Tue, 3 May 2022 12:56:49 -0400 Subject: [PATCH 10/12] remove logger Signed-off-by: Bryce Palmer --- prune/prunables.go | 5 ++--- prune/prune.go | 33 ++------------------------------- prune/prune_test.go | 29 ++++++++++++----------------- prune/registry.go | 5 ++--- 4 files changed, 18 insertions(+), 54 deletions(-) diff --git a/prune/prunables.go b/prune/prunables.go index a01b990..7137448 100644 --- a/prune/prunables.go +++ b/prune/prunables.go @@ -15,7 +15,6 @@ package prune import ( - "github.com/go-logr/logr" "sigs.k8s.io/controller-runtime/pkg/client" batchv1 "k8s.io/api/batch/v1" @@ -25,7 +24,7 @@ import ( // DefaultPodIsPrunable is a default IsPrunableFunc to be used specifically with Pod resources. // It marks a Pod resource as prunable if it's Status.Phase is "Succeeded" // This can be overridden by registering your own IsPrunableFunc via the RegisterIsPrunableFunc method -func DefaultPodIsPrunable(obj client.Object, logger logr.Logger) error { +func DefaultPodIsPrunable(obj client.Object) error { pod := obj.(*corev1.Pod) if pod.Status.Phase != corev1.PodSucceeded { return &Unprunable{ @@ -40,7 +39,7 @@ func DefaultPodIsPrunable(obj client.Object, logger logr.Logger) error { // DefaultJobIsPrunable is a default IsPrunableFunc to be used specifically with Job resources. // It marks a Job resource as prunable if it's Status.CompletionTime value is not `nil`, indicating that the Job has completed // This can be overridden by registering your own IsPrunableFunc via the RegisterIsPrunableFunc method -func DefaultJobIsPrunable(obj client.Object, logger logr.Logger) error { +func DefaultJobIsPrunable(obj client.Object) error { job := obj.(*batchv1.Job) if job.Status.CompletionTime == nil { return &Unprunable{ diff --git a/prune/prune.go b/prune/prune.go index d5b2584..d0e5768 100644 --- a/prune/prune.go +++ b/prune/prune.go @@ -19,14 +19,12 @@ import ( "errors" "fmt" - "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/client" - ctrllog "sigs.k8s.io/controller-runtime/pkg/log" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" @@ -58,9 +56,6 @@ type Pruner struct { // namespace is the namespace to use when looking for resources namespace string - - // logger is the logger to use when running pruning functionality - logger logr.Logger } // Unprunable indicates that it is not allowed to prune a specific object. @@ -80,7 +75,7 @@ type StrategyFunc func(ctx context.Context, objs []client.Object) ([]client.Obje // IsPrunableFunc is a function that checks the data of an object to see whether or not it is safe to prune it. // It should return `nil` if it is safe to prune, `Unprunable` if it is unsafe, or another error. // It should safely assert the object is the expected type, otherwise it might panic. -type IsPrunableFunc func(obj client.Object, logger logr.Logger) error +type IsPrunableFunc func(obj client.Object) error // PrunerOption configures the pruner. type PrunerOption func(p *Pruner) @@ -92,13 +87,6 @@ func WithNamespace(namespace string) PrunerOption { } } -// WithLogger can be used to set the Logger field when configuring a Pruner -func WithLogger(logger logr.Logger) PrunerOption { - return func(p *Pruner) { - p.logger = logger - } -} - // WithLabels can be used to set the Labels field when configuring a Pruner func WithLabels(labels map[string]string) PrunerOption { return func(p *Pruner) { @@ -131,7 +119,6 @@ func NewPruner(prunerClient client.Client, gvk schema.GroupVersionKind, strategy pruner := Pruner{ registry: defaultRegistry, client: prunerClient, - logger: Logger(context.Background(), Pruner{}), gvk: gvk, strategy: strategy, } @@ -146,7 +133,6 @@ func NewPruner(prunerClient client.Client, gvk schema.GroupVersionKind, strategy // Prune runs the pruner. func (p Pruner) Prune(ctx context.Context) ([]client.Object, error) { var objs []client.Object - p.logger.Info("Starting the pruning process...") listOpts := client.ListOptions{ LabelSelector: labels.Set(p.labels).AsSelector(), Namespace: p.namespace, @@ -164,8 +150,7 @@ func (p Pruner) Prune(ctx context.Context) ([]client.Object, error) { return nil, err } - if err := p.registry.IsPrunable(obj, p.logger); isUnprunable(err) { - p.logger.Info(fmt.Errorf("object is unprunable: %w", err).Error()) + if err := p.registry.IsPrunable(obj); isUnprunable(err) { continue } else if err != nil { return nil, err @@ -189,20 +174,6 @@ func (p Pruner) Prune(ctx context.Context) ([]client.Object, error) { return objsToPrune, 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, pruner Pruner, keysAndValues ...interface{}) logr.Logger { - var log logr.Logger - if pruner.logger != (logr.Logger{}) { - log = pruner.logger - } else { - log = ctrllog.FromContext(ctx) - } - return log.WithValues(keysAndValues...) -} - func isUnprunable(target error) bool { var unprunable *Unprunable return errors.As(target, &unprunable) diff --git a/prune/prune_test.go b/prune/prune_test.go index 8a8776d..cf31846 100644 --- a/prune/prune_test.go +++ b/prune/prune_test.go @@ -20,7 +20,6 @@ import ( "fmt" "time" - "github.com/go-logr/logr" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" @@ -98,7 +97,7 @@ var _ = Describe("Prune", func() { Kind: "NotReal", }) - Expect(NewRegistry().IsPrunable(obj, logr.Logger{})).Should(BeNil()) + Expect(NewRegistry().IsPrunable(obj)).Should(BeNil()) }) }) @@ -114,20 +113,16 @@ var _ = Describe("Prune", func() { It("Should Return a New Pruner Object with Custom Configuration", func() { namespace := "namespace" labels := map[string]string{"app": "churro"} - logger := &logr.Logger{} pruner, err := NewPruner(fakeClient, jobGVK, myStrategy, WithNamespace(namespace), - WithLabels(labels), - WithLogger(*logger)) - + WithLabels(labels)) Expect(err).Should(BeNil()) Expect(pruner).ShouldNot(BeNil()) Expect(&pruner.registry).Should(Equal(DefaultRegistry())) Expect(pruner.namespace).Should(Equal(namespace)) Expect(pruner.labels).Should(Equal(labels)) - Expect(&pruner.logger).Should(Equal(logger)) Expect(pruner.strategy).ShouldNot(BeNil()) Expect(pruner.gvk).Should(Equal(jobGVK)) Expect(pruner.client).Should(Equal(fakeClient)) @@ -266,7 +261,7 @@ var _ = Describe("Prune", func() { Expect(pruner).ShouldNot(BeNil()) // IsPrunableFunc that throws Unprunable error - errorPrunableFunc := func(obj client.Object, logger logr.Logger) error { + errorPrunableFunc := func(obj client.Object) error { return &Unprunable{ Obj: &obj, Reason: "TEST", @@ -305,7 +300,7 @@ var _ = Describe("Prune", func() { Expect(pruner).ShouldNot(BeNil()) // IsPrunableFunc that throws non Unprunable error - errorPrunableFunc := func(obj client.Object, logger logr.Logger) error { + errorPrunableFunc := func(obj client.Object) error { return fmt.Errorf("TEST") } @@ -376,7 +371,7 @@ var _ = Describe("Prune", func() { // IsPrunableFunc that returns nil but also deletes the object // so that it will throw an error when attempting to remove the object - prunableFunc := func(obj client.Object, logger logr.Logger) error { + prunableFunc := func(obj client.Object) error { _ = fakeClient.Delete(context.TODO(), obj, &client.DeleteOptions{}) return nil } @@ -442,7 +437,7 @@ var _ = Describe("Prune", func() { pod.SetGroupVersionKind(podGVK) // Run it through DefaultPodIsPrunable - err := DefaultPodIsPrunable(pod, logr.Logger{}) + err := DefaultPodIsPrunable(pod) Expect(err).Should(BeNil()) }) @@ -453,7 +448,7 @@ var _ = Describe("Prune", func() { defer expectPanic() // Run it through DefaultPodIsPrunable - _ = DefaultPodIsPrunable(notPod, logr.Logger{}) + _ = DefaultPodIsPrunable(notPod) }) It("Should Return An Error When Kind Is 'Pod' But Phase Is Not 'Succeeded'", func() { @@ -471,7 +466,7 @@ var _ = Describe("Prune", func() { pod.SetGroupVersionKind(podGVK) // Run it through DefaultPodIsPrunable - err := DefaultPodIsPrunable(pod, logr.Logger{}) + err := DefaultPodIsPrunable(pod) Expect(err).ShouldNot(BeNil()) var expectErr *Unprunable Expect(errors.As(err, &expectErr)).Should(BeTrue()) @@ -497,7 +492,7 @@ var _ = Describe("Prune", func() { job.SetGroupVersionKind(jobGVK) // Run it through DefaultJobIsPrunable - err := DefaultJobIsPrunable(job, logr.Logger{}) + err := DefaultJobIsPrunable(job) Expect(err).Should(BeNil()) }) @@ -508,7 +503,7 @@ var _ = Describe("Prune", func() { defer expectPanic() // Run it through DefaultJobIsPrunable - _ = DefaultJobIsPrunable(notJob, logr.Logger{}) + _ = DefaultJobIsPrunable(notJob) }) It("Should Return An Error When Kind Is 'Job' But 'CompletionTime' is 'nil'", func() { @@ -526,7 +521,7 @@ var _ = Describe("Prune", func() { job.SetGroupVersionKind(jobGVK) // Run it through DefaultJobIsPrunable - err := DefaultJobIsPrunable(job, logr.Logger{}) + err := DefaultJobIsPrunable(job) Expect(err).ShouldNot(BeNil()) var expectErr *Unprunable Expect(errors.As(err, &expectErr)).Should(BeTrue()) @@ -732,6 +727,6 @@ func expectPanic() { // myIsPrunable shows how you can write your own IsPrunableFunc // In this example it simply removes all resources -func myIsPrunable(obj client.Object, logger logr.Logger) error { +func myIsPrunable(obj client.Object) error { return nil } diff --git a/prune/registry.go b/prune/registry.go index 0a20ebf..3ce4c18 100644 --- a/prune/registry.go +++ b/prune/registry.go @@ -15,7 +15,6 @@ package prune import ( - "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -48,13 +47,13 @@ func (r *Registry) RegisterIsPrunableFunc(gvk schema.GroupVersionKind, isPrunabl } // IsPrunable checks if an object is prunable -func (r *Registry) IsPrunable(obj client.Object, logger logr.Logger) error { +func (r *Registry) IsPrunable(obj client.Object) error { isPrunable, ok := r.prunables[obj.GetObjectKind().GroupVersionKind()] if !ok { return nil } - return isPrunable(obj, logger) + return isPrunable(obj) } // RegisterIsPrunableFunc registers a function to check whether it is safe to prune a resource of a certain type. From 7fb8237479526529abdc4dfabe4a4cc30ba4028b Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Tue, 3 May 2022 12:58:32 -0400 Subject: [PATCH 11/12] export IsUnprunable Signed-off-by: Bryce Palmer --- prune/prune.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prune/prune.go b/prune/prune.go index d0e5768..b097bea 100644 --- a/prune/prune.go +++ b/prune/prune.go @@ -150,7 +150,7 @@ func (p Pruner) Prune(ctx context.Context) ([]client.Object, error) { return nil, err } - if err := p.registry.IsPrunable(obj); isUnprunable(err) { + if err := p.registry.IsPrunable(obj); IsUnprunable(err) { continue } else if err != nil { return nil, err @@ -174,7 +174,7 @@ func (p Pruner) Prune(ctx context.Context) ([]client.Object, error) { return objsToPrune, nil } -func isUnprunable(target error) bool { +func IsUnprunable(target error) bool { var unprunable *Unprunable return errors.As(target, &unprunable) } From f881c0d3295f4e50df7969dec1e6ef06631d8671 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Tue, 3 May 2022 13:57:49 -0400 Subject: [PATCH 12/12] updates from review Signed-off-by: Bryce Palmer --- prune/prune.go | 8 +++++--- prune/prune_test.go | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/prune/prune.go b/prune/prune.go index b097bea..460a154 100644 --- a/prune/prune.go +++ b/prune/prune.go @@ -110,10 +110,10 @@ func (p Pruner) Namespace() string { } // NewPruner returns a pruner that uses the given strategy to prune objects that have the given GVK -func NewPruner(prunerClient client.Client, gvk schema.GroupVersionKind, strategy StrategyFunc, opts ...PrunerOption) (Pruner, error) { +func NewPruner(prunerClient client.Client, gvk schema.GroupVersionKind, strategy StrategyFunc, opts ...PrunerOption) (*Pruner, error) { if gvk.Empty() { - return Pruner{}, fmt.Errorf("error when creating a new Pruner: gvk parameter can not be empty") + return nil, fmt.Errorf("error when creating a new Pruner: gvk parameter can not be empty") } pruner := Pruner{ @@ -127,7 +127,7 @@ func NewPruner(prunerClient client.Client, gvk schema.GroupVersionKind, strategy opt(&pruner) } - return pruner, nil + return &pruner, nil } // Prune runs the pruner. @@ -174,6 +174,8 @@ func (p Pruner) Prune(ctx context.Context) ([]client.Object, error) { return objsToPrune, nil } +// IsUnprunable checks if a given error is that of Unprunable. +// Returns true if the given error is of type Unprunable, and false if it is not func IsUnprunable(target error) bool { var unprunable *Unprunable return errors.As(target, &unprunable) diff --git a/prune/prune_test.go b/prune/prune_test.go index cf31846..aa464ac 100644 --- a/prune/prune_test.go +++ b/prune/prune_test.go @@ -133,7 +133,7 @@ var _ = Describe("Prune", func() { pruner, err := NewPruner(fakeClient, schema.GroupVersionKind{}, myStrategy) Expect(err).ShouldNot(BeNil()) Expect(err.Error()).Should(Equal("error when creating a new Pruner: gvk parameter can not be empty")) - Expect(pruner).ShouldNot(BeNil()) + Expect(pruner).Should(BeNil()) }) })