Repository: Azure/ARO-RP
Pull request #2551 (2022-11-21 08:14:29)

Hive: exit if we enter ProvisionFailed status
### Which issue this PR addresses:

Fixes https://dev.azure.com/msazure/AzureRedHatOpenShift/_workitems/edit/15728844

### What this PR does / why we need it:

Currently the Hive manager is only checking for `spec.Installed`, but this does not check for any failure conditions and results in all failures taking 60 minutes to return. This PR adds a single failure condition, Provision Failed, but scaffolding has been put in place making it trivial to add any additional error conditions in the future.

### Test plan for issue:

Tested cluster installation failure propagation in a dedicated RP instance.

### Is there any documentation that needs to be updated for this PR?

No.



Patch:
diff --git a/pkg/hive/manager.go b/pkg/hive/manager.go
index 911db8dbf..3c88440c0 100644
--- a/pkg/hive/manager.go
+++ b/pkg/hive/manager.go
@@ -5,6 +5,7 @@ package hive
 
 import (
 	"context"
+	"fmt"
 
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	hiveclient "github.com/openshift/hive/pkg/client/clientset/versioned"
@@ -160,10 +161,26 @@ func (hr *clusterManager) IsClusterDeploymentReady(ctx context.Context, doc *api
 
 func (hr *clusterManager) IsClusterInstallationComplete(ctx context.Context, doc *api.OpenShiftClusterDocument) (bool, error) {
 	cd, err := hr.hiveClientset.HiveV1().ClusterDeployments(doc.OpenShiftCluster.Properties.HiveProfile.Namespace).Get(ctx, ClusterDeploymentName, metav1.GetOptions{})
-	if err == nil {
-		return cd.Spec.Installed, nil
+	if err != nil {
+		return false, err
+	}
+
+	if cd.Spec.Installed {
+		return true, nil
+	}
+
+	checkFailureConditions := map[hivev1.ClusterDeploymentConditionType]corev1.ConditionStatus{
+		hivev1.ProvisionFailedCondition: corev1.ConditionTrue,
 	}
-	return false, err
+
+	for _, cond := range cd.Status.Conditions {
+		conditionStatus, found := checkFailureConditions[cond.Type]
+		if found && conditionStatus == cond.Status {
+			return false, fmt.Errorf("clusterdeployment has failed: %s == %s", cond.Type, cond.Status)
+		}
+	}
+
+	return false, nil
 }
 
 func (hr *clusterManager) ResetCorrelationData(ctx context.Context, doc *api.OpenShiftClusterDocument) error {


Test patch:
diff --git a/pkg/hive/manager_test.go b/pkg/hive/manager_test.go
index 67332db50..01e93757a 100644
--- a/pkg/hive/manager_test.go
+++ b/pkg/hive/manager_test.go
@@ -169,6 +169,109 @@ func TestIsClusterDeploymentReady(t *testing.T) {
 	}
 }
 
+func TestIsClusterInstallationComplete(t *testing.T) {
+	fakeNamespace := "fake-namespace"
+	doc := &api.OpenShiftClusterDocument{
+		OpenShiftCluster: &api.OpenShiftCluster{
+			Properties: api.OpenShiftClusterProperties{
+				HiveProfile: api.HiveProfile{
+					Namespace: fakeNamespace,
+				},
+			},
+		},
+	}
+
+	for _, tt := range []struct {
+		name       string
+		cd         kruntime.Object
+		wantResult bool
+		wantErr    string
+	}{
+		{
+			name: "is installed",
+			cd: &hivev1.ClusterDeployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      ClusterDeploymentName,
+					Namespace: fakeNamespace,
+				},
+				Spec: hivev1.ClusterDeploymentSpec{
+					Installed: true,
+				},
+				Status: hivev1.ClusterDeploymentStatus{
+					Conditions: []hivev1.ClusterDeploymentCondition{
+						{
+							Type:   hivev1.ProvisionFailedCondition,
+							Status: corev1.ConditionFalse,
+						},
+					},
+				},
+			},
+			wantResult: true,
+		},
+		{
+			name: "is not installed yet",
+			cd: &hivev1.ClusterDeployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      ClusterDeploymentName,
+					Namespace: fakeNamespace,
+				},
+				Spec: hivev1.ClusterDeploymentSpec{
+					Installed: false,
+				},
+				Status: hivev1.ClusterDeploymentStatus{
+					Conditions: []hivev1.ClusterDeploymentCondition{
+						{
+							Type:   hivev1.ProvisionFailedCondition,
+							Status: corev1.ConditionFalse,
+						},
+					},
+				},
+			},
+			wantResult: false,
+		},
+		{
+			name: "has failed provisioning",
+			cd: &hivev1.ClusterDeployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      ClusterDeploymentName,
+					Namespace: fakeNamespace,
+				},
+				Status: hivev1.ClusterDeploymentStatus{
+					Conditions: []hivev1.ClusterDeploymentCondition{
+						{
+							Type:   hivev1.ProvisionFailedCondition,
+							Status: corev1.ConditionTrue,
+						},
+					},
+				},
+			},
+			wantErr:    "clusterdeployment has failed: ProvisionFailed == True",
+			wantResult: false,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			fakeClientset := hivefake.NewSimpleClientset()
+			if tt.cd != nil {
+				fakeClientset.Tracker().Add(tt.cd)
+			}
+			c := clusterManager{
+				hiveClientset: fakeClientset,
+				log:           logrus.NewEntry(logrus.StandardLogger()),
+			}
+
+			result, err := c.IsClusterInstallationComplete(context.Background(), doc)
+			if err != nil && err.Error() != tt.wantErr ||
+				err == nil && tt.wantErr != "" {
+				t.Error(err)
+			}
+
+			if tt.wantResult != result {
+				t.Error(result)
+			}
+		})
+	}
+}
+
 func TestResetCorrelationData(t *testing.T) {
 	fakeNamespace := "fake-namespace"
 	doc := &api.OpenShiftClusterDocument{