Merge pull request #425 from arangodb/bug-fix/unreliable-cluster-membership

Bug fix/unreliable cluster membership

Author: ewoutp

pkg/apis/deployment/v1alpha/member_status_list.go

@@ -41,7 +41,7 @@ func (l MemberStatusList) Equal(other MemberStatusList) bool {
 	}
 
 	for i := 0; i < len(l); i++ {
-		o, found := l.ElementByID(l[i].ID)
+		o, found := other.ElementByID(l[i].ID)
 		if !found {
 			return false
 		}

pkg/apis/deployment/v1alpha/member_status_list_test.go

@@ -66,4 +66,26 @@ func TestMemberStatusList(t *testing.T) {
 	assert.Equal(t, m1.ID, (*list)[0].ID)
 	assert.Equal(t, m2.ID, (*list)[1].ID)
 	assert.Equal(t, m3.ID, (*list)[2].ID)
+
+	list2 := &MemberStatusList{m3, m2, m1}
+	assert.True(t, list.Equal(*list2))
+	assert.True(t, list2.Equal(*list))
+
+	list3 := &MemberStatusList{m3, m1}
+	assert.False(t, list.Equal(*list3))
+	assert.False(t, list3.Equal(*list))
+
+	list4 := MemberStatusList{m3, m2, m1}
+	list4[1].Phase = "something-else"
+	assert.False(t, list.Equal(list4))
+	assert.False(t, list4.Equal(*list))
+	
+	m4 := MemberStatus{ID: "m4"}
+	list5 := &MemberStatusList{m1, m2, m4}
+	assert.False(t, list.Equal(*list5))
+	assert.False(t, list5.Equal(*list))
+
+	list6 := &MemberStatusList{m1, m2, m3, m4}
+	assert.False(t, list.Equal(*list6))
+	assert.False(t, list6.Equal(*list))
 }

pkg/deployment/resources/member_cleanup.go

@@ -71,7 +71,9 @@ func (r *Resources) cleanupRemovedClusterMembers() error {
 	r.health.mutex.Unlock()
 
 	// Only accept recent cluster health values
-	if time.Since(ts) > maxClusterHealthAge {
+	healthAge := time.Since(ts)
+	if healthAge > maxClusterHealthAge {
+		log.Info().Dur("age", healthAge).Msg("Cleanup longer than max cluster health. Exiting")
 		return nil
 	}
 
@@ -90,23 +92,32 @@ func (r *Resources) cleanupRemovedClusterMembers() error {
 			return nil
 		}
 		for _, m := range list {
+			log := log.With().
+				Str("member", m.ID).
+				Str("role", group.AsRole()).
+				Logger()
 			if serverFound(m.ID) {
 				// Member is (still) found, skip it
 				if m.Conditions.Update(api.ConditionTypeMemberOfCluster, true, "", "") {
-					status.Members.Update(m, group)
+					if err := status.Members.Update(m, group); err != nil {
+						log.Warn().Err(err).Msg("Failed to update member")
+					}
 					updateStatusNeeded = true
+					log.Debug().Msg("Updating MemberOfCluster condition to true")
 				}
 				continue
 			} else if !m.Conditions.IsTrue(api.ConditionTypeMemberOfCluster) {
 				// Member is not yet recorded as member of cluster
 				if m.Age() < minMemberAge {
+					log.Debug().Dur("age", m.Age()).Msg("Member age is below minimum for removal")
 					continue
 				}
-				log.Info().Str("member", m.ID).Str("role", group.AsRole()).Msg("Member has never been part of the cluster for a long time. Removing it.")
+				log.Info().Msg("Member has never been part of the cluster for a long time. Removing it.")
 			} else {
 				// Member no longer part of cluster, remove it
-				log.Info().Str("member", m.ID).Str("role", group.AsRole()).Msg("Member is no longer part of the ArangoDB cluster. Removing it.")
+				log.Info().Msg("Member is no longer part of the ArangoDB cluster. Removing it.")
 			}
+			log.Info().Msg("Removing member")
 			status.Members.RemoveByID(m.ID, group)
 			updateStatusNeeded = true
 			// Remove Pod & PVC (if any)
@@ -121,17 +132,23 @@ func (r *Resources) cleanupRemovedClusterMembers() error {
 	})
 
 	if updateStatusNeeded {
+		log.Debug().Msg("UpdateStatus needed")
+
 		if err := r.context.UpdateStatus(status, lastVersion); err != nil {
+			log.Warn().Err(err).Msg("Failed to update deployment status")
 			return maskAny(err)
 		}
 	}
 
 	for _, podName := range podNamesToRemove {
+		log.Info().Str("pod", podName).Msg("Removing obsolete member pod")
 		if err := r.context.DeletePod(podName); err != nil && !k8sutil.IsNotFound(err) {
 			log.Warn().Err(err).Str("pod", podName).Msg("Failed to remove obsolete pod")
 		}
 	}
+
 	for _, pvcName := range pvcNamesToRemove {
+		log.Info().Str("pvc", pvcName).Msg("Removing obsolete member PVC")
 		if err := r.context.DeletePvc(pvcName); err != nil && !k8sutil.IsNotFound(err) {
 			log.Warn().Err(err).Str("pvc", pvcName).Msg("Failed to remove obsolete PVC")
 		}