Allow a custom recommender to be set via annotation or label on the n…

…amespace
FairwindsOps · Sep 23, 2024 · 7f0145e · 7f0145e
1 parent 3c7e2f6
commit 7f0145e
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 141 deletions.
diff --git a/pkg/vpa/vpa.go b/pkg/vpa/vpa.go
@@ -22,9 +22,9 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/samber/lo"
 	"k8s.io/client-go/util/retry"
 	"k8s.io/klog/v2/klogr"
+	"github.com/samber/lo"
 
 	autoscaling "k8s.io/api/autoscaling/v1"
 
@@ -237,9 +237,6 @@ func (r Reconciler) reconcileControllerAndVPA(ns *corev1.Namespace, controller C
 				Name: recommenderName,
 			},
 		}
-	} else if vpa != nil {
-		// Preserve existing recommenders if any
-		desiredVPA.Spec.Recommenders = vpa.Spec.Recommenders
 	}
 
 	if vpa == nil {
@@ -367,7 +364,6 @@ func (r Reconciler) getVPAObject(existingVPA *vpav1.VerticalPodAutoscaler, ns *c
 				Name:      "goldilocks-" + controller.Name,
 				Namespace: ns.Name,
 			},
-			Spec: vpav1.VerticalPodAutoscalerSpec{},
 		}
 	} else {
 		// or use the existing VPA as a template to update from
@@ -377,33 +373,25 @@ func (r Reconciler) getVPAObject(existingVPA *vpav1.VerticalPodAutoscaler, ns *c
 	// update the labels on the VPA
 	desiredVPA.Labels = utils.VPALabels
 
-	// Update or set the managed fields in Spec
-	if desiredVPA.Spec.TargetRef == nil {
-		desiredVPA.Spec.TargetRef = &autoscaling.CrossVersionObjectReference{}
-	}
-	desiredVPA.Spec.TargetRef.APIVersion = controller.APIVersion
-	desiredVPA.Spec.TargetRef.Kind = controller.Kind
-	desiredVPA.Spec.TargetRef.Name = controller.Name
-
-	if desiredVPA.Spec.UpdatePolicy == nil {
-		desiredVPA.Spec.UpdatePolicy = &vpav1.PodUpdatePolicy{}
-	}
-	desiredVPA.Spec.UpdatePolicy.UpdateMode = updateMode
-
-	if resourcePolicy != nil {
-		desiredVPA.Spec.ResourcePolicy = resourcePolicy
-	} else if existingVPA != nil {
-		// Preserve existing ResourcePolicy if not overridden
-		desiredVPA.Spec.ResourcePolicy = existingVPA.Spec.ResourcePolicy
-	}
-
-	if minReplicas != nil && *minReplicas > 0 {
-		desiredVPA.Spec.UpdatePolicy.MinReplicas = minReplicas
+	// update the spec on the VPA
+	desiredVPA.Spec = vpav1.VerticalPodAutoscalerSpec{
+		TargetRef: &autoscaling.CrossVersionObjectReference{
+			APIVersion: controller.APIVersion,
+			Kind:       controller.Kind,
+			Name:       controller.Name,
+		},
+		UpdatePolicy: &vpav1.PodUpdatePolicy{
+			UpdateMode: updateMode,
+		},
+		ResourcePolicy: resourcePolicy,
+	}
+
+	if minReplicas != nil {
+		if *minReplicas > 0 {
+			desiredVPA.Spec.UpdatePolicy.MinReplicas = minReplicas
+		}
 	}
 
-	// No change to desiredVPA.Spec.Recommenders
-	// It will be preserved from existingVPA if present
-
 	return desiredVPA
 }
 
@@ -486,9 +474,12 @@ func vpaMinReplicasForResource(obj runtime.Object) (*int32, bool) {
 }
 
 func vpaRecommenderNameForNamespace(ns *corev1.Namespace) (string, error) {
-	recommenderName, ok := ns.Labels["goldilocks.fairwinds.com/vpa-recommenders"]
-	if !ok || recommenderName == "" {
-		return "", nil
+	recommenderName := ""
+	if val, ok := ns.Labels["goldilocks.fairwinds.com/vpa-recommenders"]; ok {
+		recommenderName = val
+	} else if val, ok := ns.Annotations["goldilocks.fairwinds.com/vpa-recommenders"]; ok {
+		recommenderName = val
 	}
+
 	return recommenderName, nil
 }
diff --git a/pkg/vpa/vpa_test.go b/pkg/vpa/vpa_test.go
@@ -16,7 +16,6 @@ package vpa
 
 import (
 	"context"
-	autoscaling "k8s.io/api/autoscaling/v1"
 	"strings"
 	"testing"
 
@@ -762,110 +761,3 @@ func Test_ReconcileNamespaceStatefulSet(t *testing.T) {
 	assert.Equal(t, 1, len(vpaList.Items))
 	assert.Equal(t, "goldilocks-test-sts", vpaList.Items[0].ObjectMeta.Name)
 }
-
-func Test_ReconcileVPAWithCustomRecommenders(t *testing.T) {
-	setupVPAForTests(t)
-	rec := GetInstance()
-	VPAClient := rec.VPAClient
-	DynamicClient := rec.DynamicClient.Client
-
-	// Use the existing namespace nsLabeledTrue and its unstructured version
-	nsName := nsLabeledTrue.ObjectMeta.Name
-	nsUnstructured := nsLabeledTrueUnstructured
-
-	// Create the namespace in the dynamic client
-	_, err := DynamicClient.Resource(schema.GroupVersionResource{
-		Group:    "",
-		Version:  "v1",
-		Resource: "namespaces",
-	}).Create(context.TODO(), nsUnstructured, metav1.CreateOptions{})
-	assert.NoError(t, err)
-
-	// Create a deployment in the namespace
-	deploymentName := "test-deployment"
-	testDeployment := &unstructured.Unstructured{
-		Object: map[string]interface{}{
-			"apiVersion": "apps/v1",
-			"kind":       "Deployment",
-			"metadata": map[string]interface{}{
-				"name":      deploymentName,
-				"namespace": nsName,
-			},
-			"spec": map[string]interface{}{
-				"replicas": int64(1),
-				"selector": map[string]interface{}{
-					"matchLabels": map[string]interface{}{
-						"app": "test",
-					},
-				},
-				"template": map[string]interface{}{
-					"metadata": map[string]interface{}{
-						"labels": map[string]interface{}{
-							"app": "test",
-						},
-					},
-					"spec": map[string]interface{}{
-						"containers": []interface{}{
-							map[string]interface{}{
-								"name":  "test-container",
-								"image": "nginx",
-							},
-						},
-					},
-				},
-			},
-		},
-	}
-
-	// Create the deployment in the dynamic client
-	_, err = DynamicClient.Resource(schema.GroupVersionResource{
-		Group:    "apps",
-		Version:  "v1",
-		Resource: "deployments",
-	}).Namespace(nsName).Create(context.TODO(), testDeployment, metav1.CreateOptions{})
-	assert.NoError(t, err)
-
-	// Manually create a VPA with a custom Recommenders field
-	customRecommenders := []*vpav1.VerticalPodAutoscalerRecommenderSelector{
-		{
-			Name: "my-custom-recommender",
-		},
-	}
-	vpaName := "goldilocks-" + deploymentName
-	updateModeOff := vpav1.UpdateModeOff
-	initialVPA := &vpav1.VerticalPodAutoscaler{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      vpaName,
-			Namespace: nsName,
-			Labels:    utils.VPALabels,
-		},
-		Spec: vpav1.VerticalPodAutoscalerSpec{
-			TargetRef: &autoscaling.CrossVersionObjectReference{
-				APIVersion: "apps/v1",
-				Kind:       "Deployment",
-				Name:       deploymentName,
-			},
-			UpdatePolicy: &vpav1.PodUpdatePolicy{
-				UpdateMode: &updateModeOff,
-			},
-			Recommenders: customRecommenders,
-		},
-	}
-
-	// Create the VPA using the VPA client
-	_, err = VPAClient.Client.AutoscalingV1().VerticalPodAutoscalers(nsName).
-		Create(context.TODO(), initialVPA, metav1.CreateOptions{})
-	assert.NoError(t, err)
-
-	// Run the reconciliation process
-	err = rec.ReconcileNamespace(&nsLabeledTrue)
-	assert.NoError(t, err)
-
-	// Retrieve the VPA after reconciliation
-	reconciledVPA, err := VPAClient.Client.AutoscalingV1().VerticalPodAutoscalers(nsName).
-		Get(context.TODO(), vpaName, metav1.GetOptions{})
-	assert.NoError(t, err)
-
-	// Assert that the Recommenders field is preserved
-	assert.Equal(t, initialVPA.Spec.Recommenders, reconciledVPA.Spec.Recommenders, "Recommenders field should be preserved after reconciliation")
-}