clear failure message on complete reconciliation

controllers/cluster_controller.go

@@ -285,6 +285,19 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
 	// then return without any further processing.
 	if aggregatedGeneration == cluster.Status.ChildrenReconciledGeneration && cluster.Status.ReconciledGeneration == cluster.Generation {
 		log.Info("Generation and aggregated generation match reconciled generations for cluster and child objects, skipping reconciliation.")
+
+		// Failure messages are cleared in the reconciler loop after running validations. But sometimes,
+		// it seems that Cluster failure messages on the status are is not cleared for some reason
+		//  after successfully passing the validation. The theory is that if the inital patch operation
+		// is not successful, and the reconciliation is skipped going forward, it may never be cleared.
+		//
+		// When the controller reaches here, it denotes a completed reconcile. So, we can safely
+		// clear any failure messages or reasons that may be left over as there is no further processing
+		// for the controller to do.
+		if cluster.HasFailure() {
+			cluster.ClearFailure()
+		}
+
 		return ctrl.Result{}, nil
 	}
 

controllers/cluster_controller_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/aws/eks-anywhere/controllers/mocks"
 	"github.com/aws/eks-anywhere/internal/test"
 	"github.com/aws/eks-anywhere/internal/test/envtest"
+	"github.com/aws/eks-anywhere/pkg/api/v1alpha1"
 	anywherev1 "github.com/aws/eks-anywhere/pkg/api/v1alpha1"
 	"github.com/aws/eks-anywhere/pkg/cluster"
 	"github.com/aws/eks-anywhere/pkg/constants"
@@ -191,6 +192,57 @@ func TestClusterReconcilerReconcileSelfManagedCluster(t *testing.T) {
 	g.Expect(result).To(Equal(ctrl.Result{}))
 }
 
+func TestClusterReconcilerReconcileUnclearedClusterFailure(t *testing.T) {
+	config, bundles := baseTestVsphereCluster()
+	version := test.DevEksaVersion()
+	config.Cluster.Spec.EksaVersion = &version
+	config.Cluster.Generation = 1
+
+	config.Cluster.SetFailure(v1alpha1.FailureReasonType("InvalidCluster"), "invalid cluster")
+
+	kcp := testKubeadmControlPlaneFromCluster(config.Cluster)
+	machineDeployments := machineDeploymentsFromCluster(config.Cluster)
+
+	g := NewWithT(t)
+	ctx := context.Background()
+
+	objs := make([]runtime.Object, 0, 7+len(machineDeployments))
+	objs = append(objs, config.Cluster, bundles, test.EKSARelease())
+	for _, o := range config.ChildObjects() {
+		objs = append(objs, o)
+	}
+
+	objs = append(objs, kcp)
+
+	for _, obj := range machineDeployments {
+		objs = append(objs, obj.DeepCopy())
+	}
+
+	client := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build()
+	mockCtrl := gomock.NewController(t)
+	providerReconciler := mocks.NewMockProviderClusterReconciler(mockCtrl)
+	iam := mocks.NewMockAWSIamConfigReconciler(mockCtrl)
+	clusterValidator := mocks.NewMockClusterValidator(mockCtrl)
+	registry := newRegistryMock(providerReconciler)
+	mockPkgs := mocks.NewMockPackagesClient(mockCtrl)
+	mhcReconciler := mocks.NewMockMachineHealthCheckReconciler(mockCtrl)
+
+	providerReconciler.EXPECT().Reconcile(gomock.Any(), gomock.Any(), gomock.Any()).Times(0)
+
+	r := controllers.NewClusterReconciler(client, registry, iam, clusterValidator, mockPkgs, mhcReconciler)
+
+	result, err := r.Reconcile(ctx, clusterRequest(config.Cluster))
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(result).To(Equal(ctrl.Result{}))
+
+	api := envtest.NewAPIExpecter(t, client)
+	c := envtest.CloneNameNamespace(config.Cluster)
+	api.ShouldEventuallyMatch(ctx, c, func(g Gomega) {
+		g.Expect(c.Status.FailureMessage).To(BeNil())
+		g.Expect(c.Status.FailureReason).To(BeNil())
+	})
+}
+
 func TestClusterReconcilerReconcileConditions(t *testing.T) {
 	testCases := []struct {
 		testName                string

pkg/api/v1alpha1/cluster_types.go

@@ -1445,6 +1445,11 @@ func (c *Cluster) ClearFailure() {
 	c.Status.FailureReason = nil
 }
 
+// HasFailure checks whether there is a failureMessage and/or failureReason set on the Cluster status.
+func (c *Cluster) HasFailure() bool {
+	return c.Status.FailureMessage != nil || c.Status.FailureReason != nil
+}
+
 // KubernetesVersions returns a set of all unique k8s versions specified in the cluster
 // for both CP and workers.
 func (c *Cluster) KubernetesVersions() []KubernetesVersion {

pkg/api/v1alpha1/cluster_types_test.go

@@ -2889,6 +2889,64 @@ func TestCluster_ClearFailure(t *testing.T) {
 	g.Expect(cluster.Status.FailureReason).To(BeNil())
 }
 
+func TestCluster_HasFailure(t *testing.T) {
+	g := NewWithT(t)
+	wantFailureMessage := "invalid cluster"
+	wantFailureReason := v1alpha1.FailureReasonType("InvalidCluster")
+
+	tests := []struct {
+		name    string
+		cluster *v1alpha1.Cluster
+		want    bool
+	}{
+		{
+			name: "failureReason and failureMessage set",
+			cluster: &v1alpha1.Cluster{
+				Status: v1alpha1.ClusterStatus{
+					FailureMessage: &wantFailureMessage,
+					FailureReason:  &wantFailureReason,
+				},
+			},
+			want: true,
+		},
+		{
+			name: "failureMessage only set",
+			cluster: &v1alpha1.Cluster{
+				Status: v1alpha1.ClusterStatus{
+					FailureMessage: &wantFailureMessage,
+					FailureReason:  nil,
+				},
+			},
+			want: true,
+		},
+		{
+			name: "failureReason only set",
+			cluster: &v1alpha1.Cluster{
+				Status: v1alpha1.ClusterStatus{
+					FailureMessage: nil,
+					FailureReason:  &wantFailureReason,
+				},
+			},
+			want: true,
+		},
+		{
+			name: "no failure",
+			cluster: &v1alpha1.Cluster{
+				Status: v1alpha1.ClusterStatus{
+					FailureMessage: nil,
+					FailureReason:  &wantFailureReason,
+				},
+			},
+			want: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g.Expect(tt.cluster.HasFailure()).To(Equal(tt.want))
+		})
+	}
+}
+
 func TestClusterDisableControlPlaneIPCheck(t *testing.T) {
 	tests := []struct {
 		name    string