From 420db28ea1cfe99a42cdc2a32b0247f5bc57b177 Mon Sep 17 00:00:00 2001 From: ajanikow <12255597+ajanikow@users.noreply.github.com> Date: Tue, 19 Oct 2021 23:09:53 +0000 Subject: [PATCH] [Bugfix] Member Status race condition fix --- .../phase_updates.go} | 54 +++++++++++++------ .../reconcile/action_member_phase_update.go | 4 +- pkg/deployment/reconcile/plan_executor.go | 6 +++ pkg/deployment/resources/pod_creator.go | 10 ++-- 4 files changed, 52 insertions(+), 22 deletions(-) rename pkg/deployment/{reconcile/member_phase_updates.go => member/phase_updates.go} (57%) diff --git a/pkg/deployment/reconcile/member_phase_updates.go b/pkg/deployment/member/phase_updates.go similarity index 57% rename from pkg/deployment/reconcile/member_phase_updates.go rename to pkg/deployment/member/phase_updates.go index d3dd13ab9..e0d32bd01 100644 --- a/pkg/deployment/reconcile/member_phase_updates.go +++ b/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,24 +31,17 @@ 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() @@ -56,6 +49,35 @@ var phase = phaseMap{ 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) { diff --git a/pkg/deployment/reconcile/action_member_phase_update.go b/pkg/deployment/reconcile/action_member_phase_update.go index eb41353c6..273e2855d 100644 --- a/pkg/deployment/reconcile/action_member_phase_update.go +++ b/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) } diff --git a/pkg/deployment/reconcile/plan_executor.go b/pkg/deployment/reconcile/plan_executor.go index ddb61afdb..81d95dda5 100644 --- a/pkg/deployment/reconcile/plan_executor.go +++ b/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) } diff --git a/pkg/deployment/resources/pod_creator.go b/pkg/deployment/resources/pod_creator.go index e61d2fb32..97b156176 100644 --- a/pkg/deployment/resources/pod_creator.go +++ b/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)