Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent infinite CAPIProvider reconciliations #908

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/controllers/capiprovider_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...")
Expand Down
14 changes: 2 additions & 12 deletions internal/sync/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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,
anmazzotti marked this conversation as resolved.
Show resolved Hide resolved
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),
}...)
}
37 changes: 34 additions & 3 deletions internal/sync/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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})
anmazzotti marked this conversation as resolved.
Show resolved Hide resolved
obj.SetOwnerReferences([]metav1.OwnerReference{{
APIVersion: turtlesv1.GroupVersion.String(),
Kind: turtlesv1.Kind,
Expand Down
21 changes: 21 additions & 0 deletions internal/sync/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
})
})
Loading