Don't attempt to delete aggregation from VPA if it doesn't exist

kubernetes · Aug 5, 2019 · daa0650 · daa0650
1 parent e0cc672
commit daa0650
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 1 deletion.
diff --git a/vertical-pod-autoscaler/pkg/recommender/model/vpa.go b/vertical-pod-autoscaler/pkg/recommender/model/vpa.go
@@ -155,7 +155,10 @@ func (vpa *Vpa) UsesAggregation(aggregationKey AggregateStateKey) bool {
 
 // DeleteAggregation deletes aggregation used by this container
 func (vpa *Vpa) DeleteAggregation(aggregationKey AggregateStateKey) {
-	state := vpa.aggregateContainerStates[aggregationKey]
+	state, ok := vpa.aggregateContainerStates[aggregationKey]
+	if !ok {
+		return
+	}
 	state.MarkNotAutoscaled()
 	delete(vpa.aggregateContainerStates, aggregationKey)
 }

diff --git a/vertical-pod-autoscaler/pkg/recommender/model/vpa_test.go b/vertical-pod-autoscaler/pkg/recommender/model/vpa_test.go
@@ -336,6 +336,50 @@ func TestUseAggregationIfMatching(t *testing.T) {
 	}
 }
 
+func TestDeleteAggregation(t *testing.T) {
+	cases := []struct {
+		name                     string
+		aggregateContainerStates aggregateContainerStatesMap
+		delet                    AggregateStateKey
+	}{
+		{
+			name: "delet dis",
+			aggregateContainerStates: aggregateContainerStatesMap{
+				aggregateStateKey{
+					namespace:     "ns",
+					containerName: "container",
+					labelSetKey:   "labelSetKey",
+					labelSetMap:   nil,
+				}: &AggregateContainerState{},
+			},
+			delet: aggregateStateKey{
+				namespace:     "ns",
+				containerName: "container",
+				labelSetKey:   "labelSetKey",
+				labelSetMap:   nil,
+			},
+		},
+		{
+			name: "no delet",
+			delet: aggregateStateKey{
+				namespace:     "ns",
+				containerName: "container",
+				labelSetKey:   "labelSetKey",
+				labelSetMap:   nil,
+			},
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			vpa := Vpa{
+				aggregateContainerStates: tc.aggregateContainerStates,
+			}
+			vpa.DeleteAggregation(tc.delet)
+			assert.Equal(t, 0, len(vpa.aggregateContainerStates))
+		})
+	}
+}
+
 type mockAggregateStateKey struct {
 	namespace     string
 	containerName string