diff --git a/pkg/vpa/vpa.go b/pkg/vpa/vpa.go index bcc7f2c8..e5d482b2 100644 --- 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 index 6f0b8580..9541e791 100644 --- 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") -}