From 699428e83d6ece021de33ccd55282f23b2f5bfe5 Mon Sep 17 00:00:00 2001 From: Fabian Ruff Date: Tue, 22 Nov 2022 13:08:30 +0100 Subject: [PATCH 1/2] Keep nodes while deorbiter is running deleting volumes in the deorbiter is failing for multiple minutes because the nodes are gone and the csi daemonset is not unmounting the volumes. This changes lets the launch controller wait for the deorbiter to finish work before starting to terminate nodes. For the volumes to actually be deleted we need to remove pods that hold a reference to the pvc. --- pkg/apis/kubernikus/v1/kluster.go | 4 ---- pkg/controller/deorbit/controller.go | 3 +++ pkg/controller/deorbit/deorbiter.go | 16 +++++++++++++++- pkg/controller/launch/controller.go | 4 ++++ 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/pkg/apis/kubernikus/v1/kluster.go b/pkg/apis/kubernikus/v1/kluster.go index 384d46856b..7e0df2995f 100644 --- a/pkg/apis/kubernikus/v1/kluster.go +++ b/pkg/apis/kubernikus/v1/kluster.go @@ -64,10 +64,6 @@ func (k *Kluster) NeedsFinalizer(finalizer string) bool { } func (k *Kluster) HasFinalizer(finalizer string) bool { - if k.ObjectMeta.DeletionTimestamp == nil { - // not deleted. do not remove finalizers at this time - return false - } for _, f := range k.ObjectMeta.Finalizers { if f == finalizer { diff --git a/pkg/controller/deorbit/controller.go b/pkg/controller/deorbit/controller.go index 37106430bf..071e8e3e23 100644 --- a/pkg/controller/deorbit/controller.go +++ b/pkg/controller/deorbit/controller.go @@ -82,6 +82,9 @@ func (d *DeorbitReconciler) Reconcile(kluster *v1.Kluster) (bool, error) { if kluster.TerminationProtection() { return false, nil } + if kluster.ObjectMeta.DeletionTimestamp == nil { + return false, nil //wait until the kluster is marked for deletion + } if kluster.HasFinalizer(DeorbiterFinalizer) { if err := d.deorbit(kluster); err != nil { return false, err diff --git a/pkg/controller/deorbit/deorbiter.go b/pkg/controller/deorbit/deorbiter.go index 53dfc4e756..e3b04481ed 100644 --- a/pkg/controller/deorbit/deorbiter.go +++ b/pkg/controller/deorbit/deorbiter.go @@ -106,12 +106,26 @@ func (d *ConcreteDeorbiter) DeletePersistentVolumeClaims() (deleted []core_v1.Pe if pv.Spec.Cinder == nil && pv.Spec.CSI == nil { continue } - deleted = append(deleted, pvc) err = d.Client.CoreV1().PersistentVolumeClaims(pvc.Namespace).Delete(context.TODO(), pvc.Name, meta_v1.DeleteOptions{}) if err != nil { return deleted, err } + deleted = append(deleted, pvc) + //delete pods holding a reference to the pvc + pods, err := d.Client.CoreV1().Pods(pvc.Namespace).List(context.TODO(), meta_v1.ListOptions{}) + if err != nil { + return deleted, err + } + for _, pod := range pods.Items { + for _, volume := range pod.Spec.Volumes { + if volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.ClaimName == pvc.Name { + if err := d.Client.CoreV1().Pods(pvc.Namespace).Delete(context.TODO(), pod.Name, meta_v1.DeleteOptions{}); err != nil { + return deleted, fmt.Errorf("Failed to delete pod %s/%s: %w", pod.Namespace, pod.Name, err) + } + } + } + } } return deleted, err diff --git a/pkg/controller/launch/controller.go b/pkg/controller/launch/controller.go index f10362ab56..04d746e022 100644 --- a/pkg/controller/launch/controller.go +++ b/pkg/controller/launch/controller.go @@ -11,6 +11,7 @@ import ( v1 "github.com/sapcc/kubernikus/pkg/apis/kubernikus/v1" "github.com/sapcc/kubernikus/pkg/controller/base" "github.com/sapcc/kubernikus/pkg/controller/config" + "github.com/sapcc/kubernikus/pkg/controller/deorbit" "github.com/sapcc/kubernikus/pkg/controller/metrics" "github.com/sapcc/kubernikus/pkg/controller/nodeobservatory" informers_kubernikus "github.com/sapcc/kubernikus/pkg/generated/informers/externalversions/kubernikus/v1" @@ -91,6 +92,9 @@ func (lr *LaunchReconciler) Reconcile(kluster *v1.Kluster) (requeue bool, err er if kluster.TerminationProtection() { return false, nil } + if kluster.HasFinalizer(deorbit.DeorbiterFinalizer) { + return false, nil //Wait for dorbiter so that volumes are detached before deleting nodes + } return lr.terminatePools(kluster) } From 55a0416beb8944a71c6b8c68050320ab73d051d1 Mon Sep 17 00:00:00 2001 From: Fabian Ruff Date: Wed, 23 Nov 2022 17:05:37 +0100 Subject: [PATCH 2/2] cordon nodes to avoid volume re-attachment --- pkg/controller/deorbit/deorbiter.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/controller/deorbit/deorbiter.go b/pkg/controller/deorbit/deorbiter.go index e3b04481ed..a027422fd7 100644 --- a/pkg/controller/deorbit/deorbiter.go +++ b/pkg/controller/deorbit/deorbiter.go @@ -10,6 +10,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes" core_v1 "k8s.io/api/core/v1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/record" @@ -88,6 +89,18 @@ func NewDeorbiter(kluster *v1.Kluster, stopCh <-chan struct{}, clients config.Cl func (d *ConcreteDeorbiter) DeletePersistentVolumeClaims() (deleted []core_v1.PersistentVolumeClaim, err error) { deleted = []core_v1.PersistentVolumeClaim{} + // Cordon all nodes to avoid scheduling new pods that bind volumes + nodes, err := d.Client.CoreV1().Nodes().List(context.TODO(), meta_v1.ListOptions{}) + if err != nil { + return deleted, fmt.Errorf("Failed to list nodes: %w", err) + } + for _, node := range nodes.Items { + _, err := d.Client.CoreV1().Nodes().Patch(context.TODO(), node.Name, types.JSONPatchType, []byte(`[{"op": "replace", "path": "/spec/unschedulable", "value": true}]`), meta_v1.PatchOptions{}) + if err != nil { + return deleted, fmt.Errorf("Failed to cordon node %s: %w", node.Name, err) + } + } + pvcs, err := d.Client.CoreV1().PersistentVolumeClaims(meta_v1.NamespaceAll).List(context.TODO(), meta_v1.ListOptions{}) if err != nil { return deleted, err