fix: handle existing resources gracefully in create operation #2351

When a test exits unexpectedly after creating resources, those resources
aren't cleaned up. When the test is run again, it detects the existing
resources but returns an error instead of cleaning them up, leaving them
stale indefinitely.

This change modifies the tryCreateResource method to not return an error
when a resource already exists, but instead continue with the operation.
This ensures that pre-existing resources are properly cleaned up at the
end of the test.

Fixes #2351

Signed-off-by: Ashish Thirunagari <[email protected]>

Author: ash01ish

pkg/engine/operations/create/operation.go

@@ -2,7 +2,6 @@ package create
 
 import (
 	"context"
-	"errors"
 
 	"github.com/kyverno/chainsaw/pkg/apis"
 	"github.com/kyverno/chainsaw/pkg/apis/v1alpha1"
@@ -101,7 +100,11 @@ func (o *operation) tryCreateResource(ctx context.Context, bindings apis.Binding
 	actual.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind())
 	err := o.client.Get(ctx, client.Key(&obj), &actual)
 	if err == nil {
-		return nil, errors.New("the resource already exists in the cluster")
+		// Add pre-existing resource to cleaner for proper cleanup
+		if o.cleaner != nil {
+			o.cleaner.Add(o.client, &actual)
+		}
+		return o.handleCheck(ctx, bindings, actual, nil)
 	}
 	if kerrors.IsNotFound(err) {
 		return o.createResource(ctx, bindings, obj)

pkg/engine/operations/create/operation_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/kyverno/chainsaw/pkg/mocks"
 	"github.com/stretchr/testify/assert"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/utils/ptr"
 )
@@ -52,9 +53,13 @@ func Test_create(t *testing.T) {
 				*obj.(*unstructured.Unstructured) = pod
 				return nil
 			},
+			DeleteFn: func(ctx context.Context, _ int, obj client.Object, opts ...client.DeleteOption) error {
+				return nil
+			},
 		},
 		expect:      nil,
-		expectedErr: errors.New("the resource already exists in the cluster"),
+		expectedErr: nil,
+		cleaner:     cleaner.New(0*time.Second, nil, metav1.DeletePropagationBackground),
 	}, {
 		name:   "Dry Run Resource already exists",
 		object: pod,
@@ -65,7 +70,8 @@ func Test_create(t *testing.T) {
 			},
 		},
 		expect:      nil,
-		expectedErr: errors.New("the resource already exists in the cluster"),
+		expectedErr: nil,
+		cleaner:     cleaner.New(0*time.Second, nil, metav1.DeletePropagationBackground),
 	}, {
 		name:   "Resource does not exist, create it",
 		object: pod,
@@ -235,6 +241,10 @@ func Test_create(t *testing.T) {
 			} else {
 				assert.NoError(t, err)
 			}
+			if tt.name == "Resource already exists" && tt.cleaner != nil {
+				assert.False(t, tt.cleaner.Empty(), "Cleaner should contain the pre-existing resource for cleanup")
+				assert.NotNil(t, tt.client.DeleteFn, "DeleteFn should be set for this test case")
+			}
 		})
 	}
 }