Merge pull request #217 from openshift-cherrypick-robot/cherry-pick-2…

…15-to-release-4.8

[release-4.8] Bug 1991501: Do not recreate CA deployment when CA CR is being deleted

pkg/controller/clusterautoscaler/clusterautoscaler_controller.go

@@ -180,7 +180,7 @@ func (r *Reconciler) Reconcile(_ context.Context, request reconcile.Request) (re
 		return reconcile.Result{}, err
 	}
 
-	_, err = r.GetAutoscaler(ca)
+	existingDeployment, err := r.GetAutoscaler(ca)
 	if err != nil && !errors.IsNotFound(err) {
 		errMsg := fmt.Sprintf("Error getting cluster-autoscaler deployment: %v", err)
 		r.recorder.Event(caRef, corev1.EventTypeWarning, "FailedGetDeployment", errMsg)
@@ -189,6 +189,18 @@ func (r *Reconciler) Reconcile(_ context.Context, request reconcile.Request) (re
 		return reconcile.Result{}, err
 	}
 
+	// Make sure not to create a new deployment when the CA is being removed.
+	if ca.GetDeletionTimestamp() != nil {
+		if !errors.IsNotFound(err) {
+			// We've already checked for other errors, so this means there was no error, ie the deployment exists.
+			// Remove the deployment if it still exists (GC may have beaten us to this).
+			if err := r.client.Delete(context.TODO(), existingDeployment); err != nil && !errors.IsNotFound(err) {
+				return reconcile.Result{}, err
+			}
+		}
+		return reconcile.Result{}, nil
+	}
+
 	if err := r.ensureAutoscalerMonitoring(ca); err != nil {
 		errMsg := fmt.Sprintf("Error ensuring ClusterAutoscaler monitoring: %v", err)
 		r.recorder.Event(caRef, corev1.EventTypeWarning, "FailedCreate", errMsg)

pkg/controller/clusterautoscaler/clusterautoscaler_test.go

@@ -15,12 +15,15 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/pointer"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
@@ -300,6 +303,77 @@ func TestReconcile(t *testing.T) {
 	}
 }
 
+func TestCADeleting(t *testing.T) {
+	ca := NewClusterAutoscaler()
+	now := metav1.Now()
+	ca.DeletionTimestamp = &now
+
+	req := reconcile.Request{
+		NamespacedName: types.NamespacedName{
+			Namespace: TestNamespace,
+			Name:      "test",
+		},
+	}
+	cfg := Config{
+		ReleaseVersion: "test-1",
+		Name:           "test",
+		Namespace:      TestNamespace,
+	}
+
+	dep1 := &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "cluster-autoscaler-test",
+			Namespace: TestNamespace,
+			Annotations: map[string]string{
+				util.ReleaseVersionAnnotation: "test-1",
+			},
+			Generation: 1,
+		},
+		Status: appsv1.DeploymentStatus{
+			ObservedGeneration: 1,
+			UpdatedReplicas:    1,
+			Replicas:           1,
+			AvailableReplicas:  1,
+		},
+	}
+
+	depKey := client.ObjectKey{
+		Namespace: dep1.Namespace,
+		Name:      dep1.Name,
+	}
+
+	testCases := []struct {
+		name               string
+		existingDeployment *appsv1.Deployment
+	}{
+		{
+			name:               "With no existing deployment",
+			existingDeployment: &appsv1.Deployment{},
+		},
+		{
+			name:               "With an existing deployment",
+			existingDeployment: dep1,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			r := newFakeReconciler(ca, tc.existingDeployment)
+			r.SetConfig(cfg)
+			res, err := r.Reconcile(context.TODO(), req)
+			assert.NoError(t, err)
+			assert.Equal(t, res, reconcile.Result{})
+
+			// Ensure that after the reconcile, no deployment exists
+			err = r.client.Get(context.Background(), depKey, &appsv1.Deployment{})
+			assert.Equal(t, err, apierrors.NewNotFound(schema.GroupResource{
+				Group:    "apps",
+				Resource: "deployments",
+			}, dep1.Name))
+		})
+	}
+}
+
 func TestObjectReference(t *testing.T) {
 	testCases := []struct {
 		label     string