fixup! Add globalPruningGracePeriod flag which defaults to a long tim…

…e for non-breaking opt-in change

Signed-off-by: Max Cao <[email protected]>

vertical-pod-autoscaler/pkg/recommender/main.go

@@ -130,6 +130,11 @@ func main() {
 		klog.Fatalf("--vpa-object-namespace and --ignored-vpa-object-namespaces are mutually exclusive and can't be set together.")
 	}
 
+	err := model.ParseAndInitializePruningGracePeriodDuration()
+	if err != nil {
+		klog.Fatalf("Failed to parse --pruning-grace-period-duration: %v", err)
+	}
+
 	healthCheck := metrics.NewHealthCheck(*metricsFetcherInterval * 5)
 	metrics_recommender.Register()
 	metrics_quality.Register()

vertical-pod-autoscaler/pkg/recommender/model/aggregate_container_state.go

@@ -49,18 +49,22 @@ import (
 
 var (
 	globalPruningGracePeriodDuration = flag.String("pruning-grace-period-duration", "", `The grace period for deleting stale aggregates and recommendations. An empty duration will disable the grace period for all containers by default.`)
-	parsedPruningGracePeriodDuration = parsePruningGracePeriodDuration()
+	parsedPruningGracePeriodDuration *time.Duration
 )
 
-func parsePruningGracePeriodDuration() *time.Duration {
+// ParseAndInitializePruningGracePeriod parses and sets the global pruning grace period duration.
+func ParseAndInitializePruningGracePeriodDuration() error {
 	if globalPruningGracePeriodDuration == nil || *globalPruningGracePeriodDuration == "" {
+		parsedPruningGracePeriodDuration = nil // in tests, global variable does not reset, so nil it explicitly
 		return nil
 	}
 	duration, err := time.ParseDuration(*globalPruningGracePeriodDuration)
 	if err != nil {
-		panic(fmt.Sprintf("Failed to parse --pruning-grace-period-duration: %v", err))
+		parsedPruningGracePeriodDuration = nil
+		return err
 	}
-	return &duration
+	parsedPruningGracePeriodDuration = &duration
+	return nil
 }
 
 // ContainerNameToAggregateStateMap maps a container name to AggregateContainerState

vertical-pod-autoscaler/pkg/recommender/model/aggregate_container_state_test.go

@@ -300,41 +300,49 @@ func TestUpdateFromPolicyControlledResources(t *testing.T) {
 func TestParsePruningGracePeriodDuration(t *testing.T) {
 
 	testCases := []struct {
-		name        string
-		initialFlag string
-		gracePeriod *time.Duration
-		expected    *time.Duration
-		checkPanic  bool
+		name                 string
+		initialFlag          string
+		aggregateGracePeriod *time.Duration
+		expected             *time.Duration
+		expectError          bool
 	}{
 		{
-			name:        "Explicit PruningGracePeriod",
-			initialFlag: "10m",
-			gracePeriod: ptr.To(10 * time.Minute),
-			expected:    ptr.To(10 * time.Minute),
-			checkPanic:  false,
+			name:                 "PruningGracePeriod set from aggregate container state - ignores global flag",
+			initialFlag:          "10m",
+			aggregateGracePeriod: ptr.To(20 * time.Minute),
+			expected:             ptr.To(20 * time.Minute),
+			expectError:          false,
 		}, {
-			name:        "No PruningGracePeriod specified - default to nil",
-			initialFlag: "",
-			gracePeriod: nil,
-			expected:    nil,
-			checkPanic:  false,
+			name:                 "PruningGracePeriod not set from aggregate container state - default to set global flag",
+			initialFlag:          "10m",
+			aggregateGracePeriod: nil,
+			expected:             ptr.To(10 * time.Minute),
+			expectError:          false,
 		}, {
-			name:        "Invalid global PruningGracePeriod - exit with error",
-			initialFlag: "badDuration",
-			gracePeriod: nil,
-			expected:    nil,
-			checkPanic:  true,
+			name:                 "PruningGracePeriod not set from aggregate container state - default to nil global flag",
+			initialFlag:          "",
+			aggregateGracePeriod: nil,
+			expected:             nil,
+			expectError:          false,
+		}, {
+			name:                 "Invalid global PruningGracePeriod flag - error",
+			initialFlag:          "badDuration",
+			aggregateGracePeriod: nil,
+			expected:             nil,
+			expectError:          true,
 		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			flag.Set("pruning-grace-period-duration", tc.initialFlag)
+			err := ParseAndInitializePruningGracePeriodDuration()
 			cs := NewAggregateContainerState()
-			cs.UpdatePruningGracePeriod(tc.gracePeriod)
-			if !tc.checkPanic {
-				assert.Equal(t, tc.expected, parsePruningGracePeriodDuration())
+			cs.UpdatePruningGracePeriod(tc.aggregateGracePeriod)
+			assert.Equal(t, tc.expected, cs.PruningGracePeriod)
+			if tc.expectError {
+				assert.Error(t, err)
 			} else {
-				assert.Panics(t, func() { parsePruningGracePeriodDuration() })
+				assert.NoError(t, err)
 			}
 		})
 	}