Merge pull request #937 from fluxcd/delete-suspended

Allow deleting suspended objects

controllers/bucket_controller.go

@@ -258,12 +258,6 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
 	// Record suspended status metric
 	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 
-	// Return early if the object is suspended
-	if obj.Spec.Suspend {
-		log.Info("reconciliation is suspended for this object")
-		return ctrl.Result{}, nil
-	}
-
 	// Initialize the patch helper with the current version of the object.
 	patchHelper, err := patch.NewHelper(obj, r.Client)
 	if err != nil {
@@ -309,6 +303,13 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
 		return
 	}
 
+	// Return if the object is suspended.
+	if obj.Spec.Suspend {
+		log.Info("reconciliation is suspended for this object")
+		recResult, retErr = sreconcile.ResultEmpty, nil
+		return
+	}
+
 	// Reconcile actual object
 	reconcilers := []bucketReconcileFunc{
 		r.reconcileStorage,

controllers/bucket_controller_test.go

@@ -30,7 +30,6 @@ import (
 
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/record"
 	kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status"
@@ -85,7 +84,7 @@ func TestBucketReconciler_Reconcile(t *testing.T) {
 	g.Expect(testEnv.Create(ctx, secret)).To(Succeed())
 	defer testEnv.Delete(ctx, secret)
 
-	obj := &sourcev1.Bucket{
+	origObj := &sourcev1.Bucket{
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "bucket-reconcile-",
 			Namespace:    "default",
@@ -102,6 +101,7 @@ func TestBucketReconciler_Reconcile(t *testing.T) {
 			},
 		},
 	}
+	obj := origObj.DeepCopy()
 	g.Expect(testEnv.Create(ctx, obj)).To(Succeed())
 
 	key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}
@@ -115,17 +115,7 @@ func TestBucketReconciler_Reconcile(t *testing.T) {
 	}, timeout).Should(BeTrue())
 
 	// Wait for Bucket to be Ready
-	g.Eventually(func() bool {
-		if err := testEnv.Get(ctx, key, obj); err != nil {
-			return false
-		}
-		if !conditions.IsReady(obj) || obj.Status.Artifact == nil {
-			return false
-		}
-		readyCondition := conditions.Get(obj, meta.ReadyCondition)
-		return obj.Generation == readyCondition.ObservedGeneration &&
-			obj.Generation == obj.Status.ObservedGeneration
-	}, timeout).Should(BeTrue())
+	waitForSourceReadyWithArtifact(ctx, g, obj)
 
 	// Check if the object status is valid.
 	condns := &conditionscheck.Conditions{NegativePolarity: bucketReadyCondition.NegativePolarity}
@@ -157,12 +147,11 @@ func TestBucketReconciler_Reconcile(t *testing.T) {
 	g.Expect(testEnv.Delete(ctx, obj)).To(Succeed())
 
 	// Wait for Bucket to be deleted
-	g.Eventually(func() bool {
-		if err := testEnv.Get(ctx, key, obj); err != nil {
-			return apierrors.IsNotFound(err)
-		}
-		return false
-	}, timeout).Should(BeTrue())
+	waitForSourceDeletion(ctx, g, obj)
+
+	// Check if a suspended object gets deleted.
+	obj = origObj.DeepCopy()
+	testSuspendedObjectDeleteWithArtifact(ctx, g, obj)
 }
 
 func TestBucketReconciler_reconcileStorage(t *testing.T) {

controllers/common_test.go

@@ -0,0 +1,146 @@
+/*
+Copyright 2022 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controllers
+
+import (
+	"context"
+
+	. "github.com/onsi/gomega"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/fluxcd/pkg/apis/meta"
+	"github.com/fluxcd/pkg/runtime/conditions"
+	"github.com/fluxcd/pkg/runtime/patch"
+
+	"github.com/fluxcd/source-controller/internal/object"
+)
+
+// waitForSourceDeletion is a generic test helper to wait for object deletion of
+// any source kind.
+func waitForSourceDeletion(ctx context.Context, g *WithT, obj conditions.Setter) {
+	g.THelper()
+
+	key := client.ObjectKeyFromObject(obj)
+	g.Eventually(func() bool {
+		if err := testEnv.Get(ctx, key, obj); err != nil {
+			return apierrors.IsNotFound(err)
+		}
+		return false
+	}, timeout).Should(BeTrue())
+}
+
+// waitForSuspended is a generic test helper to wait for object to be suspended
+// of any source kind.
+func waitForSuspended(ctx context.Context, g *WithT, obj conditions.Setter) {
+	g.THelper()
+
+	key := client.ObjectKeyFromObject(obj)
+	g.Eventually(func() bool {
+		if err := testEnv.Get(ctx, key, obj); err != nil {
+			return false
+		}
+		suspended, err := object.GetSuspend(obj)
+		if err != nil {
+			return false
+		}
+		return suspended == true
+	}, timeout).Should(BeTrue())
+}
+
+// waitForSourceReadyWithArtifact is a generic test helper to wait for an object
+// to be ready of any source kind that have artifact in status when ready.
+func waitForSourceReadyWithArtifact(ctx context.Context, g *WithT, obj conditions.Setter) {
+	g.THelper()
+	waitForSourceReady(ctx, g, obj, true)
+}
+
+// waitForSourceReadyWithoutArtifact is a generic test helper to wait for an object
+// to be ready of any source kind that don't have artifact in status when ready.
+func waitForSourceReadyWithoutArtifact(ctx context.Context, g *WithT, obj conditions.Setter) {
+	g.THelper()
+	waitForSourceReady(ctx, g, obj, false)
+}
+
+// waitForSourceReady is a generic test helper to wait for an object to be
+// ready of any source kind.
+func waitForSourceReady(ctx context.Context, g *WithT, obj conditions.Setter, withArtifact bool) {
+	g.THelper()
+
+	key := client.ObjectKeyFromObject(obj)
+	g.Eventually(func() bool {
+		if err := testEnv.Get(ctx, key, obj); err != nil {
+			return false
+		}
+		if withArtifact {
+			artifact, err := object.GetArtifact(obj)
+			if err != nil {
+				return false
+			}
+			if artifact == nil {
+				return false
+			}
+		}
+		if !conditions.IsReady(obj) {
+			return false
+		}
+		readyCondition := conditions.Get(obj, meta.ReadyCondition)
+		statusObservedGen, err := object.GetStatusObservedGeneration(obj)
+		if err != nil {
+			return false
+		}
+		return obj.GetGeneration() == readyCondition.ObservedGeneration &&
+			obj.GetGeneration() == statusObservedGen
+	}, timeout).Should(BeTrue())
+}
+
+// testSuspendedObjectDeleteWithArtifact is a generic test helper to test if a
+// suspended object can be deleted for objects that have artifact in status when
+// ready.
+func testSuspendedObjectDeleteWithArtifact(ctx context.Context, g *WithT, obj conditions.Setter) {
+	g.THelper()
+	testSuspendedObjectDelete(ctx, g, obj, true)
+}
+
+// testSuspendedObjectDeleteWithoutArtifact is a generic test helper to test if
+// a suspended object can be deleted for objects that don't have artifact in
+// status when ready.
+func testSuspendedObjectDeleteWithoutArtifact(ctx context.Context, g *WithT, obj conditions.Setter) {
+	g.THelper()
+	testSuspendedObjectDelete(ctx, g, obj, false)
+}
+
+// testSuspendedObjectDelete is a generic test helper to test if a suspended
+// object can be deleted.
+func testSuspendedObjectDelete(ctx context.Context, g *WithT, obj conditions.Setter, withArtifact bool) {
+	g.THelper()
+
+	// Create the object and wait for it to be ready.
+	g.Expect(testEnv.Create(ctx, obj)).To(Succeed())
+	waitForSourceReady(ctx, g, obj, withArtifact)
+
+	// Suspend the object.
+	patchHelper, err := patch.NewHelper(obj, testEnv.Client)
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(object.SetSuspend(obj, true)).ToNot(HaveOccurred())
+	g.Expect(patchHelper.Patch(ctx, obj)).ToNot(HaveOccurred())
+	waitForSuspended(ctx, g, obj)
+
+	// Delete the object.
+	g.Expect(testEnv.Delete(ctx, obj)).To(Succeed())
+	waitForSourceDeletion(ctx, g, obj)
+}

controllers/gitrepository_controller.go

@@ -172,12 +172,6 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	// Record suspended status metric
 	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 
-	// Return early if the object is suspended
-	if obj.Spec.Suspend {
-		log.Info("reconciliation is suspended for this object")
-		return ctrl.Result{}, nil
-	}
-
 	// Initialize the patch helper with the current version of the object.
 	patchHelper, err := patch.NewHelper(obj, r.Client)
 	if err != nil {
@@ -225,6 +219,13 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return
 	}
 
+	// Return if the object is suspended.
+	if obj.Spec.Suspend {
+		log.Info("reconciliation is suspended for this object")
+		recResult, retErr = sreconcile.ResultEmpty, nil
+		return
+	}
+
 	// Reconcile actual object
 	reconcilers := []gitRepositoryReconcileFunc{
 		r.reconcileStorage,

controllers/gitrepository_controller_test.go

@@ -37,7 +37,6 @@ import (
 	. "github.com/onsi/gomega"
 	sshtestdata "golang.org/x/crypto/ssh/testdata"
 	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
@@ -168,7 +167,7 @@ func TestGitRepositoryReconciler_Reconcile(t *testing.T) {
 	_, err = initGitRepo(server, "testdata/git/repository", git.DefaultBranch, repoPath)
 	g.Expect(err).NotTo(HaveOccurred())
 
-	obj := &sourcev1.GitRepository{
+	origObj := &sourcev1.GitRepository{
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "gitrepository-reconcile-",
 			Namespace:    "default",
@@ -178,6 +177,7 @@ func TestGitRepositoryReconciler_Reconcile(t *testing.T) {
 			URL:      server.HTTPAddress() + repoPath,
 		},
 	}
+	obj := origObj.DeepCopy()
 	g.Expect(testEnv.Create(ctx, obj)).To(Succeed())
 
 	key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}
@@ -191,17 +191,7 @@ func TestGitRepositoryReconciler_Reconcile(t *testing.T) {
 	}, timeout).Should(BeTrue())
 
 	// Wait for GitRepository to be Ready
-	g.Eventually(func() bool {
-		if err := testEnv.Get(ctx, key, obj); err != nil {
-			return false
-		}
-		if !conditions.IsReady(obj) || obj.Status.Artifact == nil {
-			return false
-		}
-		readyCondition := conditions.Get(obj, meta.ReadyCondition)
-		return obj.Generation == readyCondition.ObservedGeneration &&
-			obj.Generation == obj.Status.ObservedGeneration
-	}, timeout).Should(BeTrue())
+	waitForSourceReadyWithArtifact(ctx, g, obj)
 
 	// Check if the object status is valid.
 	condns := &conditionscheck.Conditions{NegativePolarity: gitRepositoryReadyCondition.NegativePolarity}
@@ -233,12 +223,11 @@ func TestGitRepositoryReconciler_Reconcile(t *testing.T) {
 	g.Expect(testEnv.Delete(ctx, obj)).To(Succeed())
 
 	// Wait for GitRepository to be deleted
-	g.Eventually(func() bool {
-		if err := testEnv.Get(ctx, key, obj); err != nil {
-			return apierrors.IsNotFound(err)
-		}
-		return false
-	}, timeout).Should(BeTrue())
+	waitForSourceDeletion(ctx, g, obj)
+
+	// Check if a suspended object gets deleted.
+	obj = origObj.DeepCopy()
+	testSuspendedObjectDeleteWithArtifact(ctx, g, obj)
 }
 
 func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {

controllers/helmchart_controller.go

@@ -194,12 +194,6 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	// Record suspended status metric
 	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 
-	// Return early if the object is suspended
-	if obj.Spec.Suspend {
-		log.Info("Reconciliation is suspended for this object")
-		return ctrl.Result{}, nil
-	}
-
 	// Initialize the patch helper with the current version of the object.
 	patchHelper, err := patch.NewHelper(obj, r.Client)
 	if err != nil {
@@ -246,6 +240,13 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return
 	}
 
+	// Return if the object is suspended.
+	if obj.Spec.Suspend {
+		log.Info("Reconciliation is suspended for this object")
+		recResult, retErr = sreconcile.ResultEmpty, nil
+		return
+	}
+
 	// Reconcile actual object
 	reconcilers := []helmChartReconcileFunc{
 		r.reconcileStorage,

controllers/helmchart_controller_test.go

@@ -85,6 +85,8 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) {
 		{
 			name: "Reconciles chart build",
 			assertFunc: func(g *WithT, obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) {
+				origObj := obj.DeepCopy()
+
 				key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}
 
 				// Wait for finalizer to be set
@@ -96,17 +98,7 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) {
 				}, timeout).Should(BeTrue())
 
 				// Wait for HelmChart to be Ready
-				g.Eventually(func() bool {
-					if err := testEnv.Get(ctx, key, obj); err != nil {
-						return false
-					}
-					if !conditions.IsReady(obj) || obj.Status.Artifact == nil {
-						return false
-					}
-					readyCondition := conditions.Get(obj, meta.ReadyCondition)
-					return obj.Generation == readyCondition.ObservedGeneration &&
-						obj.Generation == obj.Status.ObservedGeneration
-				}, timeout).Should(BeTrue())
+				waitForSourceReadyWithArtifact(ctx, g, obj)
 
 				// Check if the object status is valid.
 				condns := &conditionscheck.Conditions{NegativePolarity: helmChartReadyCondition.NegativePolarity}
@@ -146,12 +138,15 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) {
 				g.Expect(testEnv.Delete(ctx, obj)).To(Succeed())
 
 				// Wait for HelmChart to be deleted
-				g.Eventually(func() bool {
-					if err := testEnv.Get(ctx, key, obj); err != nil {
-						return apierrors.IsNotFound(err)
-					}
-					return false
-				}, timeout).Should(BeTrue())
+				waitForSourceDeletion(ctx, g, obj)
+
+				// Check if a suspended object gets deleted.
+				// NOTE: Since the object is already created when received in
+				// this assertFunc, reset the ResourceVersion from the object
+				// before recreating it to avoid API server error.
+				obj = origObj.DeepCopy()
+				obj.ResourceVersion = ""
+				testSuspendedObjectDeleteWithArtifact(ctx, g, obj)
 			},
 		},
 		{

controllers/helmrepository_controller.go

@@ -159,12 +159,6 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	// Record suspended status metric
 	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 
-	// Return early if the object is suspended
-	if obj.Spec.Suspend {
-		log.Info("reconciliation is suspended for this object")
-		return ctrl.Result{}, nil
-	}
-
 	// Initialize the patch helper with the current version of the object.
 	patchHelper, err := patch.NewHelper(obj, r.Client)
 	if err != nil {
@@ -212,6 +206,13 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		return
 	}
 
+	// Return if the object is suspended.
+	if obj.Spec.Suspend {
+		log.Info("reconciliation is suspended for this object")
+		recResult, retErr = sreconcile.ResultEmpty, nil
+		return
+	}
+
 	// Reconcile actual object
 	reconcilers := []helmRepositoryReconcileFunc{
 		r.reconcileStorage,

controllers/helmrepository_controller_oci.go

@@ -120,12 +120,6 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re
 	// Record suspended status metric
 	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 
-	// Return early if the object is suspended
-	if obj.Spec.Suspend {
-		log.Info("reconciliation is suspended for this object")
-		return ctrl.Result{}, nil
-	}
-
 	// Initialize the patch helper with the current version of the object.
 	patchHelper, err := patch.NewHelper(obj, r.Client)
 	if err != nil {
@@ -179,6 +173,12 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		return r.reconcileDelete(ctx, obj)
 	}
 
+	// Return if the object is suspended.
+	if obj.Spec.Suspend {
+		log.Info("reconciliation is suspended for this object")
+		return ctrl.Result{}, nil
+	}
+
 	// Examine if a type change has happened and act accordingly
 	if obj.Spec.Type != sourcev1.HelmRepositoryTypeOCI {
 		// Remove any stale condition and ignore the object if the type has

controllers/helmrepository_controller_oci_test.go

@@ -23,7 +23,6 @@ import (
 
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/record"
 	kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status"
@@ -89,7 +88,7 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) {
 
 			g.Expect(testEnv.CreateAndWait(ctx, secret)).To(Succeed())
 
-			obj := &sourcev1.HelmRepository{
+			origObj := &sourcev1.HelmRepository{
 				ObjectMeta: metav1.ObjectMeta{
 					GenerateName: "helmrepository-oci-reconcile-",
 					Namespace:    ns.Name,
@@ -104,6 +103,7 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) {
 					Type:     sourcev1.HelmRepositoryTypeOCI,
 				},
 			}
+			obj := origObj.DeepCopy()
 			g.Expect(testEnv.Create(ctx, obj)).To(Succeed())
 
 			key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}
@@ -117,17 +117,7 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) {
 			}, timeout).Should(BeTrue())
 
 			// Wait for HelmRepository to be Ready
-			g.Eventually(func() bool {
-				if err := testEnv.Get(ctx, key, obj); err != nil {
-					return false
-				}
-				if !conditions.IsReady(obj) {
-					return false
-				}
-				readyCondition := conditions.Get(obj, meta.ReadyCondition)
-				return obj.Generation == readyCondition.ObservedGeneration &&
-					obj.Generation == obj.Status.ObservedGeneration
-			}, timeout).Should(BeTrue())
+			waitForSourceReadyWithoutArtifact(ctx, g, obj)
 
 			// Check if the object status is valid.
 			condns := &conditionscheck.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity}
@@ -159,12 +149,11 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) {
 			g.Expect(testEnv.Delete(ctx, obj)).To(Succeed())
 
 			// Wait for HelmRepository to be deleted
-			g.Eventually(func() bool {
-				if err := testEnv.Get(ctx, key, obj); err != nil {
-					return apierrors.IsNotFound(err)
-				}
-				return false
-			}, timeout).Should(BeTrue())
+			waitForSourceDeletion(ctx, g, obj)
+
+			// Check if a suspended object gets deleted.
+			obj = origObj.DeepCopy()
+			testSuspendedObjectDeleteWithoutArtifact(ctx, g, obj)
 		})
 	}
 }

controllers/helmrepository_controller_test.go

@@ -65,7 +65,7 @@ func TestHelmRepositoryReconciler_Reconcile(t *testing.T) {
 	testServer.Start()
 	defer testServer.Stop()
 
-	obj := &sourcev1.HelmRepository{
+	origObj := &sourcev1.HelmRepository{
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "helmrepository-reconcile-",
 			Namespace:    "default",
@@ -75,6 +75,7 @@ func TestHelmRepositoryReconciler_Reconcile(t *testing.T) {
 			URL:      testServer.URL(),
 		},
 	}
+	obj := origObj.DeepCopy()
 	g.Expect(testEnv.Create(ctx, obj)).To(Succeed())
 
 	key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}
@@ -88,18 +89,7 @@ func TestHelmRepositoryReconciler_Reconcile(t *testing.T) {
 	}, timeout).Should(BeTrue())
 
 	// Wait for HelmRepository to be Ready
-	g.Eventually(func() bool {
-		if err := testEnv.Get(ctx, key, obj); err != nil {
-			return false
-		}
-		if !conditions.IsReady(obj) && obj.Status.Artifact == nil {
-			return false
-		}
-		readyCondition := conditions.Get(obj, meta.ReadyCondition)
-		return readyCondition.Status == metav1.ConditionTrue &&
-			obj.Generation == readyCondition.ObservedGeneration &&
-			obj.Generation == obj.Status.ObservedGeneration
-	}, timeout).Should(BeTrue())
+	waitForSourceReadyWithArtifact(ctx, g, obj)
 
 	// Check if the object status is valid.
 	condns := &conditionscheck.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity}
@@ -131,12 +121,11 @@ func TestHelmRepositoryReconciler_Reconcile(t *testing.T) {
 	g.Expect(testEnv.Delete(ctx, obj)).To(Succeed())
 
 	// Wait for HelmRepository to be deleted
-	g.Eventually(func() bool {
-		if err := testEnv.Get(ctx, key, obj); err != nil {
-			return apierrors.IsNotFound(err)
-		}
-		return false
-	}, timeout).Should(BeTrue())
+	waitForSourceDeletion(ctx, g, obj)
+
+	// Check if a suspended object gets deleted.
+	obj = origObj.DeepCopy()
+	testSuspendedObjectDeleteWithArtifact(ctx, g, obj)
 }
 
 func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {

controllers/ocirepository_controller.go

@@ -177,12 +177,6 @@ func (r *OCIRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	// Record suspended status metric
 	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 
-	// Return early if the object is suspended
-	if obj.Spec.Suspend {
-		log.Info("reconciliation is suspended for this object")
-		return ctrl.Result{}, nil
-	}
-
 	// Initialize the patch helper with the current version of the object.
 	patchHelper, err := patch.NewHelper(obj, r.Client)
 	if err != nil {
@@ -229,6 +223,13 @@ func (r *OCIRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return
 	}
 
+	// Return if the object is suspended.
+	if obj.Spec.Suspend {
+		log.Info("reconciliation is suspended for this object")
+		recResult, retErr = sreconcile.ResultEmpty, nil
+		return
+	}
+
 	// Reconcile actual object
 	reconcilers := []ociRepositoryReconcileFunc{
 		r.reconcileStorage,

controllers/ocirepository_controller_test.go

@@ -135,7 +135,7 @@ func TestOCIRepository_Reconcile(t *testing.T) {
 			g.Expect(err).ToNot(HaveOccurred())
 			defer func() { g.Expect(testEnv.Delete(ctx, ns)).To(Succeed()) }()
 
-			obj := &sourcev1.OCIRepository{
+			origObj := &sourcev1.OCIRepository{
 				ObjectMeta: metav1.ObjectMeta{
 					GenerateName: "ocirepository-reconcile",
 					Namespace:    ns.Name,
@@ -146,6 +146,7 @@ func TestOCIRepository_Reconcile(t *testing.T) {
 					Reference: &sourcev1.OCIRepositoryRef{},
 				},
 			}
+			obj := origObj.DeepCopy()
 
 			if tt.tag != "" {
 				obj.Spec.Reference.Tag = tt.tag
@@ -174,17 +175,7 @@ func TestOCIRepository_Reconcile(t *testing.T) {
 			}, timeout).Should(BeTrue())
 
 			// Wait for the object to be Ready
-			g.Eventually(func() bool {
-				if err := testEnv.Get(ctx, key, obj); err != nil {
-					return false
-				}
-				if !conditions.IsReady(obj) {
-					return false
-				}
-				readyCondition := conditions.Get(obj, meta.ReadyCondition)
-				return obj.Generation == readyCondition.ObservedGeneration &&
-					obj.Generation == obj.Status.ObservedGeneration
-			}, timeout).Should(BeTrue())
+			waitForSourceReadyWithArtifact(ctx, g, obj)
 
 			// Check if the revision matches the expected digest
 			g.Expect(obj.Status.Artifact.Revision).To(Equal(tt.digest))
@@ -252,12 +243,11 @@ func TestOCIRepository_Reconcile(t *testing.T) {
 
 			// Wait for the object to be deleted
 			g.Expect(testEnv.Delete(ctx, obj)).To(Succeed())
-			g.Eventually(func() bool {
-				if err := testEnv.Get(ctx, key, obj); err != nil {
-					return apierrors.IsNotFound(err)
-				}
-				return false
-			}, timeout).Should(BeTrue())
+			waitForSourceDeletion(ctx, g, obj)
+
+			// Check if a suspended object gets deleted.
+			obj = origObj.DeepCopy()
+			testSuspendedObjectDeleteWithArtifact(ctx, g, obj)
 		})
 	}
 }

internal/object/object.go

@@ -17,11 +17,14 @@ limitations under the License.
 package object
 
 import (
+	"encoding/json"
 	"errors"
 	"time"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+
+	sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
 )
 
 var (
@@ -112,3 +115,59 @@ func GetRequeueInterval(obj runtime.Object) (time.Duration, error) {
 	}
 	return time.ParseDuration(interval)
 }
+
+// GetSuspend returns the spec.suspend of a given runtime object.
+func GetSuspend(obj runtime.Object) (bool, error) {
+	u, err := toUnstructured(obj)
+	if err != nil {
+		return false, err
+	}
+	suspend, found, err := unstructured.NestedBool(u.Object, "spec", "suspend")
+	if err != nil {
+		return false, err
+	}
+	// Since suspend is an optional field, it's false when not found.
+	if !found {
+		return false, nil
+	}
+	return suspend, nil
+}
+
+// SetSuspend sets the spec.suspend value of a given runtime object.
+func SetSuspend(obj runtime.Object, val bool) error {
+	content, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
+	if err != nil {
+		return err
+	}
+	u := unstructured.Unstructured{}
+	u.SetUnstructuredContent(content)
+	if err := unstructured.SetNestedField(u.Object, val, "spec", "suspend"); err != nil {
+		return err
+	}
+	return runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, obj)
+}
+
+// GetArtifact returns the status.artifact of a given runtime object.
+func GetArtifact(obj runtime.Object) (*sourcev1.Artifact, error) {
+	u, err := toUnstructured(obj)
+	if err != nil {
+		return nil, err
+	}
+	artifact, found, err := unstructured.NestedFieldNoCopy(u.Object, "status", "artifact")
+	if err != nil {
+		return nil, err
+	}
+	// Since artifact is an optional field, return nil when not found.
+	if !found {
+		return nil, nil
+	}
+	enc, err := json.Marshal(artifact)
+	if err != nil {
+		return nil, err
+	}
+	outArtifact := &sourcev1.Artifact{}
+	if err := json.Unmarshal(enc, outArtifact); err != nil {
+		return nil, err
+	}
+	return outArtifact, nil
+}

internal/object/object_test.go

@@ -86,3 +86,51 @@ func TestGetRequeueInterval(t *testing.T) {
 	_, err = GetRequeueInterval(obj2)
 	g.Expect(err).To(Equal(ErrRequeueIntervalNotFound))
 }
+
+func TestGetSuspend(t *testing.T) {
+	g := NewWithT(t)
+
+	// Get unset suspend value.
+	obj := &sourcev1.GitRepository{}
+	suspend, err := GetSuspend(obj)
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(suspend).To(BeFalse())
+
+	// Get set suspend value.
+	obj.Spec.Suspend = true
+	suspend, err = GetSuspend(obj)
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(suspend).To(BeTrue())
+}
+
+func TestSetSuspend(t *testing.T) {
+	g := NewWithT(t)
+
+	obj := &sourcev1.GitRepository{}
+	err := SetSuspend(obj, true)
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(obj.Spec.Suspend).To(BeTrue())
+
+	// Overwrite previous value.
+	err = SetSuspend(obj, false)
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(obj.Spec.Suspend).To(BeFalse())
+}
+
+func TestGetArtifact(t *testing.T) {
+	g := NewWithT(t)
+
+	// Get unset artifact value.
+	obj := &sourcev1.GitRepository{}
+	artifact, err := GetArtifact(obj)
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(artifact).To(BeNil())
+
+	// Get set artifact value.
+	obj.Status.Artifact = &sourcev1.Artifact{Path: "aaa", Revision: "zzz"}
+	artifact, err = GetArtifact(obj)
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(artifact).ToNot(BeNil())
+	g.Expect(artifact.Path).To(Equal("aaa"))
+	g.Expect(artifact.Revision).To(Equal("zzz"))
+}