From c2f97dd87a4775917f38e58f3b8433fa6063ba9f Mon Sep 17 00:00:00 2001
From: qingliu <qingliu@alauda.io>
Date: Sat, 20 Apr 2024 14:08:50 +0800
Subject: [PATCH] fix: prevent upgrade failures caused by deleting resources

When installing resources in TektonInstallerSet , if the original resource is
in the process of being deleted, it is necessary to wait for the deletion to
complete before proceeding with the installation. Otherwise, it may appear that
the TektonInstallerSets have been installed successfully, but in the end, there
may be missing resources.
---
 .../kubernetes/tektoninstallerset/install.go  | 14 +++++
 .../tektoninstallerset/install_test.go        | 54 +++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/pkg/reconciler/kubernetes/tektoninstallerset/install.go b/pkg/reconciler/kubernetes/tektoninstallerset/install.go
index 6ee00f2b67..7112f5bfda 100644
--- a/pkg/reconciler/kubernetes/tektoninstallerset/install.go
+++ b/pkg/reconciler/kubernetes/tektoninstallerset/install.go
@@ -150,6 +150,13 @@ func (i *installer) ensureResources(resources []unstructured.Unstructured) error
 			return err
 		}
 
+		if res.GetDeletionTimestamp() != nil {
+			err = fmt.Errorf("waiting for resource %s: %s/%s to be deleted",
+				r.GetKind(), r.GetNamespace(), r.GetName())
+			i.logger.Infof("found resource in deletion state, waiting! %v", err)
+			return err
+		}
+
 		i.logger.Infof("found resource %s: %s/%s, checking for update!", r.GetKind(), r.GetNamespace(), r.GetName())
 
 		// if resource exist then check if expected hash is different from the one
@@ -443,6 +450,13 @@ func (i *installer) ensureResource(ctx context.Context, expected *unstructured.U
 		"kind", existing.GetKind(),
 	)
 
+	if existing.GetDeletionTimestamp() != nil {
+		err = fmt.Errorf("waiting for resource %s: %s/%s to be deleted",
+			existing.GetKind(), existing.GetNamespace(), existing.GetName())
+		i.logger.Infof("found resource in deletion state, waiting! %v", err)
+		return err
+	}
+
 	// get list of reconcile fields
 	reconcileFields := i.resourceReconcileFields(expected)
 
diff --git a/pkg/reconciler/kubernetes/tektoninstallerset/install_test.go b/pkg/reconciler/kubernetes/tektoninstallerset/install_test.go
index a011baeb3b..eccbb90048 100644
--- a/pkg/reconciler/kubernetes/tektoninstallerset/install_test.go
+++ b/pkg/reconciler/kubernetes/tektoninstallerset/install_test.go
@@ -19,6 +19,7 @@ package tektoninstallerset
 import (
 	"context"
 	"testing"
+	"time"
 
 	"knative.dev/pkg/ptr"
 
@@ -114,6 +115,33 @@ func TestEnsureResources_UpdateResource(t *testing.T) {
 	assert.Equal(t, res.GetAnnotations()[v1alpha1.LastAppliedHashKey], expectedHash)
 }
 
+func TestEnsureResources_WaitingDeletion(t *testing.T) {
+	k8sClient := k8sfake.NewSimpleClientset()
+	observer, _ := zapobserver.New(zap.InfoLevel)
+	logger := zap.New(observer).Sugar()
+
+	serviceAccount := serviceAccount.DeepCopy()
+	serviceAccount.SetDeletionTimestamp(&metav1.Time{Time: time.Now()})
+
+	var saObjectFromInstallerSet unstructured.Unstructured
+	data, err := runtime.DefaultUnstructuredConverter.ToUnstructured(serviceAccount)
+	assert.NilError(t, err)
+	saObjectFromInstallerSet.Object = data
+	inExist := []unstructured.Unstructured{saObjectFromInstallerSet}
+
+	fakeClient := fake.New([]runtime.Object{serviceAccount}...)
+	manifest, err := mf.ManifestFrom(mf.Slice(inExist), mf.UseClient(fakeClient))
+	if err != nil {
+		t.Fatalf("Failed to generate manifest: %v", err)
+	}
+
+	i := NewInstaller(&manifest, fakeClient, k8sClient, logger)
+
+	// waiting for old resource to be deleted
+	err = i.EnsureNamespaceScopedResources()
+	assert.Error(t, err, `waiting for resource ServiceAccount: test/test-service-account to be deleted`)
+}
+
 var (
 	notReadyStatefulset = &appsv1.StatefulSet{
 		TypeMeta: metav1.TypeMeta{
@@ -817,6 +845,32 @@ func TestEnsureResourceWithHPA(t *testing.T) {
 
 }
 
+func TestEnsureResourceWaitingDeletion(t *testing.T) {
+	ctx := context.TODO()
+	k8sClient := k8sfake.NewSimpleClientset()
+
+	existStatefulset := existStatefulset.DeepCopy()
+	existStatefulset.SetDeletionTimestamp(&metav1.Time{Time: time.Now()})
+
+	var sfsObjectFromInstallerSet unstructured.Unstructured
+	data, err := runtime.DefaultUnstructuredConverter.ToUnstructured(existStatefulset)
+	assert.NilError(t, err)
+	sfsObjectFromInstallerSet.Object = data
+	inExist := []unstructured.Unstructured{sfsObjectFromInstallerSet}
+
+	client := fake.New([]runtime.Object{existStatefulset}...)
+	manifest, err := mf.ManifestFrom(mf.Slice(inExist), mf.UseClient(client))
+	assert.NilError(t, err)
+
+	observer, _ := zapobserver.New(zap.InfoLevel)
+	logger := zap.New(observer).Sugar()
+	i := NewInstaller(&manifest, client, k8sClient, logger)
+
+	// waiting for old statefulset to be deleted
+	err = i.ensureResource(ctx, &sfsObjectFromInstallerSet)
+	assert.Error(t, err, `waiting for resource StatefulSet: test/exist-statefulset to be deleted`)
+}
+
 func getDeployment(name, namespace string, replicas int32) *appsv1.Deployment {
 	return &appsv1.Deployment{
 		TypeMeta: metav1.TypeMeta{