[Feature] Detach PVC in Ordered indexing (#1118)

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

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

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

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

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

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{

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
 	}

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)

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

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,

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.

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
+}

pkg/util/k8sutil/tools/owner.go

@@ -1,24 +1,4 @@
-//
+package tools
-// 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
 
 import (
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"

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 {