OCPBUGSM-27380 Fix confusing host registration error (#1590)

openshift · Apr 28, 2021 · d10163a · d10163a
1 parent a168981
commit d10163a
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 16 deletions.
diff --git a/internal/bminventory/inventory.go b/internal/bminventory/inventory.go
@@ -2406,7 +2406,7 @@ func (b *bareMetalInventory) RegisterHost(ctx context.Context, params installer.
 			log.WithError(err).Errorf("failed to register host <%s> to cluster %s due to: %s",
 				params.NewHostParams.HostID, params.ClusterID.String(), err.Error())
 			b.eventsHandler.AddEvent(ctx, params.ClusterID, params.NewHostParams.HostID, models.EventSeverityError,
-				"Failed to register host: cluster cannot accept new hosts in its current state", time.Now())
+				err.Error(), time.Now())
 			return common.NewApiError(http.StatusConflict, err)
 		}
 	}

diff --git a/internal/cluster/cluster.go b/internal/cluster/cluster.go
@@ -613,7 +613,11 @@ func (m *Manager) AcceptRegistration(c *common.Cluster) (err error) {
 	clusterStatus := swag.StringValue(c.Status)
 	allowedStatuses := []string{models.ClusterStatusInsufficient, models.ClusterStatusReady, models.ClusterStatusPendingForInput, models.ClusterStatusAddingHosts}
 	if !funk.ContainsString(allowedStatuses, clusterStatus) {
-		err = errors.Errorf("Cluster %s is in %s state, host can register only in one of %s", c.ID, clusterStatus, allowedStatuses)
+		if clusterStatus == models.ClusterStatusInstalled {
+			err = errors.Errorf("Cannot add host to a cluster that is already installed, please use the day2 cluster option")
+		} else {
+			err = errors.Errorf("Host can register only in one of the following states: %s", allowedStatuses)
+		}
 	}
 	return err
 }

diff --git a/internal/cluster/cluster_test.go b/internal/cluster/cluster_test.go
@@ -1104,11 +1104,12 @@ var _ = Describe("Auto assign machine CIDR", func() {
 
 var _ = Describe("VerifyRegisterHost", func() {
 	var (
-		db          *gorm.DB
-		id          strfmt.UUID
-		clusterApi  *Manager
-		errTemplate = "Cluster %s is in %s state, host can register only in one of [insufficient ready pending-for-input adding-hosts]"
-		dbName      string
+		db                 *gorm.DB
+		id                 strfmt.UUID
+		clusterApi         *Manager
+		preInstalledError  string = "Host can register only in one of the following states: [insufficient ready pending-for-input adding-hosts]"
+		postInstalledError string = "Cannot add host to a cluster that is already installed, please use the day2 cluster option"
+		dbName             string
 	)
 
 	BeforeEach(func() {
@@ -1121,35 +1122,35 @@ var _ = Describe("VerifyRegisterHost", func() {
 			nil, nil, nil, nil, dummy, mockOperators, nil, nil, nil)
 	})
 
-	checkVerifyRegisterHost := func(clusterStatus string, expectErr bool) {
+	checkVerifyRegisterHost := func(clusterStatus string, expectErr bool, errTemplate string) {
 		cluster := common.Cluster{Cluster: models.Cluster{ID: &id, Status: swag.String(clusterStatus)}}
 		Expect(db.Create(&cluster).Error).ShouldNot(HaveOccurred())
 		cluster = getClusterFromDB(id, db)
 		err := clusterApi.AcceptRegistration(&cluster)
 		if expectErr {
-			Expect(err.Error()).Should(Equal(errors.Errorf(errTemplate, id, clusterStatus).Error()))
+			Expect(err.Error()).Should(Equal(errors.Errorf(errTemplate).Error()))
 		} else {
 			Expect(err).Should(BeNil())
 		}
 	}
 	It("Register host while cluster in ready state", func() {
-		checkVerifyRegisterHost(models.ClusterStatusReady, false)
+		checkVerifyRegisterHost(models.ClusterStatusReady, false, preInstalledError)
 	})
 	It("Register host while cluster in insufficient state", func() {
-		checkVerifyRegisterHost(models.ClusterStatusInsufficient, false)
+		checkVerifyRegisterHost(models.ClusterStatusInsufficient, false, preInstalledError)
 	})
 	It("Register host while cluster in installing state", func() {
-		checkVerifyRegisterHost(models.ClusterStatusInstalling, true)
+		checkVerifyRegisterHost(models.ClusterStatusInstalling, true, preInstalledError)
 	})
-	It("Register host while cluster in installing state", func() {
-		checkVerifyRegisterHost(models.ClusterStatusFinalizing, true)
+	It("Register host while cluster in finallizing state", func() {
+		checkVerifyRegisterHost(models.ClusterStatusFinalizing, true, preInstalledError)
 	})
 	It("Register host while cluster in error state", func() {
-		checkVerifyRegisterHost(models.ClusterStatusError, true)
+		checkVerifyRegisterHost(models.ClusterStatusError, true, preInstalledError)
 	})
 
 	It("Register host while cluster in installed state", func() {
-		checkVerifyRegisterHost(models.ClusterStatusInstalled, true)
+		checkVerifyRegisterHost(models.ClusterStatusInstalled, true, postInstalledError)
 	})
 	AfterEach(func() {
 		common.DeleteTestDB(db, dbName)