Skip to content

Commit

Permalink
fix: prevent upgrade failures caused by deleting resources
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
l-qing committed Apr 29, 2024
1 parent dbd6775 commit 552172b
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 0 deletions.
14 changes: 14 additions & 0 deletions pkg/reconciler/kubernetes/tektoninstallerset/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
54 changes: 54 additions & 0 deletions pkg/reconciler/kubernetes/tektoninstallerset/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package tektoninstallerset
import (
"context"
"testing"
"time"

"knative.dev/pkg/ptr"

Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit 552172b

Please sign in to comment.