From 6994b13bb02c4f2e915c922957bddd63fd865b0e Mon Sep 17 00:00:00 2001 From: ajanikow <12255597+ajanikow@users.noreply.github.com> Date: Mon, 6 Dec 2021 10:37:02 +0000 Subject: [PATCH] [Feature] Plan Builder Recovery --- CHANGELOG.md | 1 + .../reconcile/plan_builder_appender.go | 67 ++++++++++++++++++ .../reconcile/plan_builder_appender_test.go | 69 +++++++++++++++++++ pkg/deployment/reconcile/plan_builder_high.go | 4 +- .../reconcile/plan_builder_normal.go | 4 +- pkg/deployment/rotation/check.go | 14 ++-- 6 files changed, 149 insertions(+), 10 deletions(-) create mode 100644 pkg/deployment/reconcile/plan_builder_appender_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cecf0ec1..87666aaa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Add ArangoBackup backoff functionality - Allow to abort ArangoBackup uploads by removing spec.upload - Add Agency Cache internally +- Add Recovery during PlanBuild operation ## [1.2.5](https://github.com/arangodb/kube-arangodb/tree/1.2.5) (2021-10-25) - Split & Unify Lifecycle management functionality diff --git a/pkg/deployment/reconcile/plan_builder_appender.go b/pkg/deployment/reconcile/plan_builder_appender.go index 0e4293f21..0d3bfd980 100644 --- a/pkg/deployment/reconcile/plan_builder_appender.go +++ b/pkg/deployment/reconcile/plan_builder_appender.go @@ -24,6 +24,7 @@ package reconcile import ( api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" + "github.com/rs/zerolog" ) func newPlanAppender(pb WithPlanBuilder, current api.Plan) PlanAppender { @@ -33,6 +34,13 @@ func newPlanAppender(pb WithPlanBuilder, current api.Plan) PlanAppender { } } +func recoverPlanAppender(log zerolog.Logger, p PlanAppender) PlanAppender { + return planAppenderRecovery{ + appender: p, + log: log, + } +} + type PlanAppender interface { Apply(pb planBuilder) PlanAppender ApplyWithCondition(c planBuilderCondition, pb planBuilder) PlanAppender @@ -45,6 +53,65 @@ type PlanAppender interface { Plan() api.Plan } +type planAppenderRecovery struct { + appender PlanAppender + log zerolog.Logger +} + +func (p planAppenderRecovery) create(ret func(in PlanAppender) PlanAppender) (r PlanAppender) { + defer func() { + if e := recover(); e != nil { + r = p + p.log.Error().Interface("panic", e).Msgf("Recovering from panic") + } + }() + + return planAppenderRecovery{ + appender: ret(p.appender), + log: p.log, + } +} + +func (p planAppenderRecovery) Apply(pb planBuilder) PlanAppender { + return p.create(func(in PlanAppender) PlanAppender { + return in.Apply(pb) + }) +} + +func (p planAppenderRecovery) ApplyWithCondition(c planBuilderCondition, pb planBuilder) PlanAppender { + return p.create(func(in PlanAppender) PlanAppender { + return in.ApplyWithCondition(c, pb) + }) +} + +func (p planAppenderRecovery) ApplySubPlan(pb planBuilderSubPlan, plans ...planBuilder) (r PlanAppender) { + return p.create(func(in PlanAppender) PlanAppender { + return in.ApplySubPlan(pb, plans...) + }) +} + +func (p planAppenderRecovery) ApplyIfEmpty(pb planBuilder) (r PlanAppender) { + return p.create(func(in PlanAppender) PlanAppender { + return in.ApplyIfEmpty(pb) + }) +} + +func (p planAppenderRecovery) ApplyWithConditionIfEmpty(c planBuilderCondition, pb planBuilder) (r PlanAppender) { + return p.create(func(in PlanAppender) PlanAppender { + return in.ApplyWithConditionIfEmpty(c, pb) + }) +} + +func (p planAppenderRecovery) ApplySubPlanIfEmpty(pb planBuilderSubPlan, plans ...planBuilder) (r PlanAppender) { + return p.create(func(in PlanAppender) PlanAppender { + return in.ApplySubPlanIfEmpty(pb, plans...) + }) +} + +func (p planAppenderRecovery) Plan() api.Plan { + return p.appender.Plan() +} + type planAppenderType struct { pb WithPlanBuilder current api.Plan diff --git a/pkg/deployment/reconcile/plan_builder_appender_test.go b/pkg/deployment/reconcile/plan_builder_appender_test.go new file mode 100644 index 000000000..0db3a33c0 --- /dev/null +++ b/pkg/deployment/reconcile/plan_builder_appender_test.go @@ -0,0 +1,69 @@ +// +// DISCLAIMER +// +// Copyright 2016-2021 ArangoDB GmbH, Cologne, Germany +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// + +package reconcile + +import ( + "context" + "testing" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" + inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" + "github.com/stretchr/testify/require" +) + +func Test_PlanBuilderAppender_Recovery(t *testing.T) { + t.Run("Recover", func(t *testing.T) { + require.Len(t, recoverPlanAppender(log.Logger, newPlanAppender(NewWithPlanBuilder(context.Background(), zerolog.Logger{}, nil, api.DeploymentSpec{}, api.DeploymentStatus{}, nil, nil), nil)). + Apply(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan { + panic("") + }). + Apply(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan { + panic("SomePanic") + }).Plan(), 0) + }) + t.Run("Recover with output", func(t *testing.T) { + require.Len(t, recoverPlanAppender(log.Logger, newPlanAppender(NewWithPlanBuilder(context.Background(), zerolog.Logger{}, nil, api.DeploymentSpec{}, api.DeploymentStatus{}, nil, nil), nil)). + Apply(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan { + return api.Plan{api.Action{}} + }). + ApplyIfEmpty(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan { + panic("SomePanic") + }). + ApplyIfEmpty(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan { + return api.Plan{api.Action{}, api.Action{}} + }).Plan(), 1) + }) + t.Run("Recover with multi", func(t *testing.T) { + require.Len(t, recoverPlanAppender(log.Logger, newPlanAppender(NewWithPlanBuilder(context.Background(), zerolog.Logger{}, nil, api.DeploymentSpec{}, api.DeploymentStatus{}, nil, nil), nil)). + Apply(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan { + return api.Plan{api.Action{}} + }). + ApplyIfEmpty(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan { + panic("SomePanic") + }). + Apply(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan { + return api.Plan{api.Action{}, api.Action{}} + }).Plan(), 3) + }) +} diff --git a/pkg/deployment/reconcile/plan_builder_high.go b/pkg/deployment/reconcile/plan_builder_high.go index ab1a80f94..19e1b2742 100644 --- a/pkg/deployment/reconcile/plan_builder_high.go +++ b/pkg/deployment/reconcile/plan_builder_high.go @@ -87,7 +87,7 @@ func createHighPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil.A return currentPlan, false } - return newPlanAppender(NewWithPlanBuilder(ctx, log, apiObject, spec, status, cachedStatus, builderCtx), nil). + return recoverPlanAppender(log, newPlanAppender(NewWithPlanBuilder(ctx, log, apiObject, spec, status, cachedStatus, builderCtx), currentPlan). ApplyIfEmpty(updateMemberPodTemplateSpec). ApplyIfEmpty(updateMemberPhasePlan). ApplyIfEmpty(createCleanOutPlan). @@ -95,7 +95,7 @@ func createHighPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil.A ApplyIfEmpty(updateMemberRotationConditionsPlan). ApplyIfEmpty(createTopologyMemberUpdatePlan). ApplyIfEmpty(createTopologyMemberConditionPlan). - ApplyIfEmpty(createRebalancerCheckPlan). + ApplyIfEmpty(createRebalancerCheckPlan)). Plan(), true } diff --git a/pkg/deployment/reconcile/plan_builder_normal.go b/pkg/deployment/reconcile/plan_builder_normal.go index 8485fa7d4..4112c8f10 100644 --- a/pkg/deployment/reconcile/plan_builder_normal.go +++ b/pkg/deployment/reconcile/plan_builder_normal.go @@ -84,7 +84,7 @@ func createNormalPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil return currentPlan, false } - return newPlanAppender(NewWithPlanBuilder(ctx, log, apiObject, spec, status, cachedStatus, builderCtx), nil). + return recoverPlanAppender(log, newPlanAppender(NewWithPlanBuilder(ctx, log, apiObject, spec, status, cachedStatus, builderCtx), currentPlan). // Adjust topology settings ApplyIfEmpty(createTopologyMemberAdjustmentPlan). // Define topology @@ -124,7 +124,7 @@ func createNormalPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil ApplyIfEmpty(createRebalancerGeneratePlan). // Final ApplyIfEmpty(createTLSStatusPropagated). - ApplyIfEmpty(createBootstrapPlan). + ApplyIfEmpty(createBootstrapPlan)). Plan(), true } diff --git a/pkg/deployment/rotation/check.go b/pkg/deployment/rotation/check.go index d2eefdb9a..9d47b5d87 100644 --- a/pkg/deployment/rotation/check.go +++ b/pkg/deployment/rotation/check.go @@ -64,13 +64,15 @@ func IsRotationRequired(log zerolog.Logger, cachedStatus inspectorInterface.Insp mode = SkippedRotation // We are under termination - if member.Conditions.IsTrue(api.ConditionTypeTerminating) || (pod != nil && pod.DeletionTimestamp != nil) { - if l := utils.StringList(pod.Finalizers); l.Has(constants.FinalizerPodGracefulShutdown) && !l.Has(constants.FinalizerDelayPodTermination) { - reason = "Recreation enforced by deleted state" - mode = EnforcedRotation - } + if pod != nil { + if member.Conditions.IsTrue(api.ConditionTypeTerminating) || pod.DeletionTimestamp != nil { + if l := utils.StringList(pod.Finalizers); l.Has(constants.FinalizerPodGracefulShutdown) && !l.Has(constants.FinalizerDelayPodTermination) { + reason = "Recreation enforced by deleted state" + mode = EnforcedRotation + } - return + return + } } if !CheckPossible(member) {