Skip to content

Commit ac95578

Browse files
ajanikowajanikow
authored
[Bugfix] Remove finalizer during sidecar update (#1210)
1 parent c2f819c commit ac95578

File tree

7 files changed

+195
-29
lines changed

7 files changed

+195
-29
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
- (Feature) Do not change external service ports
4444
- (Bugfix) Fix Operator Debug mode
4545
- (Bugfix) Ensure NodePort wont be duplicated
46+
- (Bugfix) Remove finalizer during sidecar update
4647

4748
## [1.2.20](https://github.com/arangodb/kube-arangodb/tree/1.2.20) (2022-10-25)
4849
- (Feature) Add action progress

‎pkg/deployment/reconcile/action_cleanout_member.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func (a *actionCleanOutMember) Start(ctx context.Context) (bool, error) {
9292
// Update status
9393
m.Phase = api.MemberPhaseCleanOut
9494
m.CleanoutJobID = jobID
95-
if a.actionCtx.UpdateMember(ctx, m); err != nil {
95+
if err := a.actionCtx.UpdateMember(ctx, m); err != nil {
9696
return false, errors.WithStack(err)
9797
}
9898
return false, nil
@@ -111,6 +111,14 @@ func (a *actionCleanOutMember) CheckProgress(ctx context.Context) (bool, bool, e
111111
return true, false, nil
112112
}
113113

114+
if m.Phase == api.MemberPhaseCreated {
115+
// Restart occurred
116+
m.Phase = api.MemberPhaseCleanOut
117+
if err := a.actionCtx.UpdateMember(ctx, m); err != nil {
118+
return false, false, errors.WithStack(err)
119+
}
120+
}
121+
114122
cache, ok := a.actionCtx.GetAgencyCache()
115123
if !ok {
116124
a.log.Debug("AgencyCache is not ready")

‎pkg/deployment/reconcile/action_runtime_container_image_update.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,8 @@ func (a actionRuntimeContainerImageUpdate) CheckProgress(ctx context.Context) (b
263263
return true, false, nil
264264
}
265265

266+
groupSpec := a.actionCtx.GetSpec().GetServerGroupSpec(a.action.Group)
267+
266268
cache, ok := a.actionCtx.ACS().ClusterCache(m.ClusterID)
267269
if !ok {
268270
a.log.Info("Cluster is not ready")
@@ -275,6 +277,10 @@ func (a actionRuntimeContainerImageUpdate) CheckProgress(ctx context.Context) (b
275277
return true, false, nil
276278
}
277279

280+
if err := k8sutil.EnsureFinalizerAbsent(ctx, cache.PodsModInterface().V1(), pod, k8sutil.GetFinalizers(groupSpec, a.action.Group)...); err != nil {
281+
a.log.Err(err).Error("Unable to enforce finalizer")
282+
}
283+
278284
name, image, ok := a.getContainerDetails()
279285
if !ok {
280286
a.log.Info("Unable to find container details")

‎pkg/deployment/resources/pod_creator_arangod.go‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -493,24 +493,7 @@ func (m *MemberArangoDPod) GetInitContainers(cachedStatus interfaces.Inspector)
493493
}
494494

495495
func (m *MemberArangoDPod) GetFinalizers() []string {
496-
var finalizers []string
497-
498-
if d := m.spec.GetServerGroupSpec(m.group).GetShutdownDelay(m.group); d != 0 {
499-
finalizers = append(finalizers, constants.FinalizerDelayPodTermination)
500-
}
501-
502-
if features.GracefulShutdown().Enabled() {
503-
finalizers = append(finalizers, constants.FinalizerPodGracefulShutdown) // No need for other finalizers, quorum will be managed
504-
} else {
505-
switch m.group {
506-
case api.ServerGroupAgents:
507-
finalizers = append(finalizers, constants.FinalizerPodAgencyServing)
508-
case api.ServerGroupDBServers:
509-
finalizers = append(finalizers, constants.FinalizerPodDrainDBServer)
510-
}
511-
}
512-
513-
return finalizers
496+
return k8sutil.GetFinalizers(m.spec.GetServerGroupSpec(m.group), m.group)
514497
}
515498

516499
func (m *MemberArangoDPod) GetTolerations() []core.Toleration {

‎pkg/deployment/resources/pod_inspector.go‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ func (r *Resources) InspectPods(ctx context.Context, cachedStatus inspectorInter
126126
}
127127

128128
spec := r.context.GetSpec()
129+
groupSpec := spec.GetServerGroupSpec(group)
129130
coreContainers := spec.GetCoreContainers(group)
130131

131132
if c, ok := memberStatus.Conditions.Get(api.ConditionTypeUpdating); ok {
@@ -135,6 +136,13 @@ func (r *Resources) InspectPods(ctx context.Context, cachedStatus inspectorInter
135136
coreContainers = coreContainers.Remove(v)
136137
}
137138
}
139+
} else {
140+
// Restore gracefulness
141+
if !k8sutil.IsPodTerminating(pod) {
142+
if err := k8sutil.EnsureFinalizerPresent(ctx, cachedStatus.PodsModInterface().V1(), pod, k8sutil.GetFinalizers(groupSpec, group)...); err != nil {
143+
log.Err(err).Error("Unable to enforce finalizer")
144+
}
145+
}
138146
}
139147

140148
// Update state
@@ -487,24 +495,19 @@ func (r *Resources) InspectPods(ctx context.Context, cachedStatus inspectorInter
487495
// Create event
488496
nextInterval = nextInterval.ReduceTo(recheckSoonPodInspectorInterval)
489497
events = append(events, k8sutil.NewPodGoneEvent(podName, group.AsRole(), apiObject))
490-
updateMemberNeeded := false
491-
if m.Conditions.Update(api.ConditionTypeReady, false, "Pod Does Not Exist", "") {
492-
updateMemberNeeded = true
493-
}
498+
m.Conditions.Update(api.ConditionTypeReady, false, "Pod Does Not Exist", "")
494499
wasTerminated := m.Conditions.IsTrue(api.ConditionTypeTerminated)
495500
if m.Conditions.Update(api.ConditionTypeTerminated, true, "Pod Does Not Exist", "") {
496501
if !wasTerminated {
497502
// Record termination time
498503
now := meta.Now()
499504
m.RecentTerminations = append(m.RecentTerminations, now)
500505
}
501-
updateMemberNeeded = true
502506
}
503-
if updateMemberNeeded {
504-
// Save it
505-
if err := status.Members.Update(m, group); err != nil {
506-
return 0, errors.WithStack(err)
507-
}
507+
508+
// Save it
509+
if err := status.Members.Update(m, group); err != nil {
510+
return 0, errors.WithStack(err)
508511
}
509512
}
510513
}

‎pkg/util/k8sutil/pods.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,15 @@ import (
3434
"k8s.io/apimachinery/pkg/types"
3535
"k8s.io/apimachinery/pkg/util/json"
3636

37+
api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
3738
"github.com/arangodb/kube-arangodb/pkg/apis/shared"
39+
"github.com/arangodb/kube-arangodb/pkg/deployment/features"
40+
"github.com/arangodb/kube-arangodb/pkg/deployment/patch"
3841
"github.com/arangodb/kube-arangodb/pkg/handlers/utils"
3942
"github.com/arangodb/kube-arangodb/pkg/util"
4043
"github.com/arangodb/kube-arangodb/pkg/util/constants"
4144
"github.com/arangodb/kube-arangodb/pkg/util/errors"
45+
"github.com/arangodb/kube-arangodb/pkg/util/globals"
4246
podv1 "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector/pod/v1"
4347
"github.com/arangodb/kube-arangodb/pkg/util/k8sutil/interfaces"
4448
"github.com/arangodb/kube-arangodb/pkg/util/k8sutil/kerrors"
@@ -705,3 +709,78 @@ func CreateEnvSecretKeySelector(name, SecretKeyName, secretKey string) core.EnvV
705709
},
706710
}
707711
}
712+
713+
func EnsureFinalizerAbsent(ctx context.Context, pods podv1.Interface, pod *core.Pod, finalizers ...string) error {
714+
var newFinalizers []string
715+
716+
c := utils.StringList(finalizers)
717+
718+
for _, fn := range pod.Finalizers {
719+
if !c.Has(fn) {
720+
newFinalizers = append(newFinalizers, fn)
721+
}
722+
}
723+
724+
if len(newFinalizers) == len(pod.Finalizers) {
725+
return nil
726+
}
727+
728+
return SetFinalizers(ctx, pods, pod, newFinalizers...)
729+
}
730+
731+
func EnsureFinalizerPresent(ctx context.Context, pods podv1.Interface, pod *core.Pod, finalizers ...string) error {
732+
var newFinalizers []string
733+
734+
newFinalizers = append(newFinalizers, pod.Finalizers...)
735+
736+
for _, fn := range finalizers {
737+
if utils.StringList(newFinalizers).Has(fn) {
738+
continue
739+
}
740+
741+
newFinalizers = append(newFinalizers, fn)
742+
}
743+
744+
if len(newFinalizers) == len(pod.Finalizers) {
745+
return nil
746+
}
747+
748+
return SetFinalizers(ctx, pods, pod, newFinalizers...)
749+
}
750+
751+
func SetFinalizers(ctx context.Context, pods podv1.Interface, pod *core.Pod, finalizers ...string) error {
752+
d, err := patch.NewPatch(patch.ItemReplace(patch.NewPath("metadata", "finalizers"), finalizers)).Marshal()
753+
if err != nil {
754+
return err
755+
}
756+
757+
nctx, c := globals.GetGlobalTimeouts().Kubernetes().WithTimeout(ctx)
758+
defer c()
759+
760+
if _, err := pods.Patch(nctx, pod.GetName(), types.JSONPatchType, d, meta.PatchOptions{}); err != nil {
761+
return err
762+
}
763+
764+
return nil
765+
}
766+
767+
func GetFinalizers(spec api.ServerGroupSpec, group api.ServerGroup) []string {
768+
var finalizers []string
769+
770+
if d := spec.GetShutdownDelay(group); d != 0 {
771+
finalizers = append(finalizers, constants.FinalizerDelayPodTermination)
772+
}
773+
774+
if features.GracefulShutdown().Enabled() {
775+
finalizers = append(finalizers, constants.FinalizerPodGracefulShutdown) // No need for other finalizers, quorum will be managed
776+
} else {
777+
switch group {
778+
case api.ServerGroupAgents:
779+
finalizers = append(finalizers, constants.FinalizerPodAgencyServing)
780+
case api.ServerGroupDBServers:
781+
finalizers = append(finalizers, constants.FinalizerPodDrainDBServer)
782+
}
783+
}
784+
785+
return finalizers
786+
}

‎pkg/util/k8sutil/pods_test.go‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,17 @@
2121
package k8sutil
2222

2323
import (
24+
"context"
2425
"testing"
2526

2627
"github.com/stretchr/testify/assert"
2728
"github.com/stretchr/testify/require"
2829
core "k8s.io/api/core/v1"
30+
meta "k8s.io/apimachinery/pkg/apis/meta/v1"
2931

3032
"github.com/arangodb/kube-arangodb/pkg/handlers/utils"
33+
"github.com/arangodb/kube-arangodb/pkg/util/constants"
34+
"github.com/arangodb/kube-arangodb/pkg/util/kclient"
3135
)
3236

3337
// TestIsPodReady tests IsPodReady.
@@ -330,3 +334,85 @@ func Test_extractContainerNamesFromConditionMessage(t *testing.T) {
330334
require.NotContains(t, c, "sidecar")
331335
})
332336
}
337+
338+
func Test_EnsureFinalizer(t *testing.T) {
339+
c := kclient.NewFakeClient()
340+
341+
f := "test.arangodb.com/test"
342+
343+
pod := &core.Pod{
344+
ObjectMeta: meta.ObjectMeta{
345+
Name: "test",
346+
Namespace: "test",
347+
348+
Finalizers: nil,
349+
},
350+
}
351+
refresh := func(t *testing.T) {
352+
p, err := c.Kubernetes().CoreV1().Pods(pod.GetNamespace()).Get(context.Background(), pod.GetName(), meta.GetOptions{})
353+
require.NoError(t, err)
354+
pod = p
355+
}
356+
357+
_, err := c.Kubernetes().CoreV1().Pods(pod.GetNamespace()).Create(context.Background(), pod, meta.CreateOptions{})
358+
require.NoError(t, err)
359+
360+
t.Run("Ensure finalizers", func(t *testing.T) {
361+
require.NoError(t, EnsureFinalizerPresent(context.Background(), c.Kubernetes().CoreV1().Pods(pod.GetNamespace()), pod, constants.FinalizerPodGracefulShutdown))
362+
363+
refresh(t)
364+
365+
require.Len(t, pod.Finalizers, 1)
366+
require.Contains(t, pod.Finalizers, constants.FinalizerPodGracefulShutdown)
367+
})
368+
369+
t.Run("Add finalizers", func(t *testing.T) {
370+
require.NoError(t, EnsureFinalizerPresent(context.Background(), c.Kubernetes().CoreV1().Pods(pod.GetNamespace()), pod, f))
371+
372+
refresh(t)
373+
374+
require.Len(t, pod.Finalizers, 2)
375+
require.Contains(t, pod.Finalizers, constants.FinalizerPodGracefulShutdown)
376+
require.Contains(t, pod.Finalizers, f)
377+
})
378+
379+
t.Run("Re Add finalizers", func(t *testing.T) {
380+
require.NoError(t, EnsureFinalizerPresent(context.Background(), c.Kubernetes().CoreV1().Pods(pod.GetNamespace()), pod, f))
381+
382+
refresh(t)
383+
384+
require.Len(t, pod.Finalizers, 2)
385+
require.Contains(t, pod.Finalizers, constants.FinalizerPodGracefulShutdown)
386+
require.Contains(t, pod.Finalizers, f)
387+
})
388+
389+
t.Run("Remove finalizers", func(t *testing.T) {
390+
require.NoError(t, EnsureFinalizerAbsent(context.Background(), c.Kubernetes().CoreV1().Pods(pod.GetNamespace()), pod, f))
391+
392+
refresh(t)
393+
394+
require.Len(t, pod.Finalizers, 1)
395+
require.Contains(t, pod.Finalizers, constants.FinalizerPodGracefulShutdown)
396+
require.NotContains(t, pod.Finalizers, f)
397+
})
398+
399+
t.Run("Re - remove finalizers", func(t *testing.T) {
400+
require.NoError(t, EnsureFinalizerAbsent(context.Background(), c.Kubernetes().CoreV1().Pods(pod.GetNamespace()), pod, f))
401+
402+
refresh(t)
403+
404+
require.Len(t, pod.Finalizers, 1)
405+
require.Contains(t, pod.Finalizers, constants.FinalizerPodGracefulShutdown)
406+
require.NotContains(t, pod.Finalizers, f)
407+
})
408+
409+
t.Run("Remove final finalizers", func(t *testing.T) {
410+
require.NoError(t, EnsureFinalizerAbsent(context.Background(), c.Kubernetes().CoreV1().Pods(pod.GetNamespace()), pod, constants.FinalizerPodGracefulShutdown))
411+
412+
refresh(t)
413+
414+
require.Len(t, pod.Finalizers, 0)
415+
require.NotContains(t, pod.Finalizers, constants.FinalizerPodGracefulShutdown)
416+
require.NotContains(t, pod.Finalizers, f)
417+
})
418+
}

0 commit comments

Comments
 (0)