diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go index d41ac0db4a..2941bd3ec1 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go @@ -276,7 +276,21 @@ func (feeder *clusterStateFeeder) InitFromCheckpoints() { func (feeder *clusterStateFeeder) GarbageCollectCheckpoints() { klog.V(3).InfoS("Starting garbage collection of checkpoints") - feeder.LoadVPAs(context.TODO()) + + allVPAKeys := map[model.VpaID]bool{} + + allVpaResources, err := feeder.vpaLister.List(labels.Everything()) + if err != nil { + klog.ErrorS(err, "Cannot list VPAs") + return + } + for _, vpa := range allVpaResources { + vpaID := model.VpaID{ + Namespace: vpa.Namespace, + VpaName: vpa.Name, + } + allVPAKeys[vpaID] = true + } namespaceList, err := feeder.coreClient.Namespaces().List(context.TODO(), metav1.ListOptions{}) if err != nil { @@ -292,7 +306,8 @@ func (feeder *clusterStateFeeder) GarbageCollectCheckpoints() { } for _, checkpoint := range checkpointList.Items { vpaID := model.VpaID{Namespace: checkpoint.Namespace, VpaName: checkpoint.Spec.VPAObjectName} - _, exists := feeder.clusterState.Vpas[vpaID] + exists := allVPAKeys[vpaID] + if !exists { err = feeder.vpaCheckpointClient.VerticalPodAutoscalerCheckpoints(namespace).Delete(context.TODO(), checkpoint.Name, metav1.DeleteOptions{}) if err == nil { diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go index 6ed5ea0983..85559ad335 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go @@ -24,12 +24,17 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + core "k8s.io/client-go/testing" autoscalingv1 "k8s.io/api/autoscaling/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + fakeautoscalingv1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned/typed/autoscaling.k8s.io/v1/fake" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/input/history" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/input/spec" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/model" @@ -632,3 +637,80 @@ func TestFilterVPAsIgnoreNamespaces(t *testing.T) { assert.ElementsMatch(t, expectedResult, result) } + +func TestCanCleanupCheckpoints(t *testing.T) { + client := fake.NewSimpleClientset() + + _, err := client.CoreV1().Namespaces().Create(context.TODO(), &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "testNamespace"}}, metav1.CreateOptions{}) + assert.NoError(t, err) + + vpaBuilder := test.VerticalPodAutoscaler().WithContainer("container").WithNamespace("testNamespace").WithTargetRef(&autoscalingv1.CrossVersionObjectReference{ + Kind: kind, + Name: name1, + APIVersion: apiVersion, + }) + + balanced := vpaBuilder.WithRecommender("balanced").WithName("balanced").Get() + performance := vpaBuilder.WithRecommender("performance").WithName("performance").Get() + savings := vpaBuilder.WithRecommender("savings").WithName("savings").Get() + defaultVpa := vpaBuilder.WithRecommender("default").WithName("default").Get() + + vpas := []*vpa_types.VerticalPodAutoscaler{balanced, performance, savings, defaultVpa} + vpaLister := &test.VerticalPodAutoscalerListerMock{} + vpaLister.On("List").Return(vpas, nil) + + checkpoints := &vpa_types.VerticalPodAutoscalerCheckpointList{ + Items: []vpa_types.VerticalPodAutoscalerCheckpoint{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "testNamespace", + Name: "nonExistentVPA", + }, + Spec: vpa_types.VerticalPodAutoscalerCheckpointSpec{ + VPAObjectName: "nonExistentVPA", + }, + }, + }, + } + + for _, vpa := range vpas { + checkpoints.Items = append(checkpoints.Items, vpa_types.VerticalPodAutoscalerCheckpoint{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: vpa.Namespace, + Name: vpa.Name, + }, + Spec: vpa_types.VerticalPodAutoscalerCheckpointSpec{ + VPAObjectName: vpa.Name, + }, + }) + } + + checkpointClient := &fakeautoscalingv1.FakeAutoscalingV1{Fake: &core.Fake{}} + checkpointClient.Fake.AddReactor("list", "verticalpodautoscalercheckpoints", func(action core.Action) (bool, runtime.Object, error) { + return true, checkpoints, nil + }) + + deletedCheckpoints := []string{} + checkpointClient.Fake.AddReactor("delete", "verticalpodautoscalercheckpoints", func(action core.Action) (bool, runtime.Object, error) { + deleteAction := action.(core.DeleteAction) + deletedCheckpoints = append(deletedCheckpoints, deleteAction.GetName()) + + return true, nil, nil + }) + + feeder := clusterStateFeeder{ + coreClient: client.CoreV1(), + vpaLister: vpaLister, + vpaCheckpointClient: checkpointClient, + clusterState: &model.ClusterState{}, + recommenderName: "default", + } + + feeder.GarbageCollectCheckpoints() + + assert.Contains(t, deletedCheckpoints, "nonExistentVPA") + + for _, vpa := range vpas { + assert.NotContains(t, deletedCheckpoints, vpa.Name) + } +}