Remove old nodes from cluster status

Author: AMecea

hack/upgrades-tests/test_upgrade_v0.3.0.sh

@@ -208,7 +208,7 @@ function cmd_teardown {
 function cmd_test {
 
     # operator install
-    install_operator 0.2.7 presslabs/mysql-operator
+    install_operator 0.2.10 presslabs/mysql-operator
 
     # install a cluster with 2 replica
     CL_NAME=my-cluster

pkg/controller/mysqlcluster/internal/upgrades/upgrades.go

@@ -181,7 +181,7 @@ func (u *upgrader) checkNode0Ok(insts []orc.Instance) error {
 	node0 := u.getNodeFrom(insts, 0)
 	if node0 == nil {
 		// continue
-		log.Info("no node found in orchestraotr")
+		log.Info("no node found in orchestrator")
 		return fmt.Errorf("node-0 not found in orchestarotr")
 	}
 

pkg/controller/orchestrator/orchestrator_reconcile.go

@@ -436,8 +436,12 @@ func (ou *orcUpdater) removeNodeConditionNotInOrc(insts InstancesSet) {
 	// remove nodes status for nodes that are not desired, nodes that are left behind from scale down
 	validIndex := 0
 	for _, ns := range ou.cluster.Status.Nodes {
-		// save only the nodes that are desired [0, 1, ..., replicas-1]
-		if indexInSts(ns.Name) < *ou.cluster.Spec.Replicas {
+		// save only the nodes that are desired [0, 1, ..., replicas-1] or if index can't be extracted
+		index, err := indexInSts(ns.Name)
+		if err != nil {
+			log.Info("failed to parse hostname for index - won't be removed", "error", err)
+		}
+		if index < *ou.cluster.Spec.Replicas || err != nil {
 			ou.cluster.Status.Nodes[validIndex] = ns
 			validIndex++
 		}
@@ -448,12 +452,15 @@ func (ou *orcUpdater) removeNodeConditionNotInOrc(insts InstancesSet) {
 }
 
 // indexInSts is a helper function that returns the index of the pod in statefulset
-func indexInSts(name string) int32 {
-	re := regexp.MustCompile(`^[\w-]+-mysql-(\d*)\.mysql\.[\w-]+$`)
+func indexInSts(name string) (int32, error) {
+	re := regexp.MustCompile(`^[\w-]+-mysql-(\d*)\.[\w-]*mysql(?:-nodes)?\.[\w-]+$`)
 	values := re.FindStringSubmatch(name)
+	if len(values) != 2 {
+		return 0, fmt.Errorf("no match found")
+	}
 
-	i, _ := strconv.Atoi(values[1])
-	return int32(i)
+	i, err := strconv.Atoi(values[1])
+	return int32(i), err
 }
 
 // set a host writable just if needed

pkg/controller/orchestrator/orchestrator_reconcile_test.go

@@ -435,6 +435,18 @@ var _ = Describe("Orchestrator reconciler", func() {
 			Expect(insts).To(HaveLen(1))
 		})
 
+		It("should remove nodes from cluster status at scale down", func() {
+			// scale down the cluster
+			cluster.Spec.Replicas = &one
+			cluster.Status.ReadyNodes = 1
+
+			// call the function
+			insts, _ := orcClient.Cluster(cluster.GetClusterAlias())
+			updater.removeNodeConditionNotInOrc(insts)
+
+			Expect(cluster.Status.Nodes).To(ConsistOf(hasNodeWithStatus(cluster.GetPodHostname(0))))
+		})
+
 		When("cluster is not ready", func() {
 			BeforeEach(func() {
 				cluster.Spec.Replicas = &two
@@ -488,6 +500,53 @@ var _ = Describe("Orchestrator reconciler", func() {
 		})
 
 	})
+
+	// NOTE: this test sute should be deleted in next major version
+	Describe("status updater unit tests at upgrade", func() {
+		var (
+			updater *orcUpdater
+		)
+
+		BeforeEach(func() {
+			updater = &orcUpdater{
+				cluster:   cluster,
+				recorder:  rec,
+				orcClient: orcClient,
+			}
+			// set cluster on readonly, master should be in read only state
+			orcClient.AddInstance(orc.Instance{
+				ClusterName: cluster.GetClusterAlias(),
+				Key:         orc.InstanceKey{Hostname: oldPodHostname(cluster, 0)},
+				ReadOnly:    false, // mark node as master
+				// mark instance as uptodate
+				IsUpToDate:       true,
+				IsLastCheckValid: true,
+			})
+			orcClient.AddInstance(orc.Instance{
+				ClusterName: cluster.GetClusterAlias(),
+				Key:         orc.InstanceKey{Hostname: oldPodHostname(cluster, 1)},
+				MasterKey:   orc.InstanceKey{Hostname: oldPodHostname(cluster, 0)},
+			})
+
+			// update cluster nodes status
+			insts, _ := orcClient.Cluster(cluster.GetClusterAlias())
+			master, _ := orcClient.Master(cluster.GetClusterAlias())
+			updater.updateStatusFromOrc(insts, master)
+		})
+
+		It("should not remove nodes from cluster when upgrading", func() {
+			// scale down the cluster
+			cluster.Spec.Replicas = &one
+			cluster.Status.ReadyNodes = 1
+
+			// call the function
+			insts, _ := orcClient.Cluster(cluster.GetClusterAlias())
+			updater.removeNodeConditionNotInOrc(insts)
+
+			// nothing should be remove
+			Expect(cluster.Status.Nodes).To(ConsistOf(hasNodeWithStatus(oldPodHostname(cluster, 0))))
+		})
+	})
 })
 
 // haveNodeCondWithStatus is a helper func that returns a matcher to check for an existing condition in a ClusterCondition list.
@@ -510,5 +569,15 @@ func haveCondWithStatus(condType api.ClusterConditionType, status core.Condition
 			"Reason": Equal(reason),
 		})),
 	})
+}
+
+func hasNodeWithStatus(host string) gomegatypes.GomegaMatcher {
+	return MatchFields(IgnoreExtras, Fields{
+		"Name": Equal(host),
+	})
+}
 
+func oldPodHostname(cluster *mysqlcluster.MysqlCluster, index int) string {
+	return fmt.Sprintf("%s-mysql-%d.%s.%s", cluster.Name, index, cluster.GetNameForResource(mysqlcluster.OldHeadlessSVC),
+		cluster.Namespace)
 }