Improve HelmRepository type switching from default to oci

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)
  })
  }
 }

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)
  })
  }
 }

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)
  })
  }
 }

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
+}

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))
  })
  }
 }

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())
 

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)
  })
  }
 }

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

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=

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)
  })
  }
 }