WIP: Address feedback

GoogleContainerTools · Jul 31, 2023 · 38f0141 · 38f0141
1 parent 646aff1
commit 38f0141
Showing 1 changed file with 13 additions and 12 deletions.
diff --git a/pkg/reconcilermanager/controllers/reconciler_base.go b/pkg/reconcilermanager/controllers/reconciler_base.go
@@ -624,9 +624,12 @@ func (r *reconcilerBase) validateCACertSecret(ctx context.Context, namespace, ca
 
 func (r *reconcilerBase) validateAnnotations(_ context.Context, rs client.Object) error {
 	autoscalingStrategy := reconcilerAutoscalingStrategy(rs)
-	if autoscalingStrategy != metadata.ReconcilerAutoscalingStrategyAuto &&
-		autoscalingStrategy != metadata.ReconcilerAutoscalingStrategyRecommend &&
-		autoscalingStrategy != metadata.ReconcilerAutoscalingStrategyDisabled {
+	switch autoscalingStrategy {
+	case metadata.ReconcilerAutoscalingStrategyAuto,
+		metadata.ReconcilerAutoscalingStrategyRecommend,
+		metadata.ReconcilerAutoscalingStrategyDisabled:
+		// valid
+	default:
 		return errors.Errorf("annotation %q has invalid value %q, must be one of %q, %q, or %q",
 			metadata.ReconcilerAutoscalingStrategyAnnotationKey,
 			autoscalingStrategy,
@@ -739,16 +742,14 @@ func (r *reconcilerBase) upsertVerticalPodAutoscaler(ctx context.Context, strate
 	case metadata.ReconcilerAutoscalingStrategyDisabled:
 		// delete if VPA is installed
 		if !vpaEnabled {
-			r.logger(ctx).Info("Managed object delete skipped - not enabled",
-				logFieldObjectRef, vpaRef.String(),
-				logFieldObjectKind, "VerticalPodAutoscaler")
+			// nothing to delete - CRD not installed
 			return vpaRef, false, nil
 		}
 		return vpaRef, false, r.deleteVerticalPodAutoscaler(ctx, reconcilerRef)
 	case metadata.ReconcilerAutoscalingStrategyAuto, metadata.ReconcilerAutoscalingStrategyRecommend:
 		// upsert if VPA is installed
 		if !vpaEnabled {
-			r.logger(ctx).Info("Managed object upsert skipped - not enabled",
+			r.logger(ctx).Info("Managed object upsert skipped - VerticalPodAutoscaler CRD/APIService not installed",
 				logFieldObjectRef, vpaRef.String(),
 				logFieldObjectKind, "VerticalPodAutoscaler")
 			return vpaRef, false, nil
@@ -757,7 +758,6 @@ func (r *reconcilerBase) upsertVerticalPodAutoscaler(ctx context.Context, strate
 		// shouldn't happen - invalid strategy should be caught by validation
 		return vpaRef, false, errors.Errorf("invalid reconciler autoscaling strategy: %v", strategy)
 	}
-	autoscale := (strategy == metadata.ReconcilerAutoscalingStrategyAuto)
 	vpa := &autoscalingv1.VerticalPodAutoscaler{}
 	vpa.Name = vpaRef.Name
 	vpa.Namespace = vpaRef.Namespace
@@ -769,10 +769,11 @@ func (r *reconcilerBase) upsertVerticalPodAutoscaler(ctx context.Context, strate
 			Name: reconcilerRef.Name,
 		}
 		var updateMode autoscalingv1.UpdateMode
-		if autoscale {
-			updateMode = autoscalingv1.UpdateModeAuto // Evict and Recreate as-needed
-		} else {
-			updateMode = autoscalingv1.UpdateModeOff // Recommend only
+		switch strategy {
+		case metadata.ReconcilerAutoscalingStrategyAuto:
+			updateMode = autoscalingv1.UpdateModeAuto
+		default: // Recommend
+			updateMode = autoscalingv1.UpdateModeOff
 		}
 		vpa.Spec.UpdatePolicy = &autoscalingv1.PodUpdatePolicy{
 			UpdateMode: &updateMode,