Merge pull request #908 from anmazzotti/fix_infinite_provider_reconcile

Prevent infinite CAPIProvider reconciliations

internal/controllers/capiprovider_controller.go

@@ -48,6 +48,7 @@ type CAPIProviderReconciler struct {
 // Reconcile reconciles the CAPIProvider object.
 func (r *CAPIProviderReconciler) Reconcile(ctx context.Context, capiProvider *turtlesv1.CAPIProvider) (ctrl.Result, error) {
 	log := log.FromContext(ctx)
+	log.Info("Reconciling CAPIProvider")
 
 	if !capiProvider.DeletionTimestamp.IsZero() {
 		log.Info("Provider is in the process of deletion, skipping reconcile...")

internal/sync/client.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
@@ -51,29 +50,20 @@ func Patch(ctx context.Context, cl client.Client, obj client.Object, options ...
 	log.Info(fmt.Sprintf("Updating %s: %s", obj.GetObjectKind().GroupVersionKind().Kind, client.ObjectKeyFromObject(obj)))
 
 	patchOptions := []client.PatchOption{
-		client.ForceOwnership,
 		client.FieldOwner(fieldOwner),
 	}
 	patchOptions = append(patchOptions, options...)
 
-	return cl.Patch(ctx, obj, client.Apply, patchOptions...)
+	return cl.Patch(ctx, obj, client.Merge, patchOptions...)
 }
 
 // PatchStatus will only patch the status subresource of the provided object.
 func PatchStatus(ctx context.Context, cl client.Client, obj client.Object) error {
 	log := log.FromContext(ctx)
 
-	obj.SetManagedFields(nil)
-	obj.SetFinalizers([]string{metav1.FinalizerDeleteDependents})
-
-	if err := setKind(cl, obj); err != nil {
-		return err
-	}
-
 	log.Info(fmt.Sprintf("Patching status %s: %s", obj.GetObjectKind().GroupVersionKind().Kind, client.ObjectKeyFromObject(obj)))
 
-	return cl.Status().Patch(ctx, obj, client.Apply, []client.SubResourcePatchOption{
-		client.ForceOwnership,
+	return cl.Status().Patch(ctx, obj, client.Merge, []client.SubResourcePatchOption{
 		client.FieldOwner(fieldOwner),
 	}...)
 }

internal/sync/core.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/utils/ptr"
@@ -49,8 +50,21 @@ func NewDefaultSynchronizer(cl client.Client, capiProvider *turtlesv1.CAPIProvid
 func (s *DefaultSynchronizer) Get(ctx context.Context) error {
 	log := log.FromContext(ctx)
 
-	if err := s.client.Get(ctx, client.ObjectKeyFromObject(s.Destination), s.Destination); client.IgnoreNotFound(err) != nil {
-		log.Error(err, "Unable to get mirrored manifest: "+client.ObjectKeyFromObject(s.Destination).String())
+	objKey := client.ObjectKeyFromObject(s.Destination)
+
+	err := s.client.Get(ctx, objKey, s.Destination)
+	if apierrors.IsNotFound(err) {
+		log.Info("Mirrored manifest is missing. Creating a new one.")
+
+		if err := s.client.Create(ctx, s.Destination); err != nil {
+			return fmt.Errorf("creating mirrored manifest: %w", err)
+		}
+
+		return nil
+	}
+
+	if err != nil {
+		log.Error(err, "Unable to get mirrored manifest: "+objKey.String())
 
 		return err
 	}
@@ -63,6 +77,7 @@ func (s *DefaultSynchronizer) Apply(ctx context.Context, reterr *error, options
 	log := log.FromContext(ctx)
 	uid := s.Destination.GetUID()
 
+	setFinalizers(s.Destination)
 	setOwnerReference(s.Source, s.Destination)
 
 	if err := Patch(ctx, s.client, s.Destination, options...); err != nil {
@@ -75,8 +90,24 @@ func (s *DefaultSynchronizer) Apply(ctx context.Context, reterr *error, options
 	}
 }
 
+func setFinalizers(obj client.Object) {
+	finalizers := obj.GetFinalizers()
+	if finalizers == nil {
+		finalizers = []string{}
+	}
+
+	// Only append the desired finalizer if it doesn't exist
+	for _, finalizer := range finalizers {
+		if finalizer == metav1.FinalizerDeleteDependents {
+			return
+		}
+	}
+
+	finalizers = append(finalizers, metav1.FinalizerDeleteDependents)
+	obj.SetFinalizers(finalizers)
+}
+
 func setOwnerReference(owner, obj client.Object) {
-	obj.SetFinalizers([]string{metav1.FinalizerDeleteDependents})
 	obj.SetOwnerReferences([]metav1.OwnerReference{{
 		APIVersion:         turtlesv1.GroupVersion.String(),
 		Kind:               turtlesv1.Kind,

internal/sync/core_test.go

@@ -17,11 +17,15 @@ limitations under the License.
 package sync_test
 
 import (
+	"time"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	turtlesv1 "github.com/rancher/turtles/api/v1alpha1"
 	"github.com/rancher/turtles/internal/sync"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	. "sigs.k8s.io/controller-runtime/pkg/envtest/komega"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -89,4 +93,21 @@ var _ = Describe("Core provider", func() {
 		Eventually(Object(secret)).Should(
 			HaveField("OwnerReferences", HaveLen(1)))
 	})
+
+	It("Should create not-found mirrored manifest", func() {
+		secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: ns.Name,
+		}}
+
+		err := testEnv.Client.Get(ctx, client.ObjectKeyFromObject(secret), secret)
+		Expect(apierrors.IsNotFound(err)).Should(BeTrue())
+
+		s := sync.NewDefaultSynchronizer(testEnv, capiProvider, secret)
+		Expect(s.Get(ctx)).To(Succeed())
+
+		Eventually(func() error {
+			return testEnv.Client.Get(ctx, client.ObjectKeyFromObject(secret), secret)
+		}).WithTimeout(time.Minute).Should(Succeed())
+	})
 })