[Bugfix] Change Plan change discovery (#784)

CHANGELOG.md

@@ -8,6 +8,7 @@
 - Check if the DB server is cleaned out.
 - Render Pod Template in ArangoMember Spec and Status
 - Add Pod PropagationModes
+- Fix MemberUp action for ActiveFailover
 
 ## [1.2.1](https://github.com/arangodb/kube-arangodb/tree/1.2.1) (2021-07-28)
 - Fix ArangoMember race with multiple ArangoDeployments within single namespace

pkg/deployment/reconcile/action.go

@@ -74,12 +74,12 @@ type ActionPlanAppender interface {
 	Action
 
 	// ActionPlanAppender modify plan after action execution
-	ActionPlanAppender(current api.Plan) api.Plan
+	ActionPlanAppender(current api.Plan) (api.Plan, bool)
 }
 
-func getActionPlanAppender(a Action, plan api.Plan) api.Plan {
+func getActionPlanAppender(a Action, plan api.Plan) (api.Plan, bool) {
 	if c, ok := a.(ActionPlanAppender); !ok {
-		return plan
+		return plan, false
 	} else {
 		return c.ActionPlanAppender(plan)
 	}

pkg/deployment/reconcile/action_add_member.go

@@ -78,7 +78,7 @@ func (a *actionAddMember) Start(ctx context.Context) (bool, error) {
 }
 
 // ActionPlanAppender appends wait methods to the plan
-func (a *actionAddMember) ActionPlanAppender(current api.Plan) api.Plan {
+func (a *actionAddMember) ActionPlanAppender(current api.Plan) (api.Plan, bool) {
 	var app api.Plan
 
 	if _, ok := a.action.Params[api.ActionTypeWaitForMemberUp.String()]; ok {
@@ -88,5 +88,10 @@ func (a *actionAddMember) ActionPlanAppender(current api.Plan) api.Plan {
 	if _, ok := a.action.Params[api.ActionTypeWaitForMemberUp.String()]; ok {
 		app = append(app, api.NewAction(api.ActionTypeWaitForMemberInSync, a.action.Group, a.newMemberID, "Wait for member in sync after creation"))
 	}
-	return append(app, current...)
+
+	if len(app) > 0 {
+		return append(app, current...), true
+	}
+
+	return current, false
 }

pkg/deployment/reconcile/action_cleanout_member.go

@@ -128,7 +128,7 @@ func (a *actionCleanoutMember) CheckProgress(ctx context.Context) (bool, bool, e
 	c, err := a.actionCtx.GetDatabaseClient(ctxChild)
 	if err != nil {
 		log.Debug().Err(err).Msg("Failed to create database client")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 
 	ctxChild, cancel = context.WithTimeout(ctx, arangod.GetRequestTimeout())
@@ -136,7 +136,7 @@ func (a *actionCleanoutMember) CheckProgress(ctx context.Context) (bool, bool, e
 	cluster, err := c.Cluster(ctxChild)
 	if err != nil {
 		log.Debug().Err(err).Msg("Failed to access cluster")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 
 	ctxChild, cancel = context.WithTimeout(ctx, arangod.GetRequestTimeout())
@@ -144,7 +144,7 @@ func (a *actionCleanoutMember) CheckProgress(ctx context.Context) (bool, bool, e
 	cleanedOut, err := cluster.IsCleanedOut(ctxChild, a.action.MemberID)
 	if err != nil {
 		log.Debug().Err(err).Msg("IsCleanedOut failed")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 	if !cleanedOut {
 		// We're not done yet, check job status
@@ -155,7 +155,7 @@ func (a *actionCleanoutMember) CheckProgress(ctx context.Context) (bool, bool, e
 		c, err := a.actionCtx.GetDatabaseClient(ctxChild)
 		if err != nil {
 			log.Debug().Err(err).Msg("Failed to create database client")
-			return false, false, errors.WithStack(err)
+			return false, false, nil
 		}
 
 		ctxChild, cancel = context.WithTimeout(ctx, arangod.GetRequestTimeout())
@@ -163,7 +163,7 @@ func (a *actionCleanoutMember) CheckProgress(ctx context.Context) (bool, bool, e
 		agency, err := a.actionCtx.GetAgency(ctxChild)
 		if err != nil {
 			log.Debug().Err(err).Msg("Failed to create agency client")
-			return false, false, errors.WithStack(err)
+			return false, false, nil
 		}
 
 		ctxChild, cancel = context.WithTimeout(ctx, arangod.GetRequestTimeout())
@@ -171,14 +171,14 @@ func (a *actionCleanoutMember) CheckProgress(ctx context.Context) (bool, bool, e
 		jobStatus, err := arangod.CleanoutServerJobStatus(ctxChild, m.CleanoutJobID, c, agency)
 		if err != nil {
 			log.Debug().Err(err).Msg("Failed to fetch cleanout job status")
-			return false, false, errors.WithStack(err)
+			return false, false, nil
 		}
 		if jobStatus.IsFailed() {
 			log.Warn().Str("reason", jobStatus.Reason()).Msg("Cleanout Job failed. Aborting plan")
 			// Revert cleanout state
 			m.Phase = api.MemberPhaseCreated
 			m.CleanoutJobID = ""
-			if a.actionCtx.UpdateMember(ctx, m); err != nil {
+			if err := a.actionCtx.UpdateMember(ctx, m); err != nil {
 				return false, false, errors.WithStack(err)
 			}
 			return false, true, nil

pkg/deployment/reconcile/action_resign_leadership.go

@@ -129,7 +129,7 @@ func (a *actionResignLeadership) CheckProgress(ctx context.Context) (bool, bool,
 
 	if enabled, err := a.actionCtx.GetAgencyMaintenanceMode(ctx); err != nil {
 		log.Error().Err(err).Msgf("Unable to get maintenance mode")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	} else if enabled {
 		log.Warn().Msgf("Maintenance is enabled, skipping action")
 		// We are done, action cannot be handled on maintenance mode
@@ -145,7 +145,7 @@ func (a *actionResignLeadership) CheckProgress(ctx context.Context) (bool, bool,
 	agency, err := a.actionCtx.GetAgency(ctxChild)
 	if err != nil {
 		log.Debug().Err(err).Msg("Failed to create agency client")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 
 	ctxChild, cancel = context.WithTimeout(ctx, arangod.GetRequestTimeout())
@@ -153,7 +153,7 @@ func (a *actionResignLeadership) CheckProgress(ctx context.Context) (bool, bool,
 	c, err := a.actionCtx.GetDatabaseClient(ctxChild)
 	if err != nil {
 		log.Debug().Err(err).Msg("Failed to create member client")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 
 	ctxChild, cancel = context.WithTimeout(ctx, arangod.GetRequestTimeout())

pkg/deployment/reconcile/action_rotate_member.go

@@ -99,7 +99,8 @@ func (a *actionRotateMember) CheckProgress(ctx context.Context) (bool, bool, err
 	// Pod is terminated, we can now remove it
 	if err := a.actionCtx.DeletePod(ctx, m.PodName); err != nil {
 		if !k8sutil.IsNotFound(err) {
-			return false, false, errors.WithStack(err)
+			log.Error().Err(err).Msg("Unable to delete pod")
+			return false, false, nil
 		}
 	}
 	// Pod is now gone, update the member status

pkg/deployment/reconcile/action_rotate_start_member.go

@@ -99,7 +99,8 @@ func (a *actionRotateStartMember) CheckProgress(ctx context.Context) (bool, bool
 	// Pod is terminated, we can now remove it
 	if err := a.actionCtx.DeletePod(ctx, m.PodName); err != nil {
 		if !k8sutil.IsNotFound(err) {
-			return false, false, errors.WithStack(err)
+			log.Error().Err(err).Msg("Unable to delete pod")
+			return false, false, nil
 		}
 	}
 

pkg/deployment/reconcile/action_upgrade_current_image.go

@@ -72,7 +72,8 @@ func (a *setCurrentImageAction) CheckProgress(ctx context.Context) (bool, bool,
 		return false, false, nil
 	}
 	if err := a.actionCtx.SetCurrentImage(ctx, imageInfo); err != nil {
-		return false, false, errors.WithStack(err)
+		log.Error().Err(err).Msg("Unable to set current image")
+		return false, false, nil
 	}
 	log.Info().Str("image", a.action.Image).Str("to", imageInfo.Image).Msg("Changed current main image")
 	return true, false, nil

pkg/deployment/reconcile/action_wait_for_member_up.go

@@ -109,11 +109,11 @@ func (a *actionWaitForMemberUp) checkProgressSingle(ctx context.Context) (bool,
 	c, err := a.actionCtx.GetDatabaseClient(ctx)
 	if err != nil {
 		log.Debug().Err(err).Msg("Failed to create database client")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 	if _, err := c.Version(ctx); err != nil {
 		log.Debug().Err(err).Msg("Failed to get version")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 	return true, false, nil
 }
@@ -125,11 +125,11 @@ func (a *actionWaitForMemberUp) checkProgressSingleInActiveFailover(ctx context.
 	c, err := a.actionCtx.GetServerClient(ctx, a.action.Group, a.action.MemberID)
 	if err != nil {
 		log.Debug().Err(err).Msg("Failed to create database client")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 	if _, err := c.Version(ctx); err != nil {
 		log.Debug().Err(err).Msg("Failed to get version")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 	return true, false, nil
 }
@@ -141,7 +141,7 @@ func (a *actionWaitForMemberUp) checkProgressAgent(ctx context.Context) (bool, b
 	clients, err := a.actionCtx.GetAgencyClients(ctx)
 	if err != nil {
 		log.Debug().Err(err).Msg("Failed to create agency clients")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 
 	for _, a := range clients {
@@ -204,11 +204,11 @@ func (a *actionWaitForMemberUp) checkProgressArangoSync(ctx context.Context) (bo
 	c, err := a.actionCtx.GetSyncServerClient(ctx, a.action.Group, a.action.MemberID)
 	if err != nil {
 		log.Debug().Err(err).Msg("Failed to create arangosync client")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 	if err := c.Health(ctx); err != nil {
 		log.Debug().Err(err).Msg("Health not ok yet")
-		return false, false, errors.WithStack(err)
+		return false, false, nil
 	}
 	return true, false, nil
 }

pkg/deployment/reconcile/helper_shutdown.go

@@ -170,7 +170,8 @@ func (s shutdownHelperDelete) CheckProgress(ctx context.Context) (bool, bool, er
 			log.Warn().Msgf("Pod still exists")
 			return false, false, nil
 		} else if !k8sutil.IsNotFound(err) {
-			return false, false, errors.WithStack(err)
+			log.Error().Err(err).Msg("Unable to get pod")
+			return false, false, nil
 		}
 	}
 

pkg/deployment/reconcile/plan_builder_high.go

@@ -101,10 +101,12 @@ func updateMemberPodTemplateSpec(ctx context.Context,
 	// Update member specs
 	status.Members.ForeachServerGroup(func(group api.ServerGroup, members api.MemberStatusList) error {
 		for _, m := range members {
+			if m.Phase != api.MemberPhaseNone {
 				if reason, changed := arangoMemberPodTemplateNeedsUpdate(ctx, log, apiObject, spec, group, status, m, cachedStatus, context); changed {
 					plan = append(plan, api.NewAction(api.ActionTypeArangoMemberUpdatePodSpec, group, m.ID, reason))
 				}
 			}
+		}
 
 		return nil
 	})

pkg/deployment/reconcile/plan_executor.go

@@ -180,7 +180,7 @@ func (d *Reconciler) executePlan(ctx context.Context, cachedStatus inspectorInte
 				}
 			}
 
-			if newPlan := getActionPlanAppender(action, plan); !newPlan.Equal(plan) {
+			if newPlan, changed := getActionPlanAppender(action, plan); changed {
 				// Our actions have been added to the end of plan
 				log.Info().Msgf("Appending new plan items")
 				return newPlan, true, nil