From 9f8f41adb4e4edee0d45a628d272be8c519c43af Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 7 Jun 2018 16:22:02 +0200 Subject: [PATCH] Added finalizer on deployment, used to remove child finalizers on delete --- pkg/deployment/deployment.go | 10 +- pkg/deployment/deployment_finalizers.go | 116 ++++++++++++++++ pkg/deployment/deployment_inspector.go | 175 ++++++++++++------------ pkg/util/constants/constants.go | 9 +- 4 files changed, 219 insertions(+), 91 deletions(-) create mode 100644 pkg/deployment/deployment_finalizers.go diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go index aa17ad492..09b7a53f9 100644 --- a/pkg/deployment/deployment.go +++ b/pkg/deployment/deployment.go @@ -345,13 +345,17 @@ func (d *Deployment) updateCRStatus(force ...bool) error { } // Send update to API server + ns := d.apiObject.GetNamespace() + depls := d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(ns) update := d.apiObject.DeepCopy() attempt := 0 for { attempt++ update.Status = d.status - ns := d.apiObject.GetNamespace() - newAPIObject, err := d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(ns).Update(update) + if update.GetDeletionTimestamp() == nil { + ensureFinalizers(update) + } + newAPIObject, err := depls.Update(update) if err == nil { // Update internal object d.apiObject = newAPIObject @@ -361,7 +365,7 @@ func (d *Deployment) updateCRStatus(force ...bool) error { // API object may have been changed already, // Reload api object and try again var current *api.ArangoDeployment - current, err = d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(ns).Get(update.GetName(), metav1.GetOptions{}) + current, err = depls.Get(update.GetName(), metav1.GetOptions{}) if err == nil { update = current.DeepCopy() continue diff --git a/pkg/deployment/deployment_finalizers.go b/pkg/deployment/deployment_finalizers.go new file mode 100644 index 000000000..ced9504cc --- /dev/null +++ b/pkg/deployment/deployment_finalizers.go @@ -0,0 +1,116 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 +// +// Author Ewout Prangsma +// + +package deployment + +import ( + "context" + + "github.com/rs/zerolog" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/arangodb/kube-arangodb/pkg/generated/clientset/versioned" + "github.com/arangodb/kube-arangodb/pkg/util/constants" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" +) + +// ensureFinalizers adds all required finalizers to the given deployment (in memory). +func ensureFinalizers(depl *api.ArangoDeployment) { + for _, f := range depl.GetFinalizers() { + if f == constants.FinalizerDeplRemoveChildFinalizers { + // Finalizer already set + return + } + } + // Set finalizers + depl.SetFinalizers(append(depl.GetFinalizers(), constants.FinalizerDeplRemoveChildFinalizers)) +} + +// runDeploymentFinalizers goes through the list of ArangoDeployoment finalizers to see if they can be removed. +func (d *Deployment) runDeploymentFinalizers(ctx context.Context) error { + log := d.deps.Log + var removalList []string + + depls := d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(d.GetNamespace()) + updated, err := depls.Get(d.apiObject.GetName(), metav1.GetOptions{}) + if err != nil { + return maskAny(err) + } + for _, f := range updated.ObjectMeta.GetFinalizers() { + switch f { + case constants.FinalizerDeplRemoveChildFinalizers: + log.Debug().Msg("Inspecting 'remove child finalizers' finalizer") + if err := d.inspectRemoveChildFinalizers(ctx, log, updated); err == nil { + removalList = append(removalList, f) + } else { + log.Debug().Err(err).Str("finalizer", f).Msg("Cannot remove finalizer yet") + } + } + } + // Remove finalizers (if needed) + if len(removalList) > 0 { + if err := removeDeploymentFinalizers(log, d.deps.DatabaseCRCli, updated, removalList); err != nil { + log.Debug().Err(err).Msg("Failed to update ArangoDeployment (to remove finalizers)") + return maskAny(err) + } + } + return nil +} + +// inspectRemoveChildFinalizers checks the finalizer condition for remove-child-finalizers. +// It returns nil if the finalizer can be removed. +func (d *Deployment) inspectRemoveChildFinalizers(ctx context.Context, log zerolog.Logger, depl *api.ArangoDeployment) error { + if err := d.removePodFinalizers(); err != nil { + return maskAny(err) + } + if err := d.removePVCFinalizers(); err != nil { + return maskAny(err) + } + + return nil +} + +// removeDeploymentFinalizers removes the given finalizers from the given PVC. +func removeDeploymentFinalizers(log zerolog.Logger, cli versioned.Interface, depl *api.ArangoDeployment, finalizers []string) error { + depls := cli.DatabaseV1alpha().ArangoDeployments(depl.GetNamespace()) + getFunc := func() (metav1.Object, error) { + result, err := depls.Get(depl.GetName(), metav1.GetOptions{}) + if err != nil { + return nil, maskAny(err) + } + return result, nil + } + updateFunc := func(updated metav1.Object) error { + updatedDepl := updated.(*api.ArangoDeployment) + result, err := depls.Update(updatedDepl) + if err != nil { + return maskAny(err) + } + *depl = *result + return nil + } + if err := k8sutil.RemoveFinalizers(log, finalizers, getFunc, updateFunc); err != nil { + return maskAny(err) + } + return nil +} diff --git a/pkg/deployment/deployment_inspector.go b/pkg/deployment/deployment_inspector.go index 3171974a4..dc67debc9 100644 --- a/pkg/deployment/deployment_inspector.go +++ b/pkg/deployment/deployment_inspector.go @@ -46,105 +46,112 @@ func (d *Deployment) inspectDeployment(lastInterval time.Duration) time.Duration ctx := context.Background() // Check deployment still exists - if _, err := d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(d.apiObject.GetNamespace()).Get(d.apiObject.GetName(), metav1.GetOptions{}); k8sutil.IsNotFound(err) { + updated, err := d.deps.DatabaseCRCli.DatabaseV1alpha().ArangoDeployments(d.apiObject.GetNamespace()).Get(d.apiObject.GetName(), metav1.GetOptions{}) + if k8sutil.IsNotFound(err) { // Deployment is gone log.Info().Msg("Deployment is gone") d.Delete() return nextInterval - } + } else if updated != nil && updated.GetDeletionTimestamp() != nil { + // Deployment is marked for deletion + if err := d.runDeploymentFinalizers(ctx); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("ArangoDeployment finalizer inspection failed", err, d.apiObject)) + } + } else { + // Is the deployment in failed state, if so, give up. + if d.status.Phase == api.DeploymentPhaseFailed { + log.Debug().Msg("Deployment is in Failed state.") + return nextInterval + } - // Is the deployment in failed state, if so, give up. - if d.status.Phase == api.DeploymentPhaseFailed { - log.Debug().Msg("Deployment is in Failed state.") - return nextInterval - } + // Inspect secret hashes + if err := d.resources.ValidateSecretHashes(); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Secret hash validation failed", err, d.apiObject)) + } - // Inspect secret hashes - if err := d.resources.ValidateSecretHashes(); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Secret hash validation failed", err, d.apiObject)) - } + // Is the deployment in a good state? + if d.status.Conditions.IsTrue(api.ConditionTypeSecretsChanged) { + log.Debug().Msg("Condition SecretsChanged is true. Revert secrets before we can continue") + return nextInterval + } - // Is the deployment in a good state? - if d.status.Conditions.IsTrue(api.ConditionTypeSecretsChanged) { - log.Debug().Msg("Condition SecretsChanged is true. Revert secrets before we can continue") - return nextInterval - } + // Ensure we have image info + if retrySoon, err := d.ensureImages(d.apiObject); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Image detection failed", err, d.apiObject)) + } else if retrySoon { + nextInterval = minInspectionInterval + } - // Ensure we have image info - if retrySoon, err := d.ensureImages(d.apiObject); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Image detection failed", err, d.apiObject)) - } else if retrySoon { - nextInterval = minInspectionInterval - } + // Inspection of generated resources needed + if err := d.resources.InspectPods(ctx); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Pod inspection failed", err, d.apiObject)) + } + if err := d.resources.InspectPVCs(ctx); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("PVC inspection failed", err, d.apiObject)) + } - // Inspection of generated resources needed - if err := d.resources.InspectPods(ctx); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Pod inspection failed", err, d.apiObject)) - } - if err := d.resources.InspectPVCs(ctx); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("PVC inspection failed", err, d.apiObject)) - } + // Check members for resilience + if err := d.resilience.CheckMemberFailure(); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Member failure detection failed", err, d.apiObject)) + } - // Check members for resilience - if err := d.resilience.CheckMemberFailure(); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Member failure detection failed", err, d.apiObject)) - } + // Create scale/update plan + if err := d.reconciler.CreatePlan(); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Plan creation failed", err, d.apiObject)) + } - // Create scale/update plan - if err := d.reconciler.CreatePlan(); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Plan creation failed", err, d.apiObject)) - } + // Execute current step of scale/update plan + retrySoon, err := d.reconciler.ExecutePlan(ctx) + if err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Plan execution failed", err, d.apiObject)) + } + if retrySoon { + nextInterval = minInspectionInterval + } - // Execute current step of scale/update plan - retrySoon, err := d.reconciler.ExecutePlan(ctx) - if err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Plan execution failed", err, d.apiObject)) - } - if retrySoon { - nextInterval = minInspectionInterval - } + // Ensure all resources are created + if err := d.resources.EnsureSecrets(); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Secret creation failed", err, d.apiObject)) + } + if err := d.resources.EnsureServices(); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Service creation failed", err, d.apiObject)) + } + if err := d.resources.EnsurePVCs(); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("PVC creation failed", err, d.apiObject)) + } + if err := d.resources.EnsurePods(); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Pod creation failed", err, d.apiObject)) + } - // Ensure all resources are created - if err := d.resources.EnsureSecrets(); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Secret creation failed", err, d.apiObject)) - } - if err := d.resources.EnsureServices(); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Service creation failed", err, d.apiObject)) - } - if err := d.resources.EnsurePVCs(); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("PVC creation failed", err, d.apiObject)) - } - if err := d.resources.EnsurePods(); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Pod creation failed", err, d.apiObject)) - } + // Create access packages + if err := d.createAccessPackages(); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("AccessPackage creation failed", err, d.apiObject)) + } - // Create access packages - if err := d.createAccessPackages(); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("AccessPackage creation failed", err, d.apiObject)) - } + // Inspect deployment for obsolete members + if err := d.resources.CleanupRemovedMembers(); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Removed member cleanup failed", err, d.apiObject)) + } - // Inspect deployment for obsolete members - if err := d.resources.CleanupRemovedMembers(); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Removed member cleanup failed", err, d.apiObject)) - } - - // At the end of the inspect, we cleanup terminated pods. - if err := d.resources.CleanupTerminatedPods(); err != nil { - hasError = true - d.CreateEvent(k8sutil.NewErrorEvent("Pod cleanup failed", err, d.apiObject)) + // At the end of the inspect, we cleanup terminated pods. + if err := d.resources.CleanupTerminatedPods(); err != nil { + hasError = true + d.CreateEvent(k8sutil.NewErrorEvent("Pod cleanup failed", err, d.apiObject)) + } } // Update next interval (on errors) diff --git a/pkg/util/constants/constants.go b/pkg/util/constants/constants.go index 8c43f6d79..3d2a7b594 100644 --- a/pkg/util/constants/constants.go +++ b/pkg/util/constants/constants.go @@ -44,8 +44,9 @@ const ( SecretAccessPackageYaml = "accessPackage.yaml" // Key in Secret.data used to store a YAML encoded access package - FinalizerPodDrainDBServer = "dbserver.database.arangodb.com/drain" // Finalizer added to DBServers, indicating the need for draining that dbserver - FinalizerPodAgencyServing = "agent.database.arangodb.com/agency-serving" // Finalizer added to Agents, indicating the need for keeping enough agents alive - FinalizerPVCMemberExists = "pvc.database.arangodb.com/member-exists" // Finalizer added to PVCs, indicating the need to keep is as long as its member exists - FinalizerDeplReplStopSync = "replication.database.arangodb.com/stop-sync" // Finalizer added to ArangoDeploymentReplication, indicating the need to stop synchronization + FinalizerPodDrainDBServer = "dbserver.database.arangodb.com/drain" // Finalizer added to DBServers, indicating the need for draining that dbserver + FinalizerPodAgencyServing = "agent.database.arangodb.com/agency-serving" // Finalizer added to Agents, indicating the need for keeping enough agents alive + FinalizerPVCMemberExists = "pvc.database.arangodb.com/member-exists" // Finalizer added to PVCs, indicating the need to keep is as long as its member exists + FinalizerDeplRemoveChildFinalizers = "database.arangodb.com/remove-child-finalizers" // Finalizer added to ArangoDeployment, indicating the need to remove finalizers from all children + FinalizerDeplReplStopSync = "replication.database.arangodb.com/stop-sync" // Finalizer added to ArangoDeploymentReplication, indicating the need to stop synchronization )