From 7a416e524dec6dd2299dc75200c968247271a9fe Mon Sep 17 00:00:00 2001 From: Adam Janikowski <12255597+ajanikow@users.noreply.github.com> Date: Wed, 21 Sep 2022 15:01:58 +0200 Subject: [PATCH] [Feature] Detach PVC in Ordered indexing (#1118) --- CHANGELOG.md | 1 + pkg/apis/deployment/v1/deployment.go | 5 ++ pkg/apis/deployment/v2alpha1/deployment.go | 5 ++ pkg/apis/replication/v1/replication.go | 5 ++ pkg/apis/replication/v2alpha1/replication.go | 5 ++ pkg/apis/storage/v1alpha/local_storage.go | 6 +++ pkg/deployment/reconcile/plan_builder_tls.go | 3 +- pkg/deployment/resources/annotations.go | 19 +++---- pkg/deployment/resources/pvc_inspector.go | 51 ++++++++++++++++++- pkg/deployment/resources/pvcs.go | 10 +++- pkg/util/k8sutil/events.go | 2 + pkg/util/k8sutil/finalizers.go | 40 +++++++++++++++ .../{annotations.go => tools/owner.go} | 22 +------- pkg/util/k8sutil/util.go | 34 +++++++++++++ 14 files changed, 173 insertions(+), 35 deletions(-) rename pkg/util/k8sutil/{annotations.go => tools/owner.go} (54%) diff --git a/CHANGELOG.md b/CHANGELOG.md index c842b897f..6a9ab1e17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - (Bugfix) Fix and document action timeouts - (Feature) Propagate sidecars' ports to a member's service - (Debug Package) Initial commit +- (Feature) Detach PVC from deployment in Ordered indexing method ## [1.2.16](https://github.com/arangodb/kube-arangodb/tree/1.2.16) (2022-09-14) - (Feature) Add ArangoDeployment ServerGroupStatus diff --git a/pkg/apis/deployment/v1/deployment.go b/pkg/apis/deployment/v1/deployment.go index 458f042a3..4f5d186b8 100644 --- a/pkg/apis/deployment/v1/deployment.go +++ b/pkg/apis/deployment/v1/deployment.go @@ -25,6 +25,7 @@ import ( "github.com/arangodb/kube-arangodb/pkg/apis/deployment" "github.com/arangodb/kube-arangodb/pkg/util/errors" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/tools" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -49,6 +50,10 @@ type ArangoDeployment struct { Status DeploymentStatus `json:"status,omitempty"` } +func (d *ArangoDeployment) OwnerOf(in meta.Object) bool { + return tools.IsOwner(d.AsOwner(), in) +} + type ServerGroupFunc func(ServerGroup, ServerGroupSpec, *MemberStatusList) error // AsOwner creates an OwnerReference for the given deployment diff --git a/pkg/apis/deployment/v2alpha1/deployment.go b/pkg/apis/deployment/v2alpha1/deployment.go index 554b0b0a9..11c4326ce 100644 --- a/pkg/apis/deployment/v2alpha1/deployment.go +++ b/pkg/apis/deployment/v2alpha1/deployment.go @@ -25,6 +25,7 @@ import ( "github.com/arangodb/kube-arangodb/pkg/apis/deployment" "github.com/arangodb/kube-arangodb/pkg/util/errors" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/tools" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -49,6 +50,10 @@ type ArangoDeployment struct { Status DeploymentStatus `json:"status,omitempty"` } +func (d *ArangoDeployment) OwnerOf(in meta.Object) bool { + return tools.IsOwner(d.AsOwner(), in) +} + type ServerGroupFunc func(ServerGroup, ServerGroupSpec, *MemberStatusList) error // AsOwner creates an OwnerReference for the given deployment diff --git a/pkg/apis/replication/v1/replication.go b/pkg/apis/replication/v1/replication.go index 39071b279..837ad19db 100644 --- a/pkg/apis/replication/v1/replication.go +++ b/pkg/apis/replication/v1/replication.go @@ -24,6 +24,7 @@ import ( meta "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/arangodb/kube-arangodb/pkg/apis/replication" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/tools" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -49,6 +50,10 @@ type ArangoDeploymentReplication struct { Status DeploymentReplicationStatus `json:"status"` } +func (d *ArangoDeploymentReplication) OwnerOf(in meta.Object) bool { + return tools.IsOwner(d.AsOwner(), in) +} + // AsOwner creates an OwnerReference for the given replication func (d *ArangoDeploymentReplication) AsOwner() meta.OwnerReference { trueVar := true diff --git a/pkg/apis/replication/v2alpha1/replication.go b/pkg/apis/replication/v2alpha1/replication.go index 60edc912a..d358657cf 100644 --- a/pkg/apis/replication/v2alpha1/replication.go +++ b/pkg/apis/replication/v2alpha1/replication.go @@ -24,6 +24,7 @@ import ( meta "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/arangodb/kube-arangodb/pkg/apis/replication" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/tools" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -49,6 +50,10 @@ type ArangoDeploymentReplication struct { Status DeploymentReplicationStatus `json:"status"` } +func (d *ArangoDeploymentReplication) OwnerOf(in meta.Object) bool { + return tools.IsOwner(d.AsOwner(), in) +} + // AsOwner creates an OwnerReference for the given replication func (d *ArangoDeploymentReplication) AsOwner() meta.OwnerReference { trueVar := true diff --git a/pkg/apis/storage/v1alpha/local_storage.go b/pkg/apis/storage/v1alpha/local_storage.go index c9f1d9435..6cef64b1c 100644 --- a/pkg/apis/storage/v1alpha/local_storage.go +++ b/pkg/apis/storage/v1alpha/local_storage.go @@ -22,6 +22,8 @@ package v1alpha import ( meta "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/tools" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -48,6 +50,10 @@ type ArangoLocalStorage struct { Status LocalStorageStatus `json:"status"` } +func (d *ArangoLocalStorage) OwnerOf(in meta.Object) bool { + return tools.IsOwner(d.AsOwner(), in) +} + // AsOwner creates an OwnerReference for the given storage func (d *ArangoLocalStorage) AsOwner() meta.OwnerReference { return meta.OwnerReference{ diff --git a/pkg/deployment/reconcile/plan_builder_tls.go b/pkg/deployment/reconcile/plan_builder_tls.go index 99ba78ce9..1c11576e7 100644 --- a/pkg/deployment/reconcile/plan_builder_tls.go +++ b/pkg/deployment/reconcile/plan_builder_tls.go @@ -43,6 +43,7 @@ import ( "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector" memberTls "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/tls" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/tools" ) const CertificateRenewalMargin = 7 * 24 * time.Hour @@ -207,7 +208,7 @@ func (r *Reconciler) createCARenewalPlan(ctx context.Context, apiObject k8sutil. return nil } - if !k8sutil.IsOwner(apiObject.AsOwner(), caSecret) { + if !tools.IsOwner(apiObject.AsOwner(), caSecret) { r.planLogger.Str("secret", spec.TLS.GetCASecretName()).Debug("CA Secret is not owned by Operator, we wont do anything") return nil } diff --git a/pkg/deployment/resources/annotations.go b/pkg/deployment/resources/annotations.go index ea5c3ce41..2761b4720 100644 --- a/pkg/deployment/resources/annotations.go +++ b/pkg/deployment/resources/annotations.go @@ -37,6 +37,7 @@ import ( "github.com/arangodb/kube-arangodb/pkg/util/globals" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/tools" ) type PatchFunc func(name string, d []byte) error @@ -180,7 +181,7 @@ func (r *Resources) ensureSecretsAnnotations(patch PatchFunc, cachedStatus inspe r.ensureAnnotationsMap(secret.Kind, secret, spec, patch) return nil }, func(secret *core.Secret) bool { - return k8sutil.IsChildResource(kind, name, namespace, secret) + return tools.IsChildResource(kind, name, namespace, secret) }); err != nil { return err } @@ -193,7 +194,7 @@ func (r *Resources) ensureServiceAccountsAnnotations(patch PatchFunc, cachedStat r.ensureAnnotationsMap(serviceAccount.Kind, serviceAccount, spec, patch) return nil }, func(serviceAccount *core.ServiceAccount) bool { - return k8sutil.IsChildResource(kind, name, namespace, serviceAccount) + return tools.IsChildResource(kind, name, namespace, serviceAccount) }); err != nil { return err } @@ -206,7 +207,7 @@ func (r *Resources) ensureServicesAnnotations(patch PatchFunc, cachedStatus insp r.ensureAnnotationsMap(service.Kind, service, spec, patch) return nil }, func(service *core.Service) bool { - return k8sutil.IsChildResource(kind, name, namespace, service) + return tools.IsChildResource(kind, name, namespace, service) }); err != nil { return err } @@ -221,7 +222,7 @@ func (r *Resources) ensurePdbsAnnotations(patch PatchFunc, cachedStatus inspecto r.ensureAnnotationsMap(podDisruptionBudget.Kind, podDisruptionBudget, spec, patch) return nil }, func(podDisruptionBudget *policyv1.PodDisruptionBudget) bool { - return k8sutil.IsChildResource(kind, name, namespace, podDisruptionBudget) + return tools.IsChildResource(kind, name, namespace, podDisruptionBudget) }); err != nil { return err } @@ -237,7 +238,7 @@ func (r *Resources) ensurePdbsAnnotations(patch PatchFunc, cachedStatus inspecto r.ensureAnnotationsMap(podDisruptionBudget.Kind, podDisruptionBudget, spec, patch) return nil }, func(podDisruptionBudget *policyv1beta1.PodDisruptionBudget) bool { - return k8sutil.IsChildResource(kind, name, namespace, podDisruptionBudget) + return tools.IsChildResource(kind, name, namespace, podDisruptionBudget) }); err != nil { return err } @@ -250,7 +251,7 @@ func (r *Resources) ensurePvcsAnnotations(patch PatchFunc, cachedStatus inspecto r.ensureGroupAnnotationsMap(persistentVolumeClaim.Kind, persistentVolumeClaim, spec, patch) return nil }, func(persistentVolumeClaim *core.PersistentVolumeClaim) bool { - return k8sutil.IsChildResource(kind, name, namespace, persistentVolumeClaim) + return tools.IsChildResource(kind, name, namespace, persistentVolumeClaim) }); err != nil { return err } @@ -270,7 +271,7 @@ func (r *Resources) ensureServiceMonitorsAnnotations(patch PatchFunc, cachedStat r.ensureAnnotationsMap(serviceMonitor.Kind, serviceMonitor, spec, patch) return nil }, func(serviceMonitor *monitoring.ServiceMonitor) bool { - return k8sutil.IsChildResource(kind, name, namespace, serviceMonitor) + return tools.IsChildResource(kind, name, namespace, serviceMonitor) }); err != nil { return err } @@ -298,7 +299,7 @@ func (r *Resources) ensurePodsAnnotations(patch PatchFunc, cachedStatus inspecto r.ensureGroupAnnotationsMap(pod.Kind, pod, spec, patch) return nil }, func(pod *core.Pod) bool { - return k8sutil.IsChildResource(kind, name, namespace, pod) + return tools.IsChildResource(kind, name, namespace, pod) }); err != nil { return err } @@ -307,7 +308,7 @@ func (r *Resources) ensurePodsAnnotations(patch PatchFunc, cachedStatus inspecto } func (r *Resources) isChildResource(obj meta.Object) bool { - return k8sutil.IsChildResource(deployment.ArangoDeploymentResourceKind, + return tools.IsChildResource(deployment.ArangoDeploymentResourceKind, r.context.GetAPIObject().GetName(), r.context.GetAPIObject().GetNamespace(), obj) diff --git a/pkg/deployment/resources/pvc_inspector.go b/pkg/deployment/resources/pvc_inspector.go index 984a8c03d..fed43888f 100644 --- a/pkg/deployment/resources/pvc_inspector.go +++ b/pkg/deployment/resources/pvc_inspector.go @@ -28,6 +28,7 @@ import ( meta "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/arangodb/kube-arangodb/pkg/deployment/patch" "github.com/arangodb/kube-arangodb/pkg/metrics" "github.com/arangodb/kube-arangodb/pkg/util" @@ -66,6 +67,12 @@ func (r *Resources) InspectPVCs(ctx context.Context, cachedStatus inspectorInter memberStatus, group, found := status.Members.MemberStatusByPVCName(pvc.GetName()) if !found { log.Str("pvc", pvc.GetName()).Debug("no memberstatus found for PVC") + + if !r.context.GetAPIObject().OwnerOf(pvc) { + log.Str("pvc", pvc.GetName()).Debug("PVC is not owned by us") + return nil + } + if k8sutil.IsPersistentVolumeClaimMarkedForDeletion(pvc) && len(pvc.GetFinalizers()) > 0 { // Strange, pvc belongs to us, but we have no member for it. // Remove all finalizers, so it can be removed. @@ -79,9 +86,23 @@ func (r *Resources) InspectPVCs(ctx context.Context, cachedStatus inspectorInter return nil } + groupSpec := r.context.GetSpec().GetServerGroupSpec(group) + owner := r.context.GetAPIObject().AsOwner() - if k8sutil.UpdateOwnerRefToObjectIfNeeded(pvc.GetObjectMeta(), &owner) { - q := patch.NewPatch(patch.ItemReplace(patch.NewPath("metadata", "ownerReferences"), pvc.ObjectMeta.OwnerReferences)) + + ownerUpdate := k8sutil.UpdateOwnerRefToObjectIfNeeded + if groupSpec.IndexMethod.Get() == api.ServerGroupIndexMethodOrdered { + ownerUpdate = k8sutil.RemoveOwnerRefToObjectIfNeeded + } + + if ownerUpdate(pvc.GetObjectMeta(), &owner) { + q := patch.NewPatch() + if f := pvc.ObjectMeta.OwnerReferences; len(f) == 0 { + q.Add(patch.ItemRemove(patch.NewPath("metadata", "ownerReferences"))) + } else { + q.Add(patch.ItemReplace(patch.NewPath("metadata", "ownerReferences"), pvc.ObjectMeta.OwnerReferences)) + } + d, err := q.Marshal() if err != nil { log.Err(err).Debug("Failed to prepare PVC patch (ownerReferences)") @@ -107,6 +128,32 @@ func (r *Resources) InspectPVCs(ctx context.Context, cachedStatus inspectorInter } else { nextInterval = nextInterval.ReduceTo(x) } + } else { + // Ensure finalizers + if r.ensurePVCFinalizers(pvc) { + q := patch.NewPatch() + if f := pvc.Finalizers; len(f) == 0 { + q.Add(patch.ItemRemove(patch.NewPath("metadata", "finalizers"))) + } else { + q.Add(patch.ItemReplace(patch.NewPath("metadata", "finalizers"), f)) + } + + d, err := q.Marshal() + if err != nil { + log.Err(err).Debug("Failed to prepare PVC patch (finalizers)") + return errors.WithStack(err) + } + + err = globals.GetGlobalTimeouts().Kubernetes().RunWithTimeout(ctx, func(ctxChild context.Context) error { + _, err := cachedStatus.PersistentVolumeClaimsModInterface().V1().Patch(ctxChild, pvc.GetName(), types.JSONPatchType, d, meta.PatchOptions{}) + return err + }) + + if err != nil { + log.Err(err).Debug("Failed to update PVC (ownerReferences)") + return errors.WithStack(err) + } + } } return nil diff --git a/pkg/deployment/resources/pvcs.go b/pkg/deployment/resources/pvcs.go index 313b25fc9..413f6ee1e 100644 --- a/pkg/deployment/resources/pvcs.go +++ b/pkg/deployment/resources/pvcs.go @@ -23,6 +23,8 @@ package resources import ( "context" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/arangodb/kube-arangodb/pkg/util/constants" "github.com/arangodb/kube-arangodb/pkg/util/errors" @@ -32,10 +34,14 @@ import ( ) // createPVCFinalizers creates a list of finalizers for a PVC created for the given group. -func (r *Resources) createPVCFinalizers(group api.ServerGroup) []string { +func (r *Resources) createPVCFinalizers() []string { return []string{constants.FinalizerPVCMemberExists} } +func (r *Resources) ensurePVCFinalizers(in meta.Object) bool { + return k8sutil.EnsureFinalizers(in, r.createPVCFinalizers(), nil) +} + // EnsurePVCs creates all PVC's listed in member status func (r *Resources) EnsurePVCs(ctx context.Context, cachedStatus inspectorInterface.Inspector) error { apiObject := r.context.GetAPIObject() @@ -59,7 +65,7 @@ func (r *Resources) EnsurePVCs(ctx context.Context, cachedStatus inspectorInterf role := group.AsRole() resources := spec.Resources vct := spec.VolumeClaimTemplate - finalizers := r.createPVCFinalizers(group) + finalizers := r.createPVCFinalizers() err := globals.GetGlobalTimeouts().Kubernetes().RunWithTimeout(ctx, func(ctxChild context.Context) error { return k8sutil.CreatePersistentVolumeClaim(ctxChild, cachedStatus.PersistentVolumeClaimsModInterface().V1(), m.PersistentVolumeClaimName, deploymentName, storageClassName, role, enforceAntiAffinity, diff --git a/pkg/util/k8sutil/events.go b/pkg/util/k8sutil/events.go index 311697b54..21b7f63c6 100644 --- a/pkg/util/k8sutil/events.go +++ b/pkg/util/k8sutil/events.go @@ -46,6 +46,8 @@ type APIObject interface { meta.Object // AsOwner creates an OwnerReference for the given deployment AsOwner() meta.OwnerReference + + OwnerOf(in meta.Object) bool } // NewMemberAddEvent creates an event indicating that a member was added. diff --git a/pkg/util/k8sutil/finalizers.go b/pkg/util/k8sutil/finalizers.go index 282b316d6..4a0a20d18 100644 --- a/pkg/util/k8sutil/finalizers.go +++ b/pkg/util/k8sutil/finalizers.go @@ -22,6 +22,7 @@ package k8sutil import ( "context" + "sort" core "k8s.io/api/core/v1" meta "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -158,3 +159,42 @@ func RemoveFinalizers(finalizers []string, getFunc func() (meta.Object, error), } } } + +func EnsureFinalizers(in meta.Object, exists []string, missing []string) bool { + present := make(map[string]bool, len(in.GetFinalizers())) + + for _, k := range in.GetFinalizers() { + present[k] = true + } + + changed := false + + for _, k := range exists { + if _, ok := present[k]; !ok { + present[k] = true + changed = true + } + } + + for _, k := range missing { + if _, ok := present[k]; ok { + delete(present, k) + changed = true + } + } + + if !changed { + return false + } + + q := make([]string, 0, len(present)) + + for k := range present { + q = append(q, k) + } + + sort.Strings(q) + + in.SetFinalizers(q) + return true +} diff --git a/pkg/util/k8sutil/annotations.go b/pkg/util/k8sutil/tools/owner.go similarity index 54% rename from pkg/util/k8sutil/annotations.go rename to pkg/util/k8sutil/tools/owner.go index 0b742d962..ec8539985 100644 --- a/pkg/util/k8sutil/annotations.go +++ b/pkg/util/k8sutil/tools/owner.go @@ -1,24 +1,4 @@ -// -// DISCLAIMER -// -// Copyright 2016-2022 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 k8sutil +package tools import ( meta "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/util/k8sutil/util.go b/pkg/util/k8sutil/util.go index 965056481..2bce2134b 100644 --- a/pkg/util/k8sutil/util.go +++ b/pkg/util/k8sutil/util.go @@ -57,6 +57,40 @@ func AddOwnerRefToObject(obj meta.Object, ownerRef *meta.OwnerReference) { } } +// RemoveOwnerRefToObjectIfNeeded removes given owner reference to given object if it exists +func RemoveOwnerRefToObjectIfNeeded(obj meta.Object, ownerRef *meta.OwnerReference) bool { + exists := -1 + if ownerRef != nil { + own := obj.GetOwnerReferences() + + for id, existingOwnerRef := range own { + if existingOwnerRef.UID == ownerRef.UID { + exists = id + break + } + } + + if exists == -1 { + return false + } + + no := make([]meta.OwnerReference, 0, len(own)) + + for id := range own { + if id == exists { + continue + } + + no = append(no, own[id]) + } + + obj.SetOwnerReferences(no) + return true + } + + return false +} + // UpdateOwnerRefToObjectIfNeeded add given owner reference to given object if it does not exist yet func UpdateOwnerRefToObjectIfNeeded(obj meta.Object, ownerRef *meta.OwnerReference) bool { if ownerRef != nil {