Merge pull request #6767 from BojanZelic/vpa-fix-checkpoint-gc

vpa - fix checkpoint gc of unknown recommenders

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 {

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)
+	}
+}