Skip to content

Commit

Permalink
Merge pull request #2238 from jbartosik/bugfix-00
Browse files Browse the repository at this point in the history
Don't attempt to delete aggregation from VPA if it doesn't exist
  • Loading branch information
k8s-ci-robot authored Aug 5, 2019
2 parents e0cc672 + daa0650 commit 100e91b
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 1 deletion.
5 changes: 4 additions & 1 deletion vertical-pod-autoscaler/pkg/recommender/model/vpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
44 changes: 44 additions & 0 deletions vertical-pod-autoscaler/pkg/recommender/model/vpa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 100e91b

Please sign in to comment.