Skip to content
Open
6 changes: 0 additions & 6 deletions docs/deletion.md

This file was deleted.

39 changes: 36 additions & 3 deletions internal/controller/clustertunnel_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package controller
import (
"context"

"github.com/adyanth/cloudflare-operator/internal/k8s"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -143,9 +145,23 @@ func (r *ClusterTunnelReconciler) Reconcile(ctx context.Context, req ctrl.Reques
if err := r.Get(ctx, req.NamespacedName, tunnel); err != nil {
if apierrors.IsNotFound(err) {
// Tunnel object not found, could have been deleted after reconcile request.
// Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
// Return and don't requeue
r.log.Info("Tunnel deleted, nothing to do")
// Owned objects are automatically garbage collected.
objectClient, err := k8s.NewObjectClient(r.Client, &r.log)
if err != nil {
return ctrl.Result{}, err
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is in the wrong place (ref: the error you saw). When this condition is hit, the tunnel is already removed from etcd.

What you need is, instead of the logic in both here and tunnel_controller.go, move this to

func cleanupTunnel(r GenericTunnelReconciler) (ctrl.Result, bool, error) {
if controllerutil.ContainsFinalizer(r.GetTunnel().GetObject(), tunnelFinalizer) {
// Run finalization logic. If the finalization logic fails,
// don't remove the finalizer so that we can retry during the next reconciliation.
r.GetLog().Info("starting deletion cycle")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, derp. Thanks will fix this

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you get around to this? Would love to merge this in

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't sorry, life has been a lot this past month! Hoping to get to this next weekend

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good. Life comes first!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanted to give you another heads up, might be another week

}
// ensure the secret associated with the tunnel has the finalizer removed
err = objectClient.RemoveFinalizer(
ctx,
client.ObjectKey{
Namespace: r.GetTunnel().GetNamespace(),
Name: r.GetTunnel().GetSpec().Cloudflare.Secret,
},
tunnelFinalizer,
)
if err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
r.log.Error(err, "unable to fetch Tunnel")
Expand All @@ -156,6 +172,23 @@ func (r *ClusterTunnelReconciler) Reconcile(ctx context.Context, req ctrl.Reques
return ctrl.Result{}, err
}

objectClient, err := k8s.NewObjectClient(r.Client, &r.log)
if err != nil {
return ctrl.Result{}, err
}
// ensure the secret associated with the tunnel has a finalizer
err = objectClient.EnsureFinalizer(
ctx,
client.ObjectKey{
Namespace: r.GetTunnel().GetNamespace(),
Name: r.GetTunnel().GetSpec().Cloudflare.Secret,
},
tunnelFinalizer,
)
if err != nil {
return ctrl.Result{}, err
}

if res, ok, err := setupTunnel(r); !ok {
return res, err
}
Expand Down
38 changes: 35 additions & 3 deletions internal/controller/tunnel_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package controller
import (
"context"

"github.com/adyanth/cloudflare-operator/internal/k8s"

corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -138,9 +140,22 @@ func (r *TunnelReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
if err := r.Get(ctx, req.NamespacedName, tunnel); err != nil {
if apierrors.IsNotFound(err) {
// Tunnel object not found, could have been deleted after reconcile request.
// Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
// Return and don't requeue
r.log.Info("Tunnel deleted, nothing to do")
// Owned objects are automatically garbage collected.
objectClient, err := k8s.NewObjectClient(r.Client, &r.log)
if err != nil {
return ctrl.Result{}, err
}
err = objectClient.RemoveFinalizer(
ctx,
client.ObjectKey{
Namespace: r.GetTunnel().GetNamespace(),
Name: r.GetTunnel().GetSpec().Cloudflare.Secret,
},
tunnelFinalizer,
)
if err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
r.log.Error(err, "unable to fetch Tunnel")
Expand All @@ -151,6 +166,23 @@ func (r *TunnelReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
return ctrl.Result{}, err
}

objectClient, err := k8s.NewObjectClient(r.Client, &r.log)
if err != nil {
return ctrl.Result{}, err
}
// ensure the secret associated with the tunnel has a finalizer
err = objectClient.EnsureFinalizer(
ctx,
client.ObjectKey{
Namespace: r.GetTunnel().GetNamespace(),
Name: r.GetTunnel().GetSpec().Cloudflare.Secret,
},
tunnelFinalizer,
)
if err != nil {
return ctrl.Result{}, err
}

if res, ok, err := setupTunnel(r); !ok {
return res, err
}
Expand Down
105 changes: 105 additions & 0 deletions internal/k8s/object.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package k8s

import (
"context"
"errors"
"fmt"

"github.com/go-logr/logr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)

type ObjectClient struct {
k8sClient client.Client
log *logr.Logger
}

func NewObjectClient(k8sClient client.Client, log *logr.Logger) (*ObjectClient, error) {
if k8sClient == nil {
return nil, errors.New("k8sClient cannot be nil")
}
if log == nil {
return nil, errors.New("log cannot be nil")
}
return &ObjectClient{
k8sClient: k8sClient,
log: log,
}, nil
}

// EnsureFinalizer adds `finalizer` to obj exactly once.
// - It NO-OPs if the finalizer is already there.
// - It uses a strategic-merge Patch so you don’t overwrite concurrent changes.
// - obj must be a pointer that already contains the latest copy from the API server.
func (s *ObjectClient) EnsureFinalizer(ctx context.Context, key client.ObjectKey, finalizer string) error {
var obj client.Object
err := s.k8sClient.Get(ctx, key, obj)
if err != nil {
return err
}
Comment thread
cyclingwithelephants marked this conversation as resolved.
// if it already contains the finalizer, there's nothing to do
if controllerutil.ContainsFinalizer(obj, finalizer) {
return nil
}

controllerutil.AddFinalizer(obj, finalizer)

base, err := s.deepCopyObject(obj)
if err != nil {
return err
}

s.log.
WithValues("finalizer", finalizer).
WithValues("object", fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName())).
Info("creating finalizer")
if err := s.k8sClient.Patch(ctx, obj, client.MergeFrom(base)); err != nil {
Comment thread
adyanth marked this conversation as resolved.
return fmt.Errorf("could not add finalizer %q: %w", finalizer, err)
}
return nil
}

// RemoveFinalizer deletes `finalizer` from obj exactly once.
// - NO-OPs if the finalizer is already gone.
// - Uses a strategic-merge Patch so you never clobber concurrent changes.
// - obj must be a *live* copy fetched from the API server.
func (s *ObjectClient) RemoveFinalizer(ctx context.Context, key client.ObjectKey, finalizer string) error {
var obj client.Object
err := s.k8sClient.Get(ctx, key, obj)
if err != nil {
return err
}
// if there is no finalizer, there's nothing to do
if !controllerutil.ContainsFinalizer(obj, finalizer) {
return nil
}

controllerutil.RemoveFinalizer(obj, finalizer)

base, err := s.deepCopyObject(obj)
if err != nil {
return err
}

s.log.
WithValues("finalizer", finalizer).
WithValues("object", fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName())).
Info("removing finalizer")
if err := s.k8sClient.Patch(ctx, obj, client.MergeFrom(base)); err != nil {
return fmt.Errorf("could not remove finalizer %q: %w", finalizer, err)
}
return nil
}

func (*ObjectClient) deepCopyObject(obj client.Object) (client.Object, error) {
objDeepCopy := obj.DeepCopyObject()
if objDeepCopy == nil {
return nil, errors.New("received nil object from DeepCopyObject")
}
base, ok := objDeepCopy.(client.Object)
if !ok {
return nil, errors.New("failed to convert object to client.Object")
}
Comment thread
adyanth marked this conversation as resolved.
Outdated
return base, nil
}