PWX-37204 , PWX- 37178 : Resolving gaps in KVDB fix (#1542)

* check kvdb nodes only on fresh install

* correcting UTs

* making requested changes

* correcting UT

Author: svijaykumar-px

pkg/controller/storagecluster/controller_test.go

@@ -3886,6 +3886,7 @@ func TestUpdateStorageClusterWithKVDBDown(t *testing.T) {
 	cluster.Spec.Kvdb = &corev1.KvdbSpec{
 		Internal: true,
 	}
+	cluster.Status.Phase = "Online"
 	k8sVersion, _ := version.NewVersion(minSupportedK8sVersion)
 	storageLabels := map[string]string{
 		constants.LabelKeyClusterName: cluster.Name,
@@ -3992,6 +3993,12 @@ func TestUpdateStorageClusterWithKVDBDown(t *testing.T) {
 	require.Contains(t, err.Error(), fmt.Sprintf("couldn't get unavailable numbers: couldn't get list of storage nodes during rolling update of storage cluster %s/%s: %s", cluster.Namespace, cluster.Name, getStorageNodeserr))
 
 	// When GetKvdbMembers returns an error
+	err = testutil.Get(k8sClient, cluster, cluster.Name, cluster.Namespace)
+	require.NoError(t, err)
+	cluster.Status.Phase = "Online"
+	err = k8sClient.Update(context.TODO(), cluster)
+	require.NoError(t, err)
+
 	getKvdbMemberserr := fmt.Errorf("test error 2")
 	driver.EXPECT().GetKVDBMembers(gomock.Any()).Return(nil, getKvdbMemberserr).Times(1)
 	driver.EXPECT().GetStorageNodes(gomock.Any()).Return(storageNodes, nil).AnyTimes()

pkg/controller/storagecluster/update.go

@@ -81,10 +81,15 @@ func (c *Controller) rollingUpdate(cluster *corev1.StorageCluster, hash string)
 	// Need to generalise code for 5 KVDB nodes
 
 	// get the number of kvdb members which are unavailable in case of internal kvdb
-	numUnavailableKvdb, kvdbNodes, err := c.getKVDBNodeAvailability(cluster)
-	if err != nil {
-		return err
+	numUnavailableKvdb := -1
+	var kvdbNodes map[string]bool
+	if !util.IsFreshInstall(cluster) {
+		numUnavailableKvdb, kvdbNodes, err = c.getKVDBNodeAvailability(cluster)
+		if err != nil {
+			return err
+		}
 	}
+
 	logrus.Debugf("Marking old pods for deletion")
 	for _, pod := range oldAvailablePods {
 		if numUnavailable >= maxUnavailable {
@@ -94,10 +99,12 @@ func (c *Controller) rollingUpdate(cluster *corev1.StorageCluster, hash string)
 		}
 
 		// check if pod is running in a node which has internal kvdb running in it
-		if _, isKvdbNode := kvdbNodes[pod.Spec.NodeName]; cluster.Spec.Kvdb != nil && cluster.Spec.Kvdb.Internal && isKvdbNode {
-			// if number of unavailable kvdb nodes is greater than or equal to 1, then dont restart portworx on this node
-			if numUnavailableKvdb > 0 {
-				logrus.Infof("Number of unavaliable KVDB members exceeds 1, temporarily skipping update for this node to prevent KVDB from going out of quorum ")
+		// numUnavailableKvdb will be 0 or more when it is not a fresh install. We want to skip this for fresh install
+		if _, isKvdbNode := kvdbNodes[pod.Spec.NodeName]; numUnavailableKvdb >= 0 && cluster.Spec.Kvdb != nil && cluster.Spec.Kvdb.Internal && isKvdbNode {
+			// if number of unavailable kvdb nodes is greater than or equal to 1, or lesser than 3 entries are present in the kvdb map
+			// then dont restart portworx on this node
+			if numUnavailableKvdb > 0 || len(kvdbNodes) < 3 {
+				logrus.Infof("One or more KVDB members are down, temporarily skipping update for this node to prevent KVDB from going out of quorum ")
 				continue
 			} else {
 				numUnavailableKvdb++