diff --git a/controllers/bucket_controller_test.go b/controllers/bucket_controller_test.go index 0593c608a..b7a342a6a 100644 --- a/controllers/bucket_controller_test.go +++ b/controllers/bucket_controller_test.go @@ -120,7 +120,7 @@ func TestBucketReconciler_Reconcile(t *testing.T) { // Check if the object status is valid. condns := &conditionscheck.Conditions{NegativePolarity: bucketReadyCondition.NegativePolarity} checker := conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) // kstatus client conformance check. uo, err := patch.ToUnstructured(obj) @@ -321,7 +321,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } } @@ -662,7 +662,7 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) { // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } } @@ -1022,7 +1022,7 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) { // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } } @@ -1201,7 +1201,7 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) { // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } } diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index dff5a4a64..777023a6c 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -186,7 +186,7 @@ func TestGitRepositoryReconciler_Reconcile(t *testing.T) { // Check if the object status is valid. condns := &conditionscheck.Conditions{NegativePolarity: gitRepositoryReadyCondition.NegativePolarity} checker := conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) // kstatus client conformance check. u, err := patch.ToUnstructured(obj) @@ -590,7 +590,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) { // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } } @@ -815,7 +815,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T) } // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } } @@ -1374,7 +1374,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } } diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 15b2424fc..98e2b82a6 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -109,7 +109,7 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) { // Check if the object status is valid. condns := &conditionscheck.Conditions{NegativePolarity: helmChartReadyCondition.NegativePolarity} checker := conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) // kstatus client conformance check. u, err := patch.ToUnstructured(obj) @@ -177,7 +177,7 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) { // Check if the object status is valid. condns := &conditionscheck.Conditions{NegativePolarity: helmChartReadyCondition.NegativePolarity} checker := conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) @@ -212,7 +212,7 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) { // Check if the object status is valid. condns := &conditionscheck.Conditions{NegativePolarity: helmChartReadyCondition.NegativePolarity} checker := conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) @@ -444,7 +444,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } } @@ -708,7 +708,7 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) { // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, &obj) + checker.WithT(g).CheckErr(ctx, &obj) }) } } diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index e971a11eb..a0424c45f 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -40,6 +40,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/predicate" + eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/oci" "github.com/fluxcd/pkg/runtime/conditions" @@ -82,6 +83,11 @@ type HelmRepositoryOCIReconciler struct { RegistryClientGenerator RegistryClientGeneratorFunc patchOptions []patch.Option + + // unmanagedConditions are the conditions that are not managed by this + // reconciler and need to be removed from the object before taking ownership + // of the object being reconciled. + unmanagedConditions []string } // RegistryClientGeneratorFunc is a function that returns a registry client @@ -95,6 +101,7 @@ func (r *HelmRepositoryOCIReconciler) SetupWithManager(mgr ctrl.Manager) error { } func (r *HelmRepositoryOCIReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmRepositoryReconcilerOptions) error { + r.unmanagedConditions = conditionsDiff(helmRepositoryReadyCondition.Owned, helmRepositoryOCIOwnedConditions) r.patchOptions = getPatchOptions(helmRepositoryOCIOwnedConditions, r.ControllerName) recoverPanic := true @@ -124,6 +131,16 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{}, client.IgnoreNotFound(err) } + // If the object contains any of the unmanaged conditions, requeue and wait + // for those conditions to be removed first before processing the object. + // NOTE: This will happen only when a HelmRepository's spec.type is switched + // from "default" to "oci". + if conditions.HasAny(obj, r.unmanagedConditions) { + r.eventLogf(ctx, obj, eventv1.EventTypeTrace, "IncompleteTransition", + "object contains conditions managed by other reconciler") + return ctrl.Result{RequeueAfter: time.Second}, nil + } + // Record suspended status metric r.RecordSuspend(ctx, obj, obj.Spec.Suspend) @@ -428,3 +445,18 @@ func makeLoginOption(auth authn.Authenticator, keychain authn.Keychain, registry return nil, nil } + +func conditionsDiff(a, b []string) []string { + bMap := make(map[string]struct{}, len(b)) + for _, j := range b { + bMap[j] = struct{}{} + } + + r := []string{} + for _, i := range a { + if _, exists := bMap[i]; !exists { + r = append(r, i) + } + } + return r +} diff --git a/controllers/helmrepository_controller_oci_test.go b/controllers/helmrepository_controller_oci_test.go index f4bbe7909..77ce28742 100644 --- a/controllers/helmrepository_controller_oci_test.go +++ b/controllers/helmrepository_controller_oci_test.go @@ -19,6 +19,7 @@ package controllers import ( "encoding/base64" "fmt" + "strconv" "testing" . "github.com/onsi/gomega" @@ -122,7 +123,7 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { // Check if the object status is valid. condns := &conditionscheck.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} checker := conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) // kstatus client conformance check. u, err := patch.ToUnstructured(obj) @@ -316,7 +317,28 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { // NOTE: Check the object directly as reconcile() doesn't apply the // final patch, the object has unapplied changes. checker.DisableFetch = true - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) + }) + } +} + +func TestConditionsDiff(t *testing.T) { + tests := []struct { + a, b, want []string + }{ + {[]string{"a", "b", "c"}, []string{"b", "d"}, []string{"a", "c"}}, + {[]string{"a", "b", "c"}, []string{}, []string{"a", "b", "c"}}, + {[]string{}, []string{"b", "d"}, []string{}}, + {[]string{}, []string{}, []string{}}, + {[]string{"a", "b"}, nil, []string{"a", "b"}}, + {nil, []string{"a", "b"}, []string{}}, + {nil, nil, []string{}}, + } + + for i, tt := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + g := NewWithT(t) + g.Expect(conditionsDiff(tt.a, tt.b)).To(Equal(tt.want)) }) } } diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go index 40b106509..4188e5eb4 100644 --- a/controllers/helmrepository_controller_test.go +++ b/controllers/helmrepository_controller_test.go @@ -94,7 +94,7 @@ func TestHelmRepositoryReconciler_Reconcile(t *testing.T) { // Check if the object status is valid. condns := &conditionscheck.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} checker := conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) // kstatus client conformance check. u, err := patch.ToUnstructured(obj) @@ -292,7 +292,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } } @@ -746,7 +746,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) { // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } } @@ -1278,7 +1278,7 @@ func TestHelmRepositoryReconciler_ReconcileTypeUpdatePredicateFilter(t *testing. // Check if the object status is valid. condns := &conditionscheck.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} checker := conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) // kstatus client conformance check. u, err := patch.ToUnstructured(obj) @@ -1330,7 +1330,7 @@ func TestHelmRepositoryReconciler_ReconcileTypeUpdatePredicateFilter(t *testing. // Check if the object status is valid. condns = &conditionscheck.Conditions{NegativePolarity: helmRepositoryOCINegativeConditions} checker = conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) @@ -1395,7 +1395,7 @@ func TestHelmRepositoryReconciler_ReconcileSpecUpdatePredicateFilter(t *testing. // Check if the object status is valid. condns := &conditionscheck.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} checker := conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) // kstatus client conformance check. u, err := patch.ToUnstructured(obj) @@ -1427,7 +1427,7 @@ func TestHelmRepositoryReconciler_ReconcileSpecUpdatePredicateFilter(t *testing. // Check if the object status is valid. condns = &conditionscheck.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} checker = conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) diff --git a/controllers/ocirepository_controller_test.go b/controllers/ocirepository_controller_test.go index b4d9ce423..e8bae1822 100644 --- a/controllers/ocirepository_controller_test.go +++ b/controllers/ocirepository_controller_test.go @@ -217,7 +217,7 @@ func TestOCIRepository_Reconcile(t *testing.T) { // Check if the object status is valid condns := &conditionscheck.Conditions{NegativePolarity: ociRepositoryReadyCondition.NegativePolarity} checker := conditionscheck.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) // kstatus client conformance check u, err := patch.ToUnstructured(obj) @@ -1998,7 +1998,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { // In-progress status condition validity. checker := conditionscheck.NewInProgressChecker(r.Client) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } } diff --git a/go.mod b/go.mod index 41dd8ba67..107d81400 100644 --- a/go.mod +++ b/go.mod @@ -31,7 +31,7 @@ require ( github.com/fluxcd/pkg/lockedfile v0.1.0 github.com/fluxcd/pkg/masktoken v0.2.0 github.com/fluxcd/pkg/oci v0.18.0 - github.com/fluxcd/pkg/runtime v0.27.0 + github.com/fluxcd/pkg/runtime v0.28.0 github.com/fluxcd/pkg/sourceignore v0.3.0 github.com/fluxcd/pkg/ssh v0.7.0 github.com/fluxcd/pkg/testserver v0.4.0 diff --git a/go.sum b/go.sum index 2efb54854..aabb676f3 100644 --- a/go.sum +++ b/go.sum @@ -541,8 +541,8 @@ github.com/fluxcd/pkg/masktoken v0.2.0 h1:HoSPTk4l1fz5Fevs2vVRvZGru33blfMwWSZKsH github.com/fluxcd/pkg/masktoken v0.2.0/go.mod h1:EA7GleAHL33kN6kTW06m5R3/Q26IyuGO7Ef/0CtpDI0= github.com/fluxcd/pkg/oci v0.18.0 h1:x5n3gW1lX6wrqvWP4ZkOXJ8LqLKy891uKwifCXSqKi4= github.com/fluxcd/pkg/oci v0.18.0/go.mod h1:zXoxvE4uuIEOgA98IM5Wv/uRxs7sdbaTlGDjzHb9yiA= -github.com/fluxcd/pkg/runtime v0.27.0 h1:zVA95Z0KvNjvZxEZhvIbJyJIwtaiv1aVttHZ4YB/FzY= -github.com/fluxcd/pkg/runtime v0.27.0/go.mod h1:fC1l4Wv1hnsqPKB46eDZBXF8RMZm5FXeU4bnJkwGkqk= +github.com/fluxcd/pkg/runtime v0.28.0 h1:FtdZk53oMFUKIGykDtWNi3Pv2lXR6NHPWNqLQV5rpPg= +github.com/fluxcd/pkg/runtime v0.28.0/go.mod h1:fC1l4Wv1hnsqPKB46eDZBXF8RMZm5FXeU4bnJkwGkqk= github.com/fluxcd/pkg/sourceignore v0.3.0 h1:pFO3hKV9ub+2SrNZPZE7xfiRhxsycRrd7JK7qB26nVw= github.com/fluxcd/pkg/sourceignore v0.3.0/go.mod h1:ak3Tve/KwVzytZ5V2yBlGGpTJ/2oQ9kcP3iuwBOAHGo= github.com/fluxcd/pkg/ssh v0.7.0 h1:FX5ky8SU9dYwbM6zEIDR3TSveLF01iyS95CtB5Ykpno= diff --git a/internal/reconcile/summarize/summary_test.go b/internal/reconcile/summarize/summary_test.go index 48ee56489..b3e6f3b97 100644 --- a/internal/reconcile/summarize/summary_test.go +++ b/internal/reconcile/summarize/summary_test.go @@ -371,7 +371,7 @@ func TestSummarizeAndPatch(t *testing.T) { // Check if the object status is valid as per kstatus. condns := &conditionscheck.Conditions{NegativePolarity: testReadyConditions.NegativePolarity} checker := conditionscheck.NewChecker(client, condns) - checker.CheckErr(ctx, obj) + checker.WithT(g).CheckErr(ctx, obj) }) } }