address comments

Author: rushmash91

vertical-pod-autoscaler/pkg/admission-controller/logic/server.go

@@ -36,6 +36,14 @@ import (
 	metrics_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/metrics/admission"
 )
 
+const (
+	vpaGroup               = "autoscaling.k8s.io"
+	vpaResource            = "verticalpodautoscalers"
+	autoDeprecationWarning = `UpdateMode "Auto" is deprecated and will be removed in a future API version. ` +
+		`Use explicit update modes like "Recreate", "Initial", or "InPlaceOrRecreate" instead. ` +
+		`See https://github.com/kubernetes/autoscaler/issues/8424 for more details.`
+)
+
 // AdmissionServer is an admission webhook server that modifies pod resources request based on VPA recommendation
 type AdmissionServer struct {
 	limitsChecker    limitrange.LimitRangeCalculator
@@ -71,27 +79,23 @@ func (s *AdmissionServer) addDeprecationWarnings(req *admissionv1.AdmissionReque
 		Resource: req.Resource.Resource,
 	}
 
-	if admittedGroupResource.Group != "autoscaling.k8s.io" || admittedGroupResource.Resource != "verticalpodautoscalers" {
+	if admittedGroupResource.Group != vpaGroup || admittedGroupResource.Resource != vpaResource {
 		return
 	}
 
 	var vpa vpa_types.VerticalPodAutoscaler
 	if err := json.Unmarshal(req.Object.Raw, &vpa); err != nil {
-		// If we can't unmarshal, skip warning
+		klog.V(4).InfoS("Failed to unmarshal VPA object for deprecation warning check", "err", err)
 		return
 	}
 
 	if vpa.Spec.UpdatePolicy != nil && vpa.Spec.UpdatePolicy.UpdateMode != nil &&
 		*vpa.Spec.UpdatePolicy.UpdateMode == vpa_types.UpdateModeAuto {
 
-		warning := `UpdateMode "Auto" is deprecated and will be removed in a future API version. ` +
-			`Use explicit update modes like "Recreate", "Initial", or "InPlaceOrRecreate" instead. ` +
-			`See https://github.com/kubernetes/autoscaler/issues/8424 for more details.`
-
 		if resp.Warnings == nil {
 			resp.Warnings = []string{}
 		}
-		resp.Warnings = append(resp.Warnings, warning)
+		resp.Warnings = append(resp.Warnings, autoDeprecationWarning)
 	}
 }
 

vertical-pod-autoscaler/pkg/updater/logic/updater.go

@@ -56,8 +56,8 @@ func logDeprecationWarnings(vpa *vpa_types.VerticalPodAutoscaler) {
 	if vpa.Spec.UpdatePolicy != nil && vpa.Spec.UpdatePolicy.UpdateMode != nil &&
 		*vpa.Spec.UpdatePolicy.UpdateMode == vpa_types.UpdateModeAuto {
 
-		klog.Warningf("VPA %s/%s uses deprecated UpdateMode 'Auto'. This mode is deprecated and will be removed in a future API version. Please use explicit update modes like 'Recreate', 'Initial', or 'InPlaceOrRecreate'. See https://github.com/kubernetes/autoscaler/issues/8424",
-			vpa.Namespace, vpa.Name)
+		klog.InfoS("VPA uses deprecated UpdateMode 'Auto'. This mode is deprecated and will be removed in a future API version. Please use explicit update modes like 'Recreate', 'Initial', or 'InPlaceOrRecreate'",
+			"vpa", klog.KObj(vpa), "issue", "https://github.com/kubernetes/autoscaler/issues/8424")
 	}
 }
 

vertical-pod-autoscaler/pkg/updater/logic/updater_test.go

@@ -507,41 +507,62 @@ func TestNewEventRecorder(t *testing.T) {
 
 func TestLogDeprecationWarnings(t *testing.T) {
 	tests := []struct {
-		name       string
-		updateMode *vpa_types.UpdateMode
+		name             string
+		updateMode       *vpa_types.UpdateMode
+		updatePolicy     *vpa_types.PodUpdatePolicy
+		shouldLogWarning bool
 	}{
 		{
-			name:       "Auto mode logs deprecation warning",
-			updateMode: &[]vpa_types.UpdateMode{vpa_types.UpdateModeAuto}[0],
+			name:             "Auto mode should trigger deprecation warning logic",
+			updateMode:       &[]vpa_types.UpdateMode{vpa_types.UpdateModeAuto}[0],
+			shouldLogWarning: true,
 		},
 		{
-			name:       "Recreate mode does not log warning",
-			updateMode: &[]vpa_types.UpdateMode{vpa_types.UpdateModeRecreate}[0],
+			name:             "Recreate mode should not trigger warning logic",
+			updateMode:       &[]vpa_types.UpdateMode{vpa_types.UpdateModeRecreate}[0],
+			shouldLogWarning: false,
 		},
 		{
-			name:       "Initial mode does not log warning",
-			updateMode: &[]vpa_types.UpdateMode{vpa_types.UpdateModeInitial}[0],
+			name:             "Initial mode should not trigger warning logic",
+			updateMode:       &[]vpa_types.UpdateMode{vpa_types.UpdateModeInitial}[0],
+			shouldLogWarning: false,
 		},
 		{
-			name:       "nil update mode does not log warning",
-			updateMode: nil,
+			name:             "nil update policy should not trigger warning logic",
+			updatePolicy:     nil,
+			shouldLogWarning: false,
 		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
+			var updatePolicy *vpa_types.PodUpdatePolicy
+			if tc.updatePolicy != nil {
+				updatePolicy = tc.updatePolicy
+			} else if tc.updateMode != nil {
+				updatePolicy = &vpa_types.PodUpdatePolicy{
+					UpdateMode: tc.updateMode,
+				}
+			}
+
 			vpa := &vpa_types.VerticalPodAutoscaler{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "test-vpa",
 					Namespace: "default",
 				},
 				Spec: vpa_types.VerticalPodAutoscalerSpec{
-					UpdatePolicy: &vpa_types.PodUpdatePolicy{
-						UpdateMode: tc.updateMode,
-					},
+					UpdatePolicy: updatePolicy,
 				},
 			}
 
+			shouldLogWarning := vpa.Spec.UpdatePolicy != nil &&
+				vpa.Spec.UpdatePolicy.UpdateMode != nil &&
+				*vpa.Spec.UpdatePolicy.UpdateMode == vpa_types.UpdateModeAuto
+
+			assert.Equal(t, tc.shouldLogWarning, shouldLogWarning,
+				"Expected shouldLogWarning=%v for test case %s", tc.shouldLogWarning, tc.name)
+
+			// Call the function to ensure it doesn't panic
 			logDeprecationWarnings(vpa)
 		})
 	}