Skip to content

Commit d12ad72

Browse files
ewoutpewoutp
authored
Merge pull request #165 from arangodb/bugfix/deployment-finalizer
Added finalizer on deployment, used to remove child finalizers on delete
2 parents c68289f + df09665 commit d12ad72

File tree

4 files changed

+219
-91
lines changed

4 files changed

+219
-91
lines changed

‎pkg/deployment/deployment.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -345,13 +345,17 @@ func (d *Deployment) updateCRStatus(force ...bool) error {
345345
}
346346

347347
// Send update to API server
348+
ns := d.apiObject.GetNamespace()
349+
depls := d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(ns)
348350
update := d.apiObject.DeepCopy()
349351
attempt := 0
350352
for {
351353
attempt++
352354
update.Status = d.status
353-
ns := d.apiObject.GetNamespace()
354-
newAPIObject, err := d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(ns).Update(update)
355+
if update.GetDeletionTimestamp() == nil {
356+
ensureFinalizers(update)
357+
}
358+
newAPIObject, err := depls.Update(update)
355359
if err == nil {
356360
// Update internal object
357361
d.apiObject = newAPIObject
@@ -361,7 +365,7 @@ func (d *Deployment) updateCRStatus(force ...bool) error {
361365
// API object may have been changed already,
362366
// Reload api object and try again
363367
var current *api.ArangoDeployment
364-
current, err = d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(ns).Get(update.GetName(), metav1.GetOptions{})
368+
current, err = depls.Get(update.GetName(), metav1.GetOptions{})
365369
if err == nil {
366370
update = current.DeepCopy()
367371
continue

‎pkg/deployment/deployment_finalizers.go‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
//
2+
// DISCLAIMER
3+
//
4+
// Copyright 2018 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+
// Author Ewout Prangsma
21+
//
22+
23+
package deployment
24+
25+
import (
26+
"context"
27+
28+
"github.com/rs/zerolog"
29+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
31+
api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha"
32+
"github.com/arangodb/kube-arangodb/pkg/generated/clientset/versioned"
33+
"github.com/arangodb/kube-arangodb/pkg/util/constants"
34+
"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
35+
)
36+
37+
// ensureFinalizers adds all required finalizers to the given deployment (in memory).
38+
func ensureFinalizers(depl *api.ArangoDeployment) {
39+
for _, f := range depl.GetFinalizers() {
40+
if f == constants.FinalizerDeplRemoveChildFinalizers {
41+
// Finalizer already set
42+
return
43+
}
44+
}
45+
// Set finalizers
46+
depl.SetFinalizers(append(depl.GetFinalizers(), constants.FinalizerDeplRemoveChildFinalizers))
47+
}
48+
49+
// runDeploymentFinalizers goes through the list of ArangoDeployoment finalizers to see if they can be removed.
50+
func (d *Deployment) runDeploymentFinalizers(ctx context.Context) error {
51+
log := d.deps.Log
52+
var removalList []string
53+
54+
depls := d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(d.GetNamespace())
55+
updated, err := depls.Get(d.apiObject.GetName(), metav1.GetOptions{})
56+
if err != nil {
57+
return maskAny(err)
58+
}
59+
for _, f := range updated.ObjectMeta.GetFinalizers() {
60+
switch f {
61+
case constants.FinalizerDeplRemoveChildFinalizers:
62+
log.Debug().Msg("Inspecting 'remove child finalizers' finalizer")
63+
if err := d.inspectRemoveChildFinalizers(ctx, log, updated); err == nil {
64+
removalList = append(removalList, f)
65+
} else {
66+
log.Debug().Err(err).Str("finalizer", f).Msg("Cannot remove finalizer yet")
67+
}
68+
}
69+
}
70+
// Remove finalizers (if needed)
71+
if len(removalList) > 0 {
72+
if err := removeDeploymentFinalizers(log, d.deps.DatabaseCRCli, updated, removalList); err != nil {
73+
log.Debug().Err(err).Msg("Failed to update ArangoDeployment (to remove finalizers)")
74+
return maskAny(err)
75+
}
76+
}
77+
return nil
78+
}
79+
80+
// inspectRemoveChildFinalizers checks the finalizer condition for remove-child-finalizers.
81+
// It returns nil if the finalizer can be removed.
82+
func (d *Deployment) inspectRemoveChildFinalizers(ctx context.Context, log zerolog.Logger, depl *api.ArangoDeployment) error {
83+
if err := d.removePodFinalizers(); err != nil {
84+
return maskAny(err)
85+
}
86+
if err := d.removePVCFinalizers(); err != nil {
87+
return maskAny(err)
88+
}
89+
90+
return nil
91+
}
92+
93+
// removeDeploymentFinalizers removes the given finalizers from the given PVC.
94+
func removeDeploymentFinalizers(log zerolog.Logger, cli versioned.Interface, depl *api.ArangoDeployment, finalizers []string) error {
95+
depls := cli.DatabaseV1alpha().ArangoDeployments(depl.GetNamespace())
96+
getFunc := func() (metav1.Object, error) {
97+
result, err := depls.Get(depl.GetName(), metav1.GetOptions{})
98+
if err != nil {
99+
return nil, maskAny(err)
100+
}
101+
return result, nil
102+
}
103+
updateFunc := func(updated metav1.Object) error {
104+
updatedDepl := updated.(*api.ArangoDeployment)
105+
result, err := depls.Update(updatedDepl)
106+
if err != nil {
107+
return maskAny(err)
108+
}
109+
*depl = *result
110+
return nil
111+
}
112+
if err := k8sutil.RemoveFinalizers(log, finalizers, getFunc, updateFunc); err != nil {
113+
return maskAny(err)
114+
}
115+
return nil
116+
}

‎pkg/deployment/deployment_inspector.go‎

Lines changed: 91 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -46,105 +46,112 @@ func (d *Deployment) inspectDeployment(lastInterval time.Duration) time.Duration
4646
ctx := context.Background()
4747

4848
// Check deployment still exists
49-
if _, err := d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(d.apiObject.GetNamespace()).Get(d.apiObject.GetName(), metav1.GetOptions{}); k8sutil.IsNotFound(err) {
49+
updated, err := d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(d.apiObject.GetNamespace()).Get(d.apiObject.GetName(), metav1.GetOptions{})
50+
if k8sutil.IsNotFound(err) {
5051
// Deployment is gone
5152
log.Info().Msg("Deployment is gone")
5253
d.Delete()
5354
return nextInterval
54-
}
55-
56-
// Is the deployment in failed state, if so, give up.
57-
if d.status.Phase == api.DeploymentPhaseFailed {
58-
log.Debug().Msg("Deployment is in Failed state.")
59-
return nextInterval
60-
}
55+
} else if updated != nil && updated.GetDeletionTimestamp() != nil {
56+
// Deployment is marked for deletion
57+
if err := d.runDeploymentFinalizers(ctx); err != nil {
58+
hasError = true
59+
d.CreateEvent(k8sutil.NewErrorEvent("ArangoDeployment finalizer inspection failed", err, d.apiObject))
60+
}
61+
} else {
62+
// Is the deployment in failed state, if so, give up.
63+
if d.status.Phase == api.DeploymentPhaseFailed {
64+
log.Debug().Msg("Deployment is in Failed state.")
65+
return nextInterval
66+
}
6167

62-
// Inspect secret hashes
63-
if err := d.resources.ValidateSecretHashes(); err != nil {
64-
hasError = true
65-
d.CreateEvent(k8sutil.NewErrorEvent("Secret hash validation failed", err, d.apiObject))
66-
}
68+
// Inspect secret hashes
69+
if err := d.resources.ValidateSecretHashes(); err != nil {
70+
hasError = true
71+
d.CreateEvent(k8sutil.NewErrorEvent("Secret hash validation failed", err, d.apiObject))
72+
}
6773

68-
// Is the deployment in a good state?
69-
if d.status.Conditions.IsTrue(api.ConditionTypeSecretsChanged) {
70-
log.Debug().Msg("Condition SecretsChanged is true. Revert secrets before we can continue")
71-
return nextInterval
72-
}
74+
// Is the deployment in a good state?
75+
if d.status.Conditions.IsTrue(api.ConditionTypeSecretsChanged) {
76+
log.Debug().Msg("Condition SecretsChanged is true. Revert secrets before we can continue")
77+
return nextInterval
78+
}
7379

74-
// Ensure we have image info
75-
if retrySoon, err := d.ensureImages(d.apiObject); err != nil {
76-
hasError = true
77-
d.CreateEvent(k8sutil.NewErrorEvent("Image detection failed", err, d.apiObject))
78-
} else if retrySoon {
79-
nextInterval = minInspectionInterval
80-
}
80+
// Ensure we have image info
81+
if retrySoon, err := d.ensureImages(d.apiObject); err != nil {
82+
hasError = true
83+
d.CreateEvent(k8sutil.NewErrorEvent("Image detection failed", err, d.apiObject))
84+
} else if retrySoon {
85+
nextInterval = minInspectionInterval
86+
}
8187

82-
// Inspection of generated resources needed
83-
if err := d.resources.InspectPods(ctx); err != nil {
84-
hasError = true
85-
d.CreateEvent(k8sutil.NewErrorEvent("Pod inspection failed", err, d.apiObject))
86-
}
87-
if err := d.resources.InspectPVCs(ctx); err != nil {
88-
hasError = true
89-
d.CreateEvent(k8sutil.NewErrorEvent("PVC inspection failed", err, d.apiObject))
90-
}
88+
// Inspection of generated resources needed
89+
if err := d.resources.InspectPods(ctx); err != nil {
90+
hasError = true
91+
d.CreateEvent(k8sutil.NewErrorEvent("Pod inspection failed", err, d.apiObject))
92+
}
93+
if err := d.resources.InspectPVCs(ctx); err != nil {
94+
hasError = true
95+
d.CreateEvent(k8sutil.NewErrorEvent("PVC inspection failed", err, d.apiObject))
96+
}
9197

92-
// Check members for resilience
93-
if err := d.resilience.CheckMemberFailure(); err != nil {
94-
hasError = true
95-
d.CreateEvent(k8sutil.NewErrorEvent("Member failure detection failed", err, d.apiObject))
96-
}
98+
// Check members for resilience
99+
if err := d.resilience.CheckMemberFailure(); err != nil {
100+
hasError = true
101+
d.CreateEvent(k8sutil.NewErrorEvent("Member failure detection failed", err, d.apiObject))
102+
}
97103

98-
// Create scale/update plan
99-
if err := d.reconciler.CreatePlan(); err != nil {
100-
hasError = true
101-
d.CreateEvent(k8sutil.NewErrorEvent("Plan creation failed", err, d.apiObject))
102-
}
104+
// Create scale/update plan
105+
if err := d.reconciler.CreatePlan(); err != nil {
106+
hasError = true
107+
d.CreateEvent(k8sutil.NewErrorEvent("Plan creation failed", err, d.apiObject))
108+
}
103109

104-
// Execute current step of scale/update plan
105-
retrySoon, err := d.reconciler.ExecutePlan(ctx)
106-
if err != nil {
107-
hasError = true
108-
d.CreateEvent(k8sutil.NewErrorEvent("Plan execution failed", err, d.apiObject))
109-
}
110-
if retrySoon {
111-
nextInterval = minInspectionInterval
112-
}
110+
// Execute current step of scale/update plan
111+
retrySoon, err := d.reconciler.ExecutePlan(ctx)
112+
if err != nil {
113+
hasError = true
114+
d.CreateEvent(k8sutil.NewErrorEvent("Plan execution failed", err, d.apiObject))
115+
}
116+
if retrySoon {
117+
nextInterval = minInspectionInterval
118+
}
113119

114-
// Ensure all resources are created
115-
if err := d.resources.EnsureSecrets(); err != nil {
116-
hasError = true
117-
d.CreateEvent(k8sutil.NewErrorEvent("Secret creation failed", err, d.apiObject))
118-
}
119-
if err := d.resources.EnsureServices(); err != nil {
120-
hasError = true
121-
d.CreateEvent(k8sutil.NewErrorEvent("Service creation failed", err, d.apiObject))
122-
}
123-
if err := d.resources.EnsurePVCs(); err != nil {
124-
hasError = true
125-
d.CreateEvent(k8sutil.NewErrorEvent("PVC creation failed", err, d.apiObject))
126-
}
127-
if err := d.resources.EnsurePods(); err != nil {
128-
hasError = true
129-
d.CreateEvent(k8sutil.NewErrorEvent("Pod creation failed", err, d.apiObject))
130-
}
120+
// Ensure all resources are created
121+
if err := d.resources.EnsureSecrets(); err != nil {
122+
hasError = true
123+
d.CreateEvent(k8sutil.NewErrorEvent("Secret creation failed", err, d.apiObject))
124+
}
125+
if err := d.resources.EnsureServices(); err != nil {
126+
hasError = true
127+
d.CreateEvent(k8sutil.NewErrorEvent("Service creation failed", err, d.apiObject))
128+
}
129+
if err := d.resources.EnsurePVCs(); err != nil {
130+
hasError = true
131+
d.CreateEvent(k8sutil.NewErrorEvent("PVC creation failed", err, d.apiObject))
132+
}
133+
if err := d.resources.EnsurePods(); err != nil {
134+
hasError = true
135+
d.CreateEvent(k8sutil.NewErrorEvent("Pod creation failed", err, d.apiObject))
136+
}
131137

132-
// Create access packages
133-
if err := d.createAccessPackages(); err != nil {
134-
hasError = true
135-
d.CreateEvent(k8sutil.NewErrorEvent("AccessPackage creation failed", err, d.apiObject))
136-
}
138+
// Create access packages
139+
if err := d.createAccessPackages(); err != nil {
140+
hasError = true
141+
d.CreateEvent(k8sutil.NewErrorEvent("AccessPackage creation failed", err, d.apiObject))
142+
}
137143

138-
// Inspect deployment for obsolete members
139-
if err := d.resources.CleanupRemovedMembers(); err != nil {
140-
hasError = true
141-
d.CreateEvent(k8sutil.NewErrorEvent("Removed member cleanup failed", err, d.apiObject))
142-
}
144+
// Inspect deployment for obsolete members
145+
if err := d.resources.CleanupRemovedMembers(); err != nil {
146+
hasError = true
147+
d.CreateEvent(k8sutil.NewErrorEvent("Removed member cleanup failed", err, d.apiObject))
148+
}
143149

144-
// At the end of the inspect, we cleanup terminated pods.
145-
if err := d.resources.CleanupTerminatedPods(); err != nil {
146-
hasError = true
147-
d.CreateEvent(k8sutil.NewErrorEvent("Pod cleanup failed", err, d.apiObject))
150+
// At the end of the inspect, we cleanup terminated pods.
151+
if err := d.resources.CleanupTerminatedPods(); err != nil {
152+
hasError = true
153+
d.CreateEvent(k8sutil.NewErrorEvent("Pod cleanup failed", err, d.apiObject))
154+
}
148155
}
149156

150157
// Update next interval (on errors)

‎pkg/util/constants/constants.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,11 @@ const (
4444

4545
SecretAccessPackageYaml = "accessPackage.yaml" // Key in Secret.data used to store a YAML encoded access package
4646

47-
FinalizerPodDrainDBServer = "dbserver.database.arangodb.com/drain" // Finalizer added to DBServers, indicating the need for draining that dbserver
48-
FinalizerPodAgencyServing = "agent.database.arangodb.com/agency-serving" // Finalizer added to Agents, indicating the need for keeping enough agents alive
49-
FinalizerPVCMemberExists = "pvc.database.arangodb.com/member-exists" // Finalizer added to PVCs, indicating the need to keep is as long as its member exists
50-
FinalizerDeplReplStopSync = "replication.database.arangodb.com/stop-sync" // Finalizer added to ArangoDeploymentReplication, indicating the need to stop synchronization
47+
FinalizerDeplRemoveChildFinalizers = "database.arangodb.com/remove-child-finalizers" // Finalizer added to ArangoDeployment, indicating the need to remove finalizers from all children
48+
FinalizerDeplReplStopSync = "replication.database.arangodb.com/stop-sync" // Finalizer added to ArangoDeploymentReplication, indicating the need to stop synchronization
49+
FinalizerPodAgencyServing = "agent.database.arangodb.com/agency-serving" // Finalizer added to Agents, indicating the need for keeping enough agents alive
50+
FinalizerPodDrainDBServer = "dbserver.database.arangodb.com/drain" // Finalizer added to DBServers, indicating the need for draining that dbserver
51+
FinalizerPVCMemberExists = "pvc.database.arangodb.com/member-exists" // Finalizer added to PVCs, indicating the need to keep is as long as its member exists
5152

5253
AnnotationEnforceAntiAffinity = "database.arangodb.com/enforce-anti-affinity" // Key of annotation added to PVC. Value is a boolean "true" or "false"
5354
)

0 commit comments

Comments
 (0)