Merge pull request #275 from arangodb/bug-fix/scale-up-error

Bug fix/scale up error
arangodb · Oct 26, 2018 · a6d511b · a6d511b
2 parents 179cf75 + f46b375
commit a6d511b
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 9 deletions.
diff --git a/pkg/apis/deployment/v1alpha/member_status_list.go b/pkg/apis/deployment/v1alpha/member_status_list.go
@@ -122,7 +122,7 @@ func (l MemberStatusList) SelectMemberToRemove() (MemberStatus, error) {
 	if len(l) > 0 {
 		// Try to find a not ready member
 		for _, m := range l {
-			if m.Phase == MemberPhaseNone {
+			if m.Phase == MemberPhaseNone || !m.Conditions.IsTrue(ConditionTypeReady) {
 				return m, nil
 			}
 		}

diff --git a/pkg/deployment/cluster_scaling_integration.go b/pkg/deployment/cluster_scaling_integration.go
@@ -192,11 +192,19 @@ func (ci *clusterScalingIntegration) updateClusterServerCount(ctx context.Contex
 	}
 	coordinatorCount := spec.Coordinators.GetCount()
 	dbserverCount := spec.DBServers.GetCount()
-	if err := arangod.SetNumberOfServers(ctx, c.Connection(), coordinatorCount, dbserverCount); err != nil {
-		if expectSuccess {
-			log.Debug().Err(err).Msg("Failed to set number of servers")
+
+	ci.lastNumberOfServers.mutex.Lock()
+	lastNumberOfServers := ci.lastNumberOfServers.NumberOfServers
+	ci.lastNumberOfServers.mutex.Unlock()
+
+	// This is to prevent unneseccary updates that may override some values written by the WebUI (in the case of a update loop)
+	if coordinatorCount != lastNumberOfServers.GetCoordinators() && dbserverCount != lastNumberOfServers.GetDBServers() {
+		if err := arangod.SetNumberOfServers(ctx, c.Connection(), coordinatorCount, dbserverCount); err != nil {
+			if expectSuccess {
+				log.Debug().Err(err).Msg("Failed to set number of servers")
+			}
+			return false, maskAny(err)
 		}
-		return false, maskAny(err)
 	}
 
 	// Success, now update internal state

diff --git a/pkg/deployment/reconcile/plan_builder.go b/pkg/deployment/reconcile/plan_builder.go
@@ -366,13 +366,22 @@ func createScalePlan(log zerolog.Logger, members api.MemberStatusList, group api
 		if m, err := members.SelectMemberToRemove(); err != nil {
 			log.Warn().Err(err).Str("role", group.AsRole()).Msg("Failed to select member to remove")
 		} else {
-			if group == api.ServerGroupDBServers {
+
+			log.Debug().
+				Str("member-id", m.ID).
+				Str("phase", string(m.Phase)).
+				Msg("Found member to remove")
+			if m.Conditions.IsTrue(api.ConditionTypeReady) {
+				if group == api.ServerGroupDBServers {
+					plan = append(plan,
+						api.NewAction(api.ActionTypeCleanOutMember, group, m.ID),
+					)
+				}
 				plan = append(plan,
-					api.NewAction(api.ActionTypeCleanOutMember, group, m.ID),
+					api.NewAction(api.ActionTypeShutdownMember, group, m.ID),
 				)
 			}
 			plan = append(plan,
-				api.NewAction(api.ActionTypeShutdownMember, group, m.ID),
 				api.NewAction(api.ActionTypeRemoveMember, group, m.ID),
 			)
 			log.Debug().

diff --git a/pkg/deployment/resources/secret_hashes.go b/pkg/deployment/resources/secret_hashes.go
@@ -38,7 +38,7 @@ import (
 // ValidateSecretHashes checks the hash of used secrets
 // against the stored ones.
 // If a hash is different, the deployment is marked
-// with a SecretChangedCondition and the operator will no
+// with a SecretChangedCondition and the operator will not
 // touch it until this is resolved.
 func (r *Resources) ValidateSecretHashes() error {
 	// validate performs a secret hash comparison for a single secret.