From daa0650504d694482e865b6bb6eca7a2376a738b Mon Sep 17 00:00:00 2001 From: Joachim Bartosik Date: Mon, 5 Aug 2019 14:29:36 +0200 Subject: [PATCH] Don't attempt to delete aggregation from VPA if it doesn't exist --- .../pkg/recommender/model/vpa.go | 5 ++- .../pkg/recommender/model/vpa_test.go | 44 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/model/vpa.go b/vertical-pod-autoscaler/pkg/recommender/model/vpa.go index 663415efbb6f..defcfb5a98fc 100644 --- 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 index 455d70bbbaa6..2cf3df624710 100644 --- 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