[Bugfix] Member Status race condition fix (#816)

Author: ajanikow

pkg/deployment/reconcile/member_phase_updates.go renamed to pkg/deployment/member/phase_updates.go

@@ -20,7 +20,7 @@
 // Author Adam Janikowski
 //
 
-package reconcile
+package member
 
 import (
 	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
@@ -31,31 +31,53 @@ type phaseMapFunc func(action api.Action, m *api.MemberStatus)
 type phaseMapTo map[api.MemberPhase]phaseMapFunc
 type phaseMap map[api.MemberPhase]phaseMapTo
 
+type PhaseExecutor interface {
+	Execute(m *api.MemberStatus, action api.Action, to api.MemberPhase) bool
+}
+
+func GetPhaseExecutor() PhaseExecutor {
+	return phase
+}
+
 var phase = phaseMap{
 	api.MemberPhaseNone: {
 		api.MemberPhasePending: func(action api.Action, m *api.MemberStatus) {
-			// Clean conditions
-			m.Conditions.Remove(api.ConditionTypeReady)
-			m.Conditions.Remove(api.ConditionTypeTerminated)
-			m.Conditions.Remove(api.ConditionTypeTerminating)
-			m.Conditions.Remove(api.ConditionTypeAgentRecoveryNeeded)
-			m.Conditions.Remove(api.ConditionTypeAutoUpgrade)
-			m.Conditions.Remove(api.ConditionTypeUpgradeFailed)
-			m.Conditions.Remove(api.ConditionTypePendingTLSRotation)
-			m.Conditions.Remove(api.ConditionTypePendingRestart)
-			m.Conditions.Remove(api.ConditionTypeRestart)
-			m.Conditions.Remove(api.ConditionTypePendingUpdate)
-			m.Conditions.Remove(api.ConditionTypeUpdating)
-			m.Conditions.Remove(api.ConditionTypeUpdateFailed)
-			m.Conditions.Remove(api.ConditionTypeCleanedOut)
-
 			// Change member RID
 			m.RID = uuid.NewUUID()
 
 			// Clean Pod details
 			m.PodUID = ""
 		},
 	},
+	api.MemberPhasePending: {
+		api.MemberPhaseCreated: func(action api.Action, m *api.MemberStatus) {
+			// Clean conditions
+			removeMemberConditionsMapFunc(m)
+		},
+		api.MemberPhaseUpgrading: func(action api.Action, m *api.MemberStatus) {
+			removeMemberConditionsMapFunc(m)
+		},
+	},
+}
+
+func removeMemberConditionsMapFunc(m *api.MemberStatus) {
+	// Clean conditions
+	m.Conditions.Remove(api.ConditionTypeReady)
+	m.Conditions.Remove(api.ConditionTypeTerminated)
+	m.Conditions.Remove(api.ConditionTypeTerminating)
+	m.Conditions.Remove(api.ConditionTypeAgentRecoveryNeeded)
+	m.Conditions.Remove(api.ConditionTypeAutoUpgrade)
+	m.Conditions.Remove(api.ConditionTypeUpgradeFailed)
+	m.Conditions.Remove(api.ConditionTypePendingTLSRotation)
+	m.Conditions.Remove(api.ConditionTypePendingRestart)
+	m.Conditions.Remove(api.ConditionTypeRestart)
+	m.Conditions.Remove(api.ConditionTypePendingUpdate)
+	m.Conditions.Remove(api.ConditionTypeUpdating)
+	m.Conditions.Remove(api.ConditionTypeUpdateFailed)
+	m.Conditions.Remove(api.ConditionTypeCleanedOut)
+	m.Conditions.Remove(api.ConditionTypeTopologyAware)
+
+	m.Upgrade = false
 }
 
 func (p phaseMap) empty(action api.Action, m *api.MemberStatus) {

pkg/deployment/reconcile/action_member_phase_update.go

@@ -25,6 +25,8 @@ package reconcile
 import (
 	"context"
 
+	"github.com/arangodb/kube-arangodb/pkg/deployment/member"
+
 	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
 	"github.com/arangodb/kube-arangodb/pkg/util/errors"
 	"github.com/rs/zerolog"
@@ -72,7 +74,7 @@ func (a *memberPhaseUpdateAction) Start(ctx context.Context) (bool, error) {
 		return true, nil
 	}
 
-	if phase.Execute(&m, a.action, p) {
+	if member.GetPhaseExecutor().Execute(&m, a.action, p) {
 		if err := a.actionCtx.UpdateMember(ctx, m); err != nil {
 			return false, errors.WithStack(err)
 		}

pkg/deployment/reconcile/plan_executor.go

@@ -145,6 +145,12 @@ func (d *Reconciler) executePlan(ctx context.Context, cachedStatus inspectorInte
 			Str("group", planAction.Group.AsRole()).
 			Str("member-id", planAction.MemberID)
 
+		if status, _ := d.context.GetStatus(); status.Members.ContainsID(planAction.MemberID) {
+			if member, _, ok := status.Members.ElementByID(planAction.MemberID); ok {
+				logContext = logContext.Str("phase", string(member.Phase))
+			}
+		}
+
 		for k, v := range planAction.Params {
 			logContext = logContext.Str(k, v)
 		}

pkg/deployment/resources/pod_creator.go

@@ -33,6 +33,8 @@ import (
 	"sync"
 	"time"
 
+	"github.com/arangodb/kube-arangodb/pkg/deployment/member"
+
 	podMod "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector/pod"
 	core "k8s.io/api/core/v1"
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -512,7 +514,7 @@ func (r *Resources) SelectImageForMember(spec api.DeploymentSpec, status api.Dep
 }
 
 // createPodForMember creates all Pods listed in member status
-func (r *Resources) createPodForMember(ctx context.Context, cachedStatus inspectorInterface.Inspector, spec api.DeploymentSpec, member *api.ArangoMember, memberID string, imageNotFoundOnce *sync.Once) error {
+func (r *Resources) createPodForMember(ctx context.Context, cachedStatus inspectorInterface.Inspector, spec api.DeploymentSpec, arangoMember *api.ArangoMember, memberID string, imageNotFoundOnce *sync.Once) error {
 	log := r.log
 	status, lastVersion := r.context.GetStatus()
 
@@ -525,7 +527,7 @@ func (r *Resources) createPodForMember(ctx context.Context, cachedStatus inspect
 		return nil
 	}
 
-	template := member.Status.Template
+	template := arangoMember.Status.Template
 
 	if template == nil {
 		// Template not yet propagated
@@ -634,8 +636,7 @@ func (r *Resources) createPodForMember(ctx context.Context, cachedStatus inspect
 		m.PodSpecVersion = template.PodSpecChecksum
 	}
 
-	// Record new member phase
-	m.Phase = newPhase
+	member.GetPhaseExecutor().Execute(&m, api.Action{}, newPhase)
 
 	if status.Topology.Enabled() {
 		if m.Topology != nil && m.Topology.ID == status.Topology.ID {
@@ -645,7 +646,6 @@ func (r *Resources) createPodForMember(ctx context.Context, cachedStatus inspect
 		}
 	}
 
-	m.Upgrade = false
 	r.log.Info().Str("pod", m.PodName).Msgf("Updating member")
 	if err := status.Members.Update(m, group); err != nil {
 		return errors.WithStack(err)