From d52107e43771b72d7774d72e99de476f3a31a786 Mon Sep 17 00:00:00 2001 From: Karsten Brusch Date: Thu, 13 Jun 2024 09:57:58 +0200 Subject: [PATCH 1/9] add --ignore-controller-kind flag --- cmd/controller.go | 2 ++ docs/advanced.md | 1 + pkg/vpa/vpa.go | 20 ++++++++++++++++++-- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/cmd/controller.go b/cmd/controller.go index f1273907..79c69aa0 100644 --- a/cmd/controller.go +++ b/cmd/controller.go @@ -28,6 +28,7 @@ import ( var onByDefault bool var includeNamespaces []string +var ignoreControllerKind []string var excludeNamespaces []string var dryRun bool @@ -37,6 +38,7 @@ func init() { controllerCmd.PersistentFlags().BoolVar(&dryRun, "dry-run", false, "If true, don't mutate resources, just list what would have been created.") controllerCmd.PersistentFlags().StringSliceVar(&includeNamespaces, "include-namespaces", []string{}, "Comma delimited list of namespaces to include from recommendations.") controllerCmd.PersistentFlags().StringSliceVar(&excludeNamespaces, "exclude-namespaces", []string{}, "Comma delimited list of namespaces to exclude from recommendations.") + controllerCmd.PersistentFlags().StringSliceVar(&ignoreControllerKind, "ignore-controller-kind", []string{}, "Comma delimited list of controller kinds to ignore from automatic VPA creation.") } var controllerCmd = &cobra.Command{ diff --git a/docs/advanced.md b/docs/advanced.md index 76e2149d..297d6f30 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -56,6 +56,7 @@ You can set the default behavior for VPA creation using some flags. When specifi * `--on-by-default` - create VPAs in all namespaces * `--include-namespaces` - create VPAs in these namespaces, in addition to any that are labeled * `--exclude-namespaces` - when `--on-by-default` is set, exclude this comma-separated list of namespaces +* `--ignore-controller-kind` - comma-separated list of controller kinds to ignore from automatic VPA creation. For example: `--ignore-controller-kind=Job,CronJob` #### Enable Namespaces diff --git a/pkg/vpa/vpa.go b/pkg/vpa/vpa.go index 1206cb61..3cff6dab 100644 --- a/pkg/vpa/vpa.go +++ b/pkg/vpa/vpa.go @@ -51,6 +51,7 @@ type Reconciler struct { DryRun bool IncludeNamespaces []string ExcludeNamespaces []string + ignoreControllerKind []string } type Controller struct { @@ -110,7 +111,7 @@ func (r Reconciler) ReconcileNamespace(namespace *corev1.Namespace) error { return err } - return r.reconcileControllersAndVPAs(namespace, vpas, controllers) + return r.reconcileControllersAndVPAs(namespace, vpas, controllers, r.ignoreControllerKind) } func (r Reconciler) cleanUpManagedVPAsInNamespace(namespace string, vpas []vpav1.VerticalPodAutoscaler) error { @@ -157,7 +158,7 @@ func (r Reconciler) namespaceIsManaged(namespace *corev1.Namespace) bool { return r.OnByDefault } -func (r Reconciler) reconcileControllersAndVPAs(ns *corev1.Namespace, vpas []vpav1.VerticalPodAutoscaler, controllers []Controller) error { +func (r Reconciler) reconcileControllersAndVPAs(ns *corev1.Namespace, vpas []vpav1.VerticalPodAutoscaler, controllers []Controller, ignoreControllerKind []string) error { defaultUpdateMode, _ := vpaUpdateModeForResource(ns) defaultResourcePolicy, _ := vpaResourcePolicyForResource(ns) defaultMinReplicas, _ := vpaMinReplicasForResource(ns) @@ -165,6 +166,11 @@ func (r Reconciler) reconcileControllersAndVPAs(ns *corev1.Namespace, vpas []vpa // these keys will eventually contain the leftover vpas that do not have a matching controller associated vpaHasAssociatedController := map[string]bool{} for _, controller := range controllers { + // Check if the controller kind is in the ignore list + if contains(ignoreControllerKind, controller.Kind) { + continue + } + var cvpa *vpav1.VerticalPodAutoscaler // search for the matching vpa (will have the same name) for idx, vpa := range vpas { @@ -202,6 +208,16 @@ func (r Reconciler) reconcileControllersAndVPAs(ns *corev1.Namespace, vpas []vpa return nil } +// Helper function to check if a string is in a slice, used for checking if a controller kind is in the ignore list +func contains(slice []string, item string) bool { + for _, a := range slice { + if a == item { + return true + } + } + return false +} + func (r Reconciler) reconcileControllerAndVPA(ns *corev1.Namespace, controller Controller, vpa *vpav1.VerticalPodAutoscaler, vpaUpdateMode *vpav1.UpdateMode, vpaResourcePolicy *vpav1.PodResourcePolicy, minReplicas *int32) error { controllerObj := controller.Unstructured.DeepCopyObject() if vpaUpdateModeOverride, explicit := vpaUpdateModeForResource(controllerObj); explicit { From d3a99cf0c9bfaeb3cc142e31b30efd1b84ae3d36 Mon Sep 17 00:00:00 2001 From: Karsten Brusch Date: Thu, 13 Jun 2024 10:02:32 +0200 Subject: [PATCH 2/9] consistent description --- cmd/controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/controller.go b/cmd/controller.go index 79c69aa0..c8f51d64 100644 --- a/cmd/controller.go +++ b/cmd/controller.go @@ -38,7 +38,7 @@ func init() { controllerCmd.PersistentFlags().BoolVar(&dryRun, "dry-run", false, "If true, don't mutate resources, just list what would have been created.") controllerCmd.PersistentFlags().StringSliceVar(&includeNamespaces, "include-namespaces", []string{}, "Comma delimited list of namespaces to include from recommendations.") controllerCmd.PersistentFlags().StringSliceVar(&excludeNamespaces, "exclude-namespaces", []string{}, "Comma delimited list of namespaces to exclude from recommendations.") - controllerCmd.PersistentFlags().StringSliceVar(&ignoreControllerKind, "ignore-controller-kind", []string{}, "Comma delimited list of controller kinds to ignore from automatic VPA creation.") + controllerCmd.PersistentFlags().StringSliceVar(&ignoreControllerKind, "ignore-controller-kind", []string{}, "Comma delimited list of controller kinds to exclude from recommendations.") } var controllerCmd = &cobra.Command{ From 008d3c830f8ecfb90716453fbd34f5b45ae66311 Mon Sep 17 00:00:00 2001 From: Karsten Brusch Date: Thu, 13 Jun 2024 18:03:18 +0200 Subject: [PATCH 3/9] use "github.com/samber/lo" instead of one-off helper function --- go.mod | 2 ++ go.sum | 4 ++++ pkg/vpa/vpa.go | 13 ++----------- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index be03e429..d7345e65 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/gobuffalo/packr/v2 v2.8.3 github.com/google/uuid v1.3.1 github.com/gorilla/mux v1.8.0 + github.com/samber/lo v1.39.0 github.com/spf13/cobra v1.7.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.4 @@ -49,6 +50,7 @@ require ( github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/sirupsen/logrus v1.9.3 // indirect + golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e // indirect golang.org/x/net v0.17.0 // indirect golang.org/x/oauth2 v0.12.0 // indirect golang.org/x/sys v0.13.0 // indirect diff --git a/go.sum b/go.sum index ada17653..20f5367d 100644 --- a/go.sum +++ b/go.sum @@ -281,6 +281,8 @@ github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjR github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= +github.com/samber/lo v1.39.0 h1:4gTz1wUhNYLhFSKl6O+8peW0v2F4BCY034GRpU9WnuA= +github.com/samber/lo v1.39.0/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA= github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc= github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= @@ -353,6 +355,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= +golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e h1:+WEEuIdZHnUeJJmEUjyYC2gfUMj69yZXw17EnHg/otA= +golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e/go.mod h1:Kr81I6Kryrl9sr8s2FK3vxD90NdsKWRuOIl2O4CvYbA= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/pkg/vpa/vpa.go b/pkg/vpa/vpa.go index 3cff6dab..ebbac466 100644 --- a/pkg/vpa/vpa.go +++ b/pkg/vpa/vpa.go @@ -24,6 +24,7 @@ import ( "k8s.io/client-go/util/retry" "k8s.io/klog/v2/klogr" + "github.com/samber/lo" autoscaling "k8s.io/api/autoscaling/v1" @@ -167,7 +168,7 @@ func (r Reconciler) reconcileControllersAndVPAs(ns *corev1.Namespace, vpas []vpa vpaHasAssociatedController := map[string]bool{} for _, controller := range controllers { // Check if the controller kind is in the ignore list - if contains(ignoreControllerKind, controller.Kind) { + if lo.Contains(ignoreControllerKind, controller.Kind) { continue } @@ -208,16 +209,6 @@ func (r Reconciler) reconcileControllersAndVPAs(ns *corev1.Namespace, vpas []vpa return nil } -// Helper function to check if a string is in a slice, used for checking if a controller kind is in the ignore list -func contains(slice []string, item string) bool { - for _, a := range slice { - if a == item { - return true - } - } - return false -} - func (r Reconciler) reconcileControllerAndVPA(ns *corev1.Namespace, controller Controller, vpa *vpav1.VerticalPodAutoscaler, vpaUpdateMode *vpav1.UpdateMode, vpaResourcePolicy *vpav1.PodResourcePolicy, minReplicas *int32) error { controllerObj := controller.Unstructured.DeepCopyObject() if vpaUpdateModeOverride, explicit := vpaUpdateModeForResource(controllerObj); explicit { From ce2480c770ee3316b2acf3dee76c9e849224f4ed Mon Sep 17 00:00:00 2001 From: Karsten Brusch Date: Thu, 13 Jun 2024 18:06:56 +0200 Subject: [PATCH 4/9] add end-2-end test for --ignore-controller-kind --- e2e/tests/20_flags.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/e2e/tests/20_flags.yaml b/e2e/tests/20_flags.yaml index 5f211ec3..d0834af0 100644 --- a/e2e/tests/20_flags.yaml +++ b/e2e/tests/20_flags.yaml @@ -48,3 +48,13 @@ testcases: assertions: - result.code ShouldEqual 0 - result.systemout ShouldEqual "" +- name: Ignore Controller Kind + steps: + - script: | + yq w ../../hack/manifests/controller/deployment.yaml -- spec.template.spec.containers[0].command[2] '--ignore-controller-kind=Deployment' | kubectl -n goldilocks apply -f - + - script: kubectl -n goldilocks wait deployment --timeout={{.timeout}} --for condition=available -l app.kubernetes.io/name=goldilocks,app.kubernetes.io/component=controller + - script: sleep {{.vpa-wait}} + - script: kubectl get verticalpodautoscalers.autoscaling.k8s.io -n demo-excluded -oname + assertions: + - result.code ShouldEqual 0 + - result.systemout ShouldEqual "" From f674432598c3371fb5301a961b8fdb8d5b52632f Mon Sep 17 00:00:00 2001 From: "k11h.de" <17837008+k11h-de@users.noreply.github.com> Date: Fri, 14 Jun 2024 17:40:05 +0200 Subject: [PATCH 5/9] Update pkg/vpa/vpa.go Co-authored-by: Andy Suderman --- pkg/vpa/vpa.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/vpa/vpa.go b/pkg/vpa/vpa.go index ebbac466..ddd335fd 100644 --- a/pkg/vpa/vpa.go +++ b/pkg/vpa/vpa.go @@ -52,7 +52,7 @@ type Reconciler struct { DryRun bool IncludeNamespaces []string ExcludeNamespaces []string - ignoreControllerKind []string + IgnoreControllerKind []string } type Controller struct { From b83274669efd7ea72f4f0637de3fef2135e42bcf Mon Sep 17 00:00:00 2001 From: "k11h.de" <17837008+k11h-de@users.noreply.github.com> Date: Fri, 14 Jun 2024 17:40:23 +0200 Subject: [PATCH 6/9] Update pkg/vpa/vpa.go Co-authored-by: Andy Suderman --- pkg/vpa/vpa.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/vpa/vpa.go b/pkg/vpa/vpa.go index ddd335fd..7e81476d 100644 --- a/pkg/vpa/vpa.go +++ b/pkg/vpa/vpa.go @@ -112,7 +112,7 @@ func (r Reconciler) ReconcileNamespace(namespace *corev1.Namespace) error { return err } - return r.reconcileControllersAndVPAs(namespace, vpas, controllers, r.ignoreControllerKind) + return r.reconcileControllersAndVPAs(namespace, vpas, controllers) } func (r Reconciler) cleanUpManagedVPAsInNamespace(namespace string, vpas []vpav1.VerticalPodAutoscaler) error { From 33eb6322e75ef434198bf0013cb927df7bcc6632 Mon Sep 17 00:00:00 2001 From: "k11h.de" <17837008+k11h-de@users.noreply.github.com> Date: Fri, 14 Jun 2024 17:40:44 +0200 Subject: [PATCH 7/9] Update pkg/vpa/vpa.go Co-authored-by: Andy Suderman --- pkg/vpa/vpa.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/vpa/vpa.go b/pkg/vpa/vpa.go index 7e81476d..9a585fea 100644 --- a/pkg/vpa/vpa.go +++ b/pkg/vpa/vpa.go @@ -159,7 +159,7 @@ func (r Reconciler) namespaceIsManaged(namespace *corev1.Namespace) bool { return r.OnByDefault } -func (r Reconciler) reconcileControllersAndVPAs(ns *corev1.Namespace, vpas []vpav1.VerticalPodAutoscaler, controllers []Controller, ignoreControllerKind []string) error { +func (r Reconciler) reconcileControllersAndVPAs(ns *corev1.Namespace, vpas []vpav1.VerticalPodAutoscaler, controllers []Controller) error { defaultUpdateMode, _ := vpaUpdateModeForResource(ns) defaultResourcePolicy, _ := vpaResourcePolicyForResource(ns) defaultMinReplicas, _ := vpaMinReplicasForResource(ns) From 707d691fc601198bd7722e020ad5008f470c038b Mon Sep 17 00:00:00 2001 From: "k11h.de" <17837008+k11h-de@users.noreply.github.com> Date: Fri, 14 Jun 2024 17:40:54 +0200 Subject: [PATCH 8/9] Update pkg/vpa/vpa.go Co-authored-by: Andy Suderman --- pkg/vpa/vpa.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/vpa/vpa.go b/pkg/vpa/vpa.go index 9a585fea..f2458ba2 100644 --- a/pkg/vpa/vpa.go +++ b/pkg/vpa/vpa.go @@ -168,7 +168,7 @@ func (r Reconciler) reconcileControllersAndVPAs(ns *corev1.Namespace, vpas []vpa vpaHasAssociatedController := map[string]bool{} for _, controller := range controllers { // Check if the controller kind is in the ignore list - if lo.Contains(ignoreControllerKind, controller.Kind) { + if lo.Contains(r.IgnoreControllerKind, controller.Kind) { continue } From 296a4ed47bc3590945ba09a3fc97f3d124e8b27a Mon Sep 17 00:00:00 2001 From: Karsten Brusch Date: Sat, 22 Jun 2024 10:03:30 +0200 Subject: [PATCH 9/9] add `IgnoreControllerKind` to the actual instance of the struct that gets declared on line 49 --- cmd/controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/controller.go b/cmd/controller.go index c8f51d64..ab33019c 100644 --- a/cmd/controller.go +++ b/cmd/controller.go @@ -50,6 +50,7 @@ var controllerCmd = &cobra.Command{ vpaReconciler.OnByDefault = onByDefault vpaReconciler.IncludeNamespaces = includeNamespaces vpaReconciler.ExcludeNamespaces = excludeNamespaces + vpaReconciler.IgnoreControllerKind = ignoreControllerKind klog.V(4).Infof("Starting controller with Reconciler: %+v", vpaReconciler)