refactor: remove old setup logic for AWSCluster

AWSCluster was not reconciling when starting after an upgrade. It had
old logic to compare versions and not do anything. We want to reconcile
even if there are no changes to the AWSCluster as the ELB logic has
changed. Also, there may be other changes like this in future.

Change the SetupWithManager logic to be more like the standard we see
with other infrastructure providers.

Signed-off-by: Richard Case <[email protected]>

Author: richardcase

controllers/awscluster_controller.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
@@ -32,9 +31,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
@@ -393,28 +390,6 @@ func (r *AWSClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Ma
 		WithOptions(options).
 		For(&infrav1.AWSCluster{}).
 		WithEventFilter(predicates.ResourceHasFilterLabel(mgr.GetScheme(), log.GetLogger(), r.WatchFilterValue)).
-		WithEventFilter(
-			predicate.Funcs{
-				// Avoid reconciling if the event triggering the reconciliation is related to incremental status updates
-				// for AWSCluster resources only
-				UpdateFunc: func(e event.UpdateEvent) bool {
-					if e.ObjectOld.GetObjectKind().GroupVersionKind().Kind != "AWSCluster" {
-						return true
-					}
-
-					oldCluster := e.ObjectOld.(*infrav1.AWSCluster).DeepCopy()
-					newCluster := e.ObjectNew.(*infrav1.AWSCluster).DeepCopy()
-
-					oldCluster.Status = infrav1.AWSClusterStatus{}
-					newCluster.Status = infrav1.AWSClusterStatus{}
-
-					oldCluster.ObjectMeta.ResourceVersion = ""
-					newCluster.ObjectMeta.ResourceVersion = ""
-
-					return !cmp.Equal(oldCluster, newCluster)
-				},
-			},
-		).
 		WithEventFilter(predicates.ResourceIsNotExternallyManaged(mgr.GetScheme(), log.GetLogger())).
 		Build(r)
 	if err != nil {

controllers/awscluster_controller_unit_test.go

@@ -28,19 +28,16 @@ import (
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/mock_services"
-	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
 )
@@ -60,7 +57,8 @@ func TestAWSClusterReconcilerReconcile(t *testing.T) {
 					Kind:       "Cluster",
 					Name:       "capi-fail-test",
 					UID:        "1",
-				}}}},
+				},
+			}}},
 			expectError: true,
 		},
 		{
@@ -532,97 +530,6 @@ func TestAWSClusterReconcileOperations(t *testing.T) {
 	})
 }
 
-func TestAWSClusterReconcilerRequeueAWSClusterForUnpausedCluster(t *testing.T) {
-	testCases := []struct {
-		name         string
-		awsCluster   *infrav1.AWSCluster
-		ownerCluster *clusterv1.Cluster
-		requeue      bool
-	}{
-		{
-			name: "Should create reconcile request successfully",
-			awsCluster: &infrav1.AWSCluster{
-				ObjectMeta: metav1.ObjectMeta{GenerateName: "aws-test-"}, TypeMeta: metav1.TypeMeta{Kind: "AWSCluster", APIVersion: infrav1.GroupVersion.String()},
-			},
-			ownerCluster: &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "capi-test"}},
-			requeue:      true,
-		},
-		{
-			name: "Should not create reconcile request if AWSCluster is externally managed",
-			awsCluster: &infrav1.AWSCluster{
-				ObjectMeta: metav1.ObjectMeta{GenerateName: "aws-test-", Annotations: map[string]string{clusterv1.ManagedByAnnotation: "capi-test"}},
-				TypeMeta:   metav1.TypeMeta{Kind: "AWSCluster", APIVersion: infrav1.GroupVersion.String()},
-			},
-			ownerCluster: &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "capi-test"}},
-			requeue:      false,
-		},
-		{
-			name:         "Should not create reconcile request for deleted clusters",
-			ownerCluster: &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "capi-test", DeletionTimestamp: &metav1.Time{Time: time.Now()}}},
-			requeue:      false,
-		},
-		{
-			name:         "Should not create reconcile request if infrastructure ref for AWSCluster on owner cluster is not set",
-			ownerCluster: &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "capi-test"}},
-			requeue:      false,
-		},
-		{
-			name: "Should not create reconcile request if infrastructure ref type on owner cluster is not AWSCluster",
-			ownerCluster: &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "capi-test"}, Spec: clusterv1.ClusterSpec{InfrastructureRef: &corev1.ObjectReference{
-				APIVersion: clusterv1.GroupVersion.String(),
-				Kind:       "Cluster",
-				Name:       "aws-test"}}},
-			requeue: false,
-		},
-		{
-			name: "Should not create reconcile request if AWSCluster not found",
-			ownerCluster: &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "capi-test"}, Spec: clusterv1.ClusterSpec{InfrastructureRef: &corev1.ObjectReference{
-				APIVersion: clusterv1.GroupVersion.String(),
-				Kind:       "AWSCluster",
-				Name:       "aws-test"}}},
-			requeue: false,
-		},
-	}
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			g := NewWithT(t)
-			log := logger.FromContext(ctx)
-			reconciler := &AWSClusterReconciler{
-				Client: testEnv.Client,
-			}
-
-			ns, err := testEnv.CreateNamespace(ctx, fmt.Sprintf("namespace-%s", util.RandomString(5)))
-			g.Expect(err).To(BeNil())
-			createCluster(g, tc.awsCluster, ns.Name)
-			defer cleanupCluster(g, tc.awsCluster, ns)
-
-			if tc.ownerCluster != nil {
-				if tc.awsCluster != nil {
-					tc.ownerCluster.Spec = clusterv1.ClusterSpec{InfrastructureRef: &corev1.ObjectReference{
-						APIVersion: infrav1.GroupVersion.String(),
-						Kind:       "AWSCluster",
-						Name:       tc.awsCluster.Name,
-						Namespace:  ns.Name,
-					}}
-				}
-				tc.ownerCluster.Namespace = ns.Name
-			}
-			handlerFunc := reconciler.requeueAWSClusterForUnpausedCluster(ctx, log)
-			result := handlerFunc(ctx, tc.ownerCluster)
-			if tc.requeue {
-				g.Expect(result).To(ContainElement(reconcile.Request{
-					NamespacedName: types.NamespacedName{
-						Namespace: ns.Name,
-						Name:      tc.awsCluster.Name,
-					},
-				}))
-			} else {
-				g.Expect(result).To(BeNil())
-			}
-		})
-	}
-}
-
 func createCluster(g *WithT, awsCluster *infrav1.AWSCluster, namespace string) {
 	if awsCluster != nil {
 		awsCluster.Namespace = namespace

main.go

@@ -128,6 +128,7 @@ var (
 // +kubebuilder:rbac:groups=authorization.k8s.io,resources=subjectaccessreviews,verbs=create
 
 func main() {
+	setupLog.Info("starting cluster-api-provider-aws", "version", version.Get().String())
 	initFlags(pflag.CommandLine)
 	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
 	pflag.Parse()

test/e2e/shared/aws_helpers.go

@@ -196,7 +196,7 @@ func CheckClassicElbHealthCheck(input CheckClassicElbHealthCheckInput, intervals
 		}
 
 		if *lb.HealthCheck.Target != input.ExpectedTarget {
-			return fmt.Errorf("health check target %s does not match expected target %s", *lb.HealthCheck.Target, input.ExpectedTarget)
+			return fmt.Errorf("health check target %q does not match expected target %q", *lb.HealthCheck.Target, input.ExpectedTarget)
 		}
 
 		return nil