diff --git a/controllers/bucket_controller.go b/controllers/bucket_controller.go index f2608bf40..40c723980 100644 --- a/controllers/bucket_controller.go +++ b/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, diff --git a/controllers/bucket_controller_test.go b/controllers/bucket_controller_test.go index b0ec4a531..883f08642 100644 --- a/controllers/bucket_controller_test.go +++ b/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) { diff --git a/controllers/common_test.go b/controllers/common_test.go new file mode 100644 index 000000000..18df1ab51 --- /dev/null +++ b/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) +} diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 8ea55aae1..1623fd6d2 100644 --- a/controllers/gitrepository_controller.go +++ b/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, diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 52b131bcf..8e2af48f2 100644 --- a/controllers/gitrepository_controller_test.go +++ b/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) { diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 965ddcedc..3f6b85040 100644 --- a/controllers/helmchart_controller.go +++ b/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, diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 40a775222..43ddd883d 100644 --- a/controllers/helmchart_controller_test.go +++ b/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) }, }, { diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index ea72a51b6..b969f1b55 100644 --- a/controllers/helmrepository_controller.go +++ b/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, diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 5d60d2b1c..d42154d6f 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/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 diff --git a/controllers/helmrepository_controller_oci_test.go b/controllers/helmrepository_controller_oci_test.go index 953e1eee6..6a0a6009c 100644 --- a/controllers/helmrepository_controller_oci_test.go +++ b/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) }) } } diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go index c2cb73a96..31d1beb61 100644 --- a/controllers/helmrepository_controller_test.go +++ b/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) { diff --git a/controllers/ocirepository_controller.go b/controllers/ocirepository_controller.go index 677e6b6da..35aec494a 100644 --- a/controllers/ocirepository_controller.go +++ b/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, diff --git a/controllers/ocirepository_controller_test.go b/controllers/ocirepository_controller_test.go index b7932d1ad..b37c049ed 100644 --- a/controllers/ocirepository_controller_test.go +++ b/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) }) } } diff --git a/internal/object/object.go b/internal/object/object.go index c4bd32c22..17fa4ef55 100644 --- a/internal/object/object.go +++ b/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 +} diff --git a/internal/object/object_test.go b/internal/object/object_test.go index 9f0d80bbb..1ab24ca5e 100644 --- a/internal/object/object_test.go +++ b/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")) +}