Skip to content

Commit bfa9e20

Browse files
authored
[Feature] Plan Builder Recovery (#851)
1 parent 0e24ee3 commit bfa9e20

File tree

6 files changed

+149
-10
lines changed

6 files changed

+149
-10
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
- Add ArangoBackup backoff functionality
55
- Allow to abort ArangoBackup uploads by removing spec.upload
66
- Add Agency Cache internally
7+
- Add Recovery during PlanBuild operation
78

89
## [1.2.5](https://github.com/arangodb/kube-arangodb/tree/1.2.5) (2021-10-25)
910
- Split & Unify Lifecycle management functionality

pkg/deployment/reconcile/plan_builder_appender.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ package reconcile
2424

2525
import (
2626
api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
27+
"github.com/rs/zerolog"
2728
)
2829

2930
func newPlanAppender(pb WithPlanBuilder, current api.Plan) PlanAppender {
@@ -33,6 +34,13 @@ func newPlanAppender(pb WithPlanBuilder, current api.Plan) PlanAppender {
3334
}
3435
}
3536

37+
func recoverPlanAppender(log zerolog.Logger, p PlanAppender) PlanAppender {
38+
return planAppenderRecovery{
39+
appender: p,
40+
log: log,
41+
}
42+
}
43+
3644
type PlanAppender interface {
3745
Apply(pb planBuilder) PlanAppender
3846
ApplyWithCondition(c planBuilderCondition, pb planBuilder) PlanAppender
@@ -45,6 +53,65 @@ type PlanAppender interface {
4553
Plan() api.Plan
4654
}
4755

56+
type planAppenderRecovery struct {
57+
appender PlanAppender
58+
log zerolog.Logger
59+
}
60+
61+
func (p planAppenderRecovery) create(ret func(in PlanAppender) PlanAppender) (r PlanAppender) {
62+
defer func() {
63+
if e := recover(); e != nil {
64+
r = p
65+
p.log.Error().Interface("panic", e).Msgf("Recovering from panic")
66+
}
67+
}()
68+
69+
return planAppenderRecovery{
70+
appender: ret(p.appender),
71+
log: p.log,
72+
}
73+
}
74+
75+
func (p planAppenderRecovery) Apply(pb planBuilder) PlanAppender {
76+
return p.create(func(in PlanAppender) PlanAppender {
77+
return in.Apply(pb)
78+
})
79+
}
80+
81+
func (p planAppenderRecovery) ApplyWithCondition(c planBuilderCondition, pb planBuilder) PlanAppender {
82+
return p.create(func(in PlanAppender) PlanAppender {
83+
return in.ApplyWithCondition(c, pb)
84+
})
85+
}
86+
87+
func (p planAppenderRecovery) ApplySubPlan(pb planBuilderSubPlan, plans ...planBuilder) (r PlanAppender) {
88+
return p.create(func(in PlanAppender) PlanAppender {
89+
return in.ApplySubPlan(pb, plans...)
90+
})
91+
}
92+
93+
func (p planAppenderRecovery) ApplyIfEmpty(pb planBuilder) (r PlanAppender) {
94+
return p.create(func(in PlanAppender) PlanAppender {
95+
return in.ApplyIfEmpty(pb)
96+
})
97+
}
98+
99+
func (p planAppenderRecovery) ApplyWithConditionIfEmpty(c planBuilderCondition, pb planBuilder) (r PlanAppender) {
100+
return p.create(func(in PlanAppender) PlanAppender {
101+
return in.ApplyWithConditionIfEmpty(c, pb)
102+
})
103+
}
104+
105+
func (p planAppenderRecovery) ApplySubPlanIfEmpty(pb planBuilderSubPlan, plans ...planBuilder) (r PlanAppender) {
106+
return p.create(func(in PlanAppender) PlanAppender {
107+
return in.ApplySubPlanIfEmpty(pb, plans...)
108+
})
109+
}
110+
111+
func (p planAppenderRecovery) Plan() api.Plan {
112+
return p.appender.Plan()
113+
}
114+
48115
type planAppenderType struct {
49116
pb WithPlanBuilder
50117
current api.Plan
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
//
2+
// DISCLAIMER
3+
//
4+
// Copyright 2016-2021 ArangoDB GmbH, Cologne, Germany
5+
//
6+
// Licensed under the Apache License, Version 2.0 (the "License");
7+
// you may not use this file except in compliance with the License.
8+
// You may obtain a copy of the License at
9+
//
10+
// http://www.apache.org/licenses/LICENSE-2.0
11+
//
12+
// Unless required by applicable law or agreed to in writing, software
13+
// distributed under the License is distributed on an "AS IS" BASIS,
14+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
// See the License for the specific language governing permissions and
16+
// limitations under the License.
17+
//
18+
// Copyright holder is ArangoDB GmbH, Cologne, Germany
19+
//
20+
21+
package reconcile
22+
23+
import (
24+
"context"
25+
"testing"
26+
27+
api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
28+
"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
29+
inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector"
30+
"github.com/rs/zerolog"
31+
"github.com/rs/zerolog/log"
32+
"github.com/stretchr/testify/require"
33+
)
34+
35+
func Test_PlanBuilderAppender_Recovery(t *testing.T) {
36+
t.Run("Recover", func(t *testing.T) {
37+
require.Len(t, recoverPlanAppender(log.Logger, newPlanAppender(NewWithPlanBuilder(context.Background(), zerolog.Logger{}, nil, api.DeploymentSpec{}, api.DeploymentStatus{}, nil, nil), nil)).
38+
Apply(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan {
39+
panic("")
40+
}).
41+
Apply(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan {
42+
panic("SomePanic")
43+
}).Plan(), 0)
44+
})
45+
t.Run("Recover with output", func(t *testing.T) {
46+
require.Len(t, recoverPlanAppender(log.Logger, newPlanAppender(NewWithPlanBuilder(context.Background(), zerolog.Logger{}, nil, api.DeploymentSpec{}, api.DeploymentStatus{}, nil, nil), nil)).
47+
Apply(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan {
48+
return api.Plan{api.Action{}}
49+
}).
50+
ApplyIfEmpty(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan {
51+
panic("SomePanic")
52+
}).
53+
ApplyIfEmpty(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan {
54+
return api.Plan{api.Action{}, api.Action{}}
55+
}).Plan(), 1)
56+
})
57+
t.Run("Recover with multi", func(t *testing.T) {
58+
require.Len(t, recoverPlanAppender(log.Logger, newPlanAppender(NewWithPlanBuilder(context.Background(), zerolog.Logger{}, nil, api.DeploymentSpec{}, api.DeploymentStatus{}, nil, nil), nil)).
59+
Apply(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan {
60+
return api.Plan{api.Action{}}
61+
}).
62+
ApplyIfEmpty(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan {
63+
panic("SomePanic")
64+
}).
65+
Apply(func(_ context.Context, _ zerolog.Logger, _ k8sutil.APIObject, _ api.DeploymentSpec, _ api.DeploymentStatus, _ inspectorInterface.Inspector, _ PlanBuilderContext) api.Plan {
66+
return api.Plan{api.Action{}, api.Action{}}
67+
}).Plan(), 3)
68+
})
69+
}

pkg/deployment/reconcile/plan_builder_high.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,15 @@ func createHighPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil.A
8787
return currentPlan, false
8888
}
8989

90-
return newPlanAppender(NewWithPlanBuilder(ctx, log, apiObject, spec, status, cachedStatus, builderCtx), nil).
90+
return recoverPlanAppender(log, newPlanAppender(NewWithPlanBuilder(ctx, log, apiObject, spec, status, cachedStatus, builderCtx), currentPlan).
9191
ApplyIfEmpty(updateMemberPodTemplateSpec).
9292
ApplyIfEmpty(updateMemberPhasePlan).
9393
ApplyIfEmpty(createCleanOutPlan).
9494
ApplyIfEmpty(updateMemberUpdateConditionsPlan).
9595
ApplyIfEmpty(updateMemberRotationConditionsPlan).
9696
ApplyIfEmpty(createTopologyMemberUpdatePlan).
9797
ApplyIfEmpty(createTopologyMemberConditionPlan).
98-
ApplyIfEmpty(createRebalancerCheckPlan).
98+
ApplyIfEmpty(createRebalancerCheckPlan)).
9999
Plan(), true
100100
}
101101

pkg/deployment/reconcile/plan_builder_normal.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func createNormalPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil
8484
return currentPlan, false
8585
}
8686

87-
return newPlanAppender(NewWithPlanBuilder(ctx, log, apiObject, spec, status, cachedStatus, builderCtx), nil).
87+
return recoverPlanAppender(log, newPlanAppender(NewWithPlanBuilder(ctx, log, apiObject, spec, status, cachedStatus, builderCtx), currentPlan).
8888
// Adjust topology settings
8989
ApplyIfEmpty(createTopologyMemberAdjustmentPlan).
9090
// Define topology
@@ -124,7 +124,7 @@ func createNormalPlan(ctx context.Context, log zerolog.Logger, apiObject k8sutil
124124
ApplyIfEmpty(createRebalancerGeneratePlan).
125125
// Final
126126
ApplyIfEmpty(createTLSStatusPropagated).
127-
ApplyIfEmpty(createBootstrapPlan).
127+
ApplyIfEmpty(createBootstrapPlan)).
128128
Plan(), true
129129
}
130130

pkg/deployment/rotation/check.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,15 @@ func IsRotationRequired(log zerolog.Logger, cachedStatus inspectorInterface.Insp
6464
mode = SkippedRotation
6565

6666
// We are under termination
67-
if member.Conditions.IsTrue(api.ConditionTypeTerminating) || (pod != nil && pod.DeletionTimestamp != nil) {
68-
if l := utils.StringList(pod.Finalizers); l.Has(constants.FinalizerPodGracefulShutdown) && !l.Has(constants.FinalizerDelayPodTermination) {
69-
reason = "Recreation enforced by deleted state"
70-
mode = EnforcedRotation
71-
}
67+
if pod != nil {
68+
if member.Conditions.IsTrue(api.ConditionTypeTerminating) || pod.DeletionTimestamp != nil {
69+
if l := utils.StringList(pod.Finalizers); l.Has(constants.FinalizerPodGracefulShutdown) && !l.Has(constants.FinalizerDelayPodTermination) {
70+
reason = "Recreation enforced by deleted state"
71+
mode = EnforcedRotation
72+
}
7273

73-
return
74+
return
75+
}
7476
}
7577

7678
if !CheckPossible(member) {

0 commit comments

Comments
 (0)