respect delete-for-move annotation (Backport #1074)

Author: glennpratt

.github/workflows/e2e-test.yaml

@@ -101,6 +101,7 @@ jobs:
             registry-1.docker.io:443
             auth.docker.io:443
             production.cloudflare.docker.com:443
+            production.cloudfront.docker.com:443
             storage.googleapis.com:443
             registry.k8s.io:443
             *.pkg.dev:443

.github/workflows/e2e-upgrade-test.yaml

@@ -63,6 +63,7 @@ jobs:
             registry-1.docker.io:443
             auth.docker.io:443
             production.cloudflare.docker.com:443
+            production.cloudfront.docker.com:443
             storage.googleapis.com:443
             registry.k8s.io:443
             *.pkg.dev:443

.github/workflows/go-analyze.yml

@@ -53,6 +53,7 @@ jobs:
             *.githubusercontent.com:443
             auth.docker.io:443
             production.cloudflare.docker.com:443
+            production.cloudfront.docker.com:443
             vuln.go.dev:443
             storage.googleapis.com:443
             golangci-lint.run:443

.github/workflows/pull_request_ci.yaml

@@ -104,6 +104,7 @@ jobs:
             registry-1.docker.io:443
             auth.docker.io:443
             production.cloudflare.docker.com:443
+            production.cloudfront.docker.com:443
             gcr.io:443
             storage.googleapis.com:443
 

Makefile

@@ -154,7 +154,7 @@ docs:
 
 .PHONY: test
 test: generate fmt vet envtest ## Run tests.
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use ${ENVTEST_K8S_VERSION#v} --bin-dir $(CACHE_BIN) -p path)" go test -race -timeout 60s `go list ./... | grep -v ./mock$$`  -coverprofile cover.out.tmp
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use ${ENVTEST_K8S_VERSION#v} --bin-dir $(CACHE_BIN) -p path)" go test -timeout 60s `go list ./... | grep -v ./mock$$` -coverpkg=./... -coverprofile cover.out.tmp
 	grep -v "zz_generated.*" cover.out.tmp > cover.out
 	rm cover.out.tmp
 

internal/controller/linodecluster_controller.go

@@ -74,17 +74,11 @@ type LinodeClusterReconciler struct {
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
 
-func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, linodeCluster *infrav1alpha2.LinodeCluster) (ctrl.Result, error) {
 	ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultedLoopTimeout(r.ReconcileTimeout))
 	defer cancel()
 
-	logger := ctrl.LoggerFrom(ctx).WithName("LinodeClusterReconciler").WithValues("name", req.String())
-	linodeCluster := &infrav1alpha2.LinodeCluster{}
-	if err := r.TracedClient().Get(ctx, req.NamespacedName, linodeCluster); err != nil {
-		logger.Info("Failed to fetch Linode cluster", "error", err.Error())
-
-		return ctrl.Result{}, client.IgnoreNotFound(err)
-	}
+	logger := ctrl.LoggerFrom(ctx).WithName("LinodeClusterReconciler").WithValues("name", linodeCluster.Name, "namespace", linodeCluster.Namespace)
 
 	cluster, err := kutil.GetOwnerCluster(ctx, r.TracedClient(), linodeCluster.ObjectMeta)
 	if err != nil {
@@ -544,7 +538,7 @@ func (r *LinodeClusterReconciler) SetupWithManager(mgr ctrl.Manager, options crc
 		Watches(
 			&infrav1alpha2.LinodeMachine{},
 			handler.EnqueueRequestsFromMapFunc(linodeMachineToLinodeCluster(r.TracedClient(), mgr.GetLogger())),
-		).Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
+		).Complete(reconciler.AsReconcilerWithTracing(r.TracedClient(), r))
 	if err != nil {
 		return fmt.Errorf("failed to build controller: %w", err)
 	}

internal/controller/linodecluster_controller_test.go

@@ -33,7 +33,6 @@ import (
 	"sigs.k8s.io/cluster-api/util/paused"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	infrav1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2"
 	"github.com/linode/cluster-api-provider-linode/cloud/scope"
@@ -289,9 +288,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc
 				}),
 				Result("no capl cluster error", func(ctx context.Context, mck Mock) {
 					reconciler.Client = k8sClient
-					_, err := reconciler.Reconcile(ctx, reconcile.Request{
-						NamespacedName: client.ObjectKeyFromObject(cScope.LinodeCluster),
-					})
+					_, err := reconciler.Reconcile(ctx, cScope.LinodeCluster)
 					Expect(err).NotTo(HaveOccurred())
 					Expect(linodeCluster.Status.Ready).To(BeFalseBecause("failed to get Cluster/no-capl-cluster: clusters.cluster.x-k8s.io \"no-capl-cluster\" not found"))
 				}),
@@ -373,7 +370,7 @@ var _ = Describe("pause handling", Label("cluster", "pause"), func() {
 			DnsClientConfig:    scope.ClientConfig{Token: "test-token"},
 		}
 
-		_, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(linodeCluster)})
+		_, err := reconciler.Reconcile(ctx, linodeCluster)
 		Expect(err).NotTo(HaveOccurred())
 
 		Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(linodeCluster), linodeCluster)).To(Succeed())

internal/controller/linodefirewall_controller_helpers.go

@@ -55,7 +55,7 @@ func findObjectsForObject(logger logr.Logger, tracedClient client.Client) handle
 				logger.Info("LinodeFirewall(s) not found for %s")
 
 				return nil
-			case err != nil:
+			default:
 				logger.Error(err, "Failed to get LinodeFirewalls")
 
 				return nil

internal/controller/linodeobjectstoragebucket_controller.go

@@ -44,7 +44,6 @@ import (
 	"github.com/linode/cluster-api-provider-linode/cloud/scope"
 	"github.com/linode/cluster-api-provider-linode/cloud/services"
 	wrappedruntimeclient "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimeclient"
-	wrappedruntimereconciler "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimereconciler"
 	"github.com/linode/cluster-api-provider-linode/util"
 	"github.com/linode/cluster-api-provider-linode/util/reconciler"
 )
@@ -76,20 +75,11 @@ type LinodeObjectStorageBucketReconciler struct {
 //
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.17.0/pkg/reconcile
-func (r *LinodeObjectStorageBucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+func (r *LinodeObjectStorageBucketReconciler) Reconcile(ctx context.Context, objectStorageBucket *infrav1alpha2.LinodeObjectStorageBucket) (ctrl.Result, error) {
 	ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultedLoopTimeout(r.ReconcileTimeout))
 	defer cancel()
 
-	logger := r.Logger.WithValues("name", req.String())
-
-	objectStorageBucket := &infrav1alpha2.LinodeObjectStorageBucket{}
-	if err := r.TracedClient().Get(ctx, req.NamespacedName, objectStorageBucket); err != nil {
-		if err = client.IgnoreNotFound(err); err != nil {
-			logger.Error(err, "Failed to fetch LinodeObjectStorageBucket", "name", req.String())
-		}
-
-		return ctrl.Result{}, err
-	}
+	logger := r.Logger.WithValues("name", objectStorageBucket.Name, "namespace", objectStorageBucket.Namespace)
 
 	if _, ok := objectStorageBucket.Labels[clusterv1.ClusterNameLabel]; ok {
 		cluster, err := kutil.GetClusterFromMetadata(ctx, r.TracedClient(), objectStorageBucket.ObjectMeta)
@@ -254,7 +244,7 @@ func (r *LinodeObjectStorageBucketReconciler) SetupWithManager(mgr ctrl.Manager,
 			&clusterv1.Cluster{},
 			handler.EnqueueRequestsFromMapFunc(linodeObjectStorageBucketMapper),
 			builder.WithPredicates(predicates.ClusterPausedTransitionsOrInfrastructureReady(mgr.GetScheme(), mgr.GetLogger())),
-		).Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
+		).Complete(reconciler.AsReconcilerWithTracing(r.TracedClient(), r))
 	if err != nil {
 		return fmt.Errorf("failed to build controller: %w", err)
 	}

internal/controller/linodeobjectstoragebucket_controller_test.go

@@ -37,6 +37,7 @@ import (
 	"github.com/linode/cluster-api-provider-linode/cloud/scope"
 	"github.com/linode/cluster-api-provider-linode/mock"
 	"github.com/linode/cluster-api-provider-linode/util"
+	rec "github.com/linode/cluster-api-provider-linode/util/reconciler"
 
 	. "github.com/linode/cluster-api-provider-linode/mock/mocktest"
 	. "github.com/onsi/ginkgo/v2"
@@ -329,7 +330,7 @@ var _ = Describe("errors", Label("bucket", "errors"), func() {
 				}),
 				Result("no error", func(ctx context.Context, mck Mock) {
 					reconciler.Client = mck.K8sClient
-					_, err := reconciler.Reconcile(ctx, reconcile.Request{
+					_, err := rec.AsReconcilerWithTracing(mck.K8sClient, &reconciler).Reconcile(ctx, reconcile.Request{
 						NamespacedName: client.ObjectKeyFromObject(bScope.Bucket),
 					})
 					Expect(err).NotTo(HaveOccurred())
@@ -342,18 +343,17 @@ var _ = Describe("errors", Label("bucket", "errors"), func() {
 				Result("error", func(ctx context.Context, mck Mock) {
 					reconciler.Client = mck.K8sClient
 					reconciler.Logger = bScope.Logger
-					_, err := reconciler.Reconcile(ctx, reconcile.Request{
+					_, err := rec.AsReconcilerWithTracing(mck.K8sClient, &reconciler).Reconcile(ctx, reconcile.Request{
 						NamespacedName: client.ObjectKeyFromObject(bScope.Bucket),
 					})
 					Expect(err.Error()).To(ContainSubstring("non-404 error"))
-					Expect(mck.Logs()).To(ContainSubstring("Failed to fetch LinodeObjectStorageBucket"))
 				}),
 			),
 		),
 		Result("scope params is missing args", func(ctx context.Context, mck Mock) {
 			reconciler.Client = mck.K8sClient
 			reconciler.Logger = bScope.Logger
-			_, err := reconciler.Reconcile(ctx, reconcile.Request{
+			_, err := rec.AsReconcilerWithTracing(mck.K8sClient, &reconciler).Reconcile(ctx, reconcile.Request{
 				NamespacedName: client.ObjectKeyFromObject(bScope.Bucket),
 			})
 			Expect(err.Error()).To(ContainSubstring("failed to create object storage bucket scope"))