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 20, 2024
1 parent e2b6806 commit c2f97dd
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 c2f97dd

Please sign in to comment.