Merge remote-tracking branch 'origin/master' into bug-fix/readiness-upgrade-fix

Author: lamai93

dashboard/package-lock.json

@@ -3,14 +3,14 @@
   "version": "0.1.0",
   "private": true,
   "dependencies": {
-    "emotion": "^9.2.4",
-    "react": "^16.4.1",
+    "emotion": "^9.2.12",
+    "react": "^16.6.0",
     "react-copy-to-clipboard": "^5.0.1",
-    "react-dom": "^16.4.1",
-    "react-emotion": "^9.2.4",
+    "react-dom": "^16.6.0",
+    "react-emotion": "^9.2.12",
     "react-router-dom": "^4.3.1",
-    "react-timeout": "^1.1.1",
-    "semantic-ui-less": "^2.2.12",
+    "react-timeout": "^1.1.2",
+    "semantic-ui-less": "^2.4.1",
     "semantic-ui-react": "^0.81.3"
   },
   "devDependencies": {

dashboard/package.json

@@ -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
 			}
 		}

pkg/apis/deployment/v1alpha/member_status_list.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

pkg/deployment/cluster_scaling_integration.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().

pkg/deployment/reconcile/plan_builder.go

@@ -176,11 +176,9 @@ func TestCreatePlanActiveFailoverScale(t *testing.T) {
 	}
 	newPlan, changed = createPlan(log, depl, nil, spec, status, nil, c)
 	assert.True(t, changed)
-	require.Len(t, newPlan, 2) // Note: Downscaling is only down 1 at a time
-	assert.Equal(t, api.ActionTypeShutdownMember, newPlan[0].Type)
-	assert.Equal(t, api.ActionTypeRemoveMember, newPlan[1].Type)
+	require.Len(t, newPlan, 1) // Note: Downscaling is only down 1 at a time
+	assert.Equal(t, api.ActionTypeRemoveMember, newPlan[0].Type)
 	assert.Equal(t, api.ServerGroupSingle, newPlan[0].Group)
-	assert.Equal(t, api.ServerGroupSingle, newPlan[1].Group)
 }
 
 // TestCreatePlanClusterScale creates a `cluster` deployment to test the creating of scaling plan.
@@ -263,6 +261,12 @@ func TestCreatePlanClusterScale(t *testing.T) {
 		api.MemberStatus{
 			ID:      "cr1",
 			PodName: "coordinator1",
+			Conditions: api.ConditionList{
+				api.Condition{
+					Type: api.ConditionTypeReady,
+					Status: v1.ConditionTrue,
+				},
+			},
 		},
 		api.MemberStatus{
 			ID:      "cr2",
@@ -273,15 +277,14 @@ func TestCreatePlanClusterScale(t *testing.T) {
 	spec.Coordinators.Count = util.NewInt(1)
 	newPlan, changed = createPlan(log, depl, nil, spec, status, nil, c)
 	assert.True(t, changed)
-	require.Len(t, newPlan, 5) // Note: Downscaling is done 1 at a time
-	assert.Equal(t, api.ActionTypeCleanOutMember, newPlan[0].Type)
+
+	fmt.Printf("%v", newPlan)
+
+	require.Len(t, newPlan, 3) // Note: Downscaling is done 1 at a time
+	assert.Equal(t, api.ActionTypeRemoveMember, newPlan[0].Type)
 	assert.Equal(t, api.ActionTypeShutdownMember, newPlan[1].Type)
 	assert.Equal(t, api.ActionTypeRemoveMember, newPlan[2].Type)
-	assert.Equal(t, api.ActionTypeShutdownMember, newPlan[3].Type)
-	assert.Equal(t, api.ActionTypeRemoveMember, newPlan[4].Type)
 	assert.Equal(t, api.ServerGroupDBServers, newPlan[0].Group)
-	assert.Equal(t, api.ServerGroupDBServers, newPlan[1].Group)
-	assert.Equal(t, api.ServerGroupDBServers, newPlan[2].Group)
-	assert.Equal(t, api.ServerGroupCoordinators, newPlan[3].Group)
-	assert.Equal(t, api.ServerGroupCoordinators, newPlan[4].Group)
+	assert.Equal(t, api.ServerGroupCoordinators, newPlan[1].Group)
+	assert.Equal(t, api.ServerGroupCoordinators, newPlan[2].Group)
 }

pkg/deployment/reconcile/plan_builder_test.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.