Fixed MemberStatusList.Equal

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,17 @@ 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))
 }

pkg/deployment/resources/member_cleanup.go

@@ -29,7 +29,6 @@ import (
 	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha"
 	"github.com/arangodb/kube-arangodb/pkg/metrics"
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
-	"github.com/rs/zerolog/log"
 )
 
 const (
@@ -63,9 +62,6 @@ func (r *Resources) CleanupRemovedMembers() error {
 
 // cleanupRemovedClusterMembers removes all arangod members that are no longer part of the cluster.
 func (r *Resources) cleanupRemovedClusterMembers() error {
-
-	log.Info().Msg("Cleanup routine 1")
-
 	log := r.log
 
 	// Fetch recent cluster health
@@ -74,53 +70,53 @@ func (r *Resources) cleanupRemovedClusterMembers() error {
 	ts := r.health.timestamp
 	r.health.mutex.Unlock()
 
-	log.Info().Msg("Cleanup routine 2")
-
 	// Only accept recent cluster health values
 	if time.Since(ts) > maxClusterHealthAge {
 		log.Info().Msg("Cleanup longer than max cluster health exiting")
 		return nil
 	}
 
-	log.Info().Msg("Cleanup routine 3")
-
 	serverFound := func(id string) bool {
 		_, found := h.Health[driver.ServerID(id)]
-		log.Info().Bool("found ", found).Str("server id ", id)
 		return found
 	}
 
-	log.Info().Msg("Cleanup routine 4")
-
 	// For over all members that can be removed
 	status, lastVersion := r.context.GetStatus()
 	updateStatusNeeded := false
 	var podNamesToRemove, pvcNamesToRemove []string
 	status.Members.ForeachServerGroup(func(group api.ServerGroup, list api.MemberStatusList) error {
 		if group != api.ServerGroupCoordinators && group != api.ServerGroupDBServers {
 			// We're not interested in these other groups
-			log.Info().Str("group ", group.AsRole()).Msg("Not interested in group ")
 			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)
@@ -134,31 +130,28 @@ func (r *Resources) cleanupRemovedClusterMembers() error {
 		return nil
 	})
 
-	log.Info().Msg("Cleanup routine 4")
-
 	if updateStatusNeeded {
-		log.Info().Msg("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)
 		}
 	}
 
-	log.Info().Msg("Cleanup routine 5")
 	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")
 		}
 	}
 
-	log.Info().Msg("Cleanup routine 6")
 	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")
 		}
 	}
 
-	log.Info().Msg("Cleanup routine 7")
-
 	return nil
 }