From 5267ee8f2fd3980a584f29bbd125ed221c132456 Mon Sep 17 00:00:00 2001 From: mlavacca Date: Wed, 16 Feb 2022 17:44:02 +0100 Subject: [PATCH 1/9] Waiting for volumeAttachments deletion Signed-off-by: Mattia Lavacca --- pkg/controller/machine/machine_controller.go | 37 +++++++++++++++----- pkg/controller/machine/machine_test.go | 2 +- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/pkg/controller/machine/machine_controller.go b/pkg/controller/machine/machine_controller.go index 7431d2634..1bc3e738f 100644 --- a/pkg/controller/machine/machine_controller.go +++ b/pkg/controller/machine/machine_controller.go @@ -47,6 +47,7 @@ import ( "github.com/kubermatic/machine-controller/pkg/userdata/rhel" corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/api/equality" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -550,7 +551,7 @@ func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes. return nil, nil } - return nil, r.deleteNodeForMachine(ctx, machine) + return r.deleteNodeForMachine(ctx, machine) } func (r *Reconciler) deleteCloudProviderInstance(prov cloudprovidertypes.Provider, machine *clusterv1alpha1.Machine) (*reconcile.Result, error) { @@ -623,26 +624,40 @@ func (r *Reconciler) deleteCloudProviderInstance(prov cloudprovidertypes.Provide }) } -func (r *Reconciler) deleteNodeForMachine(ctx context.Context, machine *clusterv1alpha1.Machine) error { +func (r *Reconciler) deleteNodeForMachine(ctx context.Context, machine *clusterv1alpha1.Machine) (*reconcile.Result, error) { + // List all the volumeAttachments in the cluster; we must be sure that all + // of them will be deleted before deleting the node + volumeAttachments := &storagev1.VolumeAttachmentList{} + if err := r.client.List(ctx, volumeAttachments); err != nil { + return nil, fmt.Errorf("failed to list volumeAttachments: %v", err) + } + // If there's NodeRef on the Machine object, remove the Node by using the // value of the NodeRef. If there's no NodeRef, try to find the Node by // listing nodes using the NodeOwner label selector. + if machine.Status.NodeRef != nil { objKey := ctrlruntimeclient.ObjectKey{Name: machine.Status.NodeRef.Name} node := &corev1.Node{} nodeFound := true if err := r.client.Get(ctx, objKey, node); err != nil { if !kerrors.IsNotFound(err) { - return fmt.Errorf("failed to get node %s: %v", machine.Status.NodeRef.Name, err) + return nil, fmt.Errorf("failed to get node %s: %v", machine.Status.NodeRef.Name, err) } nodeFound = false klog.V(2).Infof("node %q does not longer exist for machine %q", machine.Status.NodeRef.Name, machine.Spec.Name) } if nodeFound { + for _, va := range volumeAttachments.Items { + if va.Spec.NodeName == node.Name { + klog.V(3).Infof("waiting for the volumeAttachment %s to be deleted before deleting node %s", va.Name, node.Name) + return &reconcile.Result{RequeueAfter: 10 * time.Second}, nil + } + } if err := r.client.Delete(ctx, node); err != nil { if !kerrors.IsNotFound(err) { - return err + return nil, err } klog.V(2).Infof("node %q does not longer exist for machine %q", machine.Status.NodeRef.Name, machine.Spec.Name) } @@ -650,12 +665,12 @@ func (r *Reconciler) deleteNodeForMachine(ctx context.Context, machine *clusterv } else { selector, err := labels.Parse(NodeOwnerLabelName + "=" + string(machine.UID)) if err != nil { - return fmt.Errorf("failed to parse label selector: %v", err) + return nil, fmt.Errorf("failed to parse label selector: %v", err) } listOpts := &ctrlruntimeclient.ListOptions{LabelSelector: selector} nodes := &corev1.NodeList{} if err := r.client.List(ctx, nodes, listOpts); err != nil { - return fmt.Errorf("failed to list nodes: %v", err) + return nil, fmt.Errorf("failed to list nodes: %v", err) } if len(nodes.Items) == 0 { // We just want log that we didn't found the node. We don't want to @@ -664,13 +679,19 @@ func (r *Reconciler) deleteNodeForMachine(ctx context.Context, machine *clusterv } for _, node := range nodes.Items { + for _, va := range volumeAttachments.Items { + if va.Spec.NodeName == node.Name { + klog.V(3).Infof("waiting for the volumeAttachment %s to be deleted before deleting node %s", va.Name, node.Name) + return &reconcile.Result{RequeueAfter: 10 * time.Second}, nil + } + } if err := r.client.Delete(ctx, &node); err != nil { - return err + return nil, err } } } - return r.updateMachine(machine, func(m *clusterv1alpha1.Machine) { + return nil, r.updateMachine(machine, func(m *clusterv1alpha1.Machine) { finalizers := sets.NewString(m.Finalizers...) if finalizers.Has(FinalizerDeleteNode) { finalizers := sets.NewString(m.Finalizers...) diff --git a/pkg/controller/machine/machine_test.go b/pkg/controller/machine/machine_test.go index 7f4a65d91..a3913daf8 100644 --- a/pkg/controller/machine/machine_test.go +++ b/pkg/controller/machine/machine_test.go @@ -595,7 +595,7 @@ func TestControllerDeleteNodeForMachine(t *testing.T) { providerData: providerData, } - err := reconciler.deleteNodeForMachine(ctx, test.machine) + _, err := reconciler.deleteNodeForMachine(ctx, test.machine) if diff := deep.Equal(err, test.err); diff != nil { t.Errorf("expected to get %v instead got: %v", test.err, err) } From 26bc6c1a24ef8a6c17e2a75924803dd5686e06be Mon Sep 17 00:00:00 2001 From: mlavacca Date: Tue, 22 Feb 2022 17:42:55 +0100 Subject: [PATCH 2/9] volumeAttachments check only for vSphere Signed-off-by: Mattia Lavacca --- pkg/controller/machine/machine_controller.go | 131 ++++++++++--------- pkg/controller/machine/machine_test.go | 53 +++++--- 2 files changed, 103 insertions(+), 81 deletions(-) diff --git a/pkg/controller/machine/machine_controller.go b/pkg/controller/machine/machine_controller.go index 1bc3e738f..862baf9a7 100644 --- a/pkg/controller/machine/machine_controller.go +++ b/pkg/controller/machine/machine_controller.go @@ -409,7 +409,7 @@ func (r *Reconciler) reconcile(ctx context.Context, machine *clusterv1alpha1.Mac // step 2: check if a user requested to delete the machine if machine.DeletionTimestamp != nil { - return r.deleteMachine(ctx, prov, machine) + return r.deleteMachine(ctx, prov, providerConfig.CloudProvider, machine) } // Step 3: Essentially creates an instance for the given machine. @@ -522,7 +522,7 @@ func (r *Reconciler) shouldEvict(ctx context.Context, machine *clusterv1alpha1.M } // deleteMachine makes sure that an instance has gone in a series of steps. -func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes.Provider, machine *clusterv1alpha1.Machine) (*reconcile.Result, error) { +func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes.Provider, providerName providerconfigtypes.CloudProvider, machine *clusterv1alpha1.Machine) (*reconcile.Result, error) { shouldEvict, err := r.shouldEvict(ctx, machine) if err != nil { return nil, err @@ -551,7 +551,69 @@ func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes. return nil, nil } - return r.deleteNodeForMachine(ctx, machine) + nodes, err := r.retrieveNodesRelatedToMachine(ctx, machine) + if err != nil { + return nil, err + } + + if providerName == providerconfigtypes.CloudProviderVsphere { + // List all the volumeAttachments in the cluster; we must be sure that all + // of them will be deleted before deleting the node + volumeAttachments := &storagev1.VolumeAttachmentList{} + if err := r.client.List(ctx, volumeAttachments); err != nil { + return nil, fmt.Errorf("failed to list volumeAttachments: %v", err) + } + for _, va := range volumeAttachments.Items { + for _, node := range nodes { + if va.Spec.NodeName == machine.Status.NodeRef.Name { + klog.V(3).Infof("waiting for the volumeAttachment %s to be deleted before deleting node %s", va.Name, node.Name) + return &reconcile.Result{RequeueAfter: 10 * time.Second}, nil + } + } + } + } + + return nil, r.deleteNodeForMachine(ctx, nodes, machine) +} + +func (r *Reconciler) retrieveNodesRelatedToMachine(ctx context.Context, machine *clusterv1alpha1.Machine) ([]*corev1.Node, error) { + nodes := make([]*corev1.Node, 0) + + // If there's NodeRef on the Machine object, remove the Node by using the + // value of the NodeRef. If there's no NodeRef, try to find the Node by + // listing nodes using the NodeOwner label selector. + if machine.Status.NodeRef != nil { + objKey := ctrlruntimeclient.ObjectKey{Name: machine.Status.NodeRef.Name} + node := &corev1.Node{} + if err := r.client.Get(ctx, objKey, node); err != nil { + if !kerrors.IsNotFound(err) { + return nil, fmt.Errorf("failed to get node %s: %v", machine.Status.NodeRef.Name, err) + } + klog.V(2).Infof("node %q does not longer exist for machine %q", machine.Status.NodeRef.Name, machine.Spec.Name) + } else { + nodes = append(nodes, node) + } + } else { + selector, err := labels.Parse(NodeOwnerLabelName + "=" + string(machine.UID)) + if err != nil { + return nil, fmt.Errorf("failed to parse label selector: %v", err) + } + listOpts := &ctrlruntimeclient.ListOptions{LabelSelector: selector} + nodeList := &corev1.NodeList{} + if err := r.client.List(ctx, nodeList, listOpts); err != nil { + return nil, fmt.Errorf("failed to list nodes: %v", err) + } + if len(nodeList.Items) == 0 { + // We just want log that we didn't found the node. + klog.V(3).Infof("No node found for the machine %s", machine.Spec.Name) + } + + for _, node := range nodeList.Items { + nodes = append(nodes, &node) + } + } + + return nodes, nil } func (r *Reconciler) deleteCloudProviderInstance(prov cloudprovidertypes.Provider, machine *clusterv1alpha1.Machine) (*reconcile.Result, error) { @@ -624,74 +686,21 @@ func (r *Reconciler) deleteCloudProviderInstance(prov cloudprovidertypes.Provide }) } -func (r *Reconciler) deleteNodeForMachine(ctx context.Context, machine *clusterv1alpha1.Machine) (*reconcile.Result, error) { - // List all the volumeAttachments in the cluster; we must be sure that all - // of them will be deleted before deleting the node - volumeAttachments := &storagev1.VolumeAttachmentList{} - if err := r.client.List(ctx, volumeAttachments); err != nil { - return nil, fmt.Errorf("failed to list volumeAttachments: %v", err) - } - +func (r *Reconciler) deleteNodeForMachine(ctx context.Context, nodes []*corev1.Node, machine *clusterv1alpha1.Machine) error { // If there's NodeRef on the Machine object, remove the Node by using the // value of the NodeRef. If there's no NodeRef, try to find the Node by // listing nodes using the NodeOwner label selector. - if machine.Status.NodeRef != nil { - objKey := ctrlruntimeclient.ObjectKey{Name: machine.Status.NodeRef.Name} - node := &corev1.Node{} - nodeFound := true - if err := r.client.Get(ctx, objKey, node); err != nil { + for _, node := range nodes { + if err := r.client.Delete(ctx, node); err != nil { if !kerrors.IsNotFound(err) { - return nil, fmt.Errorf("failed to get node %s: %v", machine.Status.NodeRef.Name, err) + return err } - nodeFound = false klog.V(2).Infof("node %q does not longer exist for machine %q", machine.Status.NodeRef.Name, machine.Spec.Name) } - - if nodeFound { - for _, va := range volumeAttachments.Items { - if va.Spec.NodeName == node.Name { - klog.V(3).Infof("waiting for the volumeAttachment %s to be deleted before deleting node %s", va.Name, node.Name) - return &reconcile.Result{RequeueAfter: 10 * time.Second}, nil - } - } - if err := r.client.Delete(ctx, node); err != nil { - if !kerrors.IsNotFound(err) { - return nil, err - } - klog.V(2).Infof("node %q does not longer exist for machine %q", machine.Status.NodeRef.Name, machine.Spec.Name) - } - } - } else { - selector, err := labels.Parse(NodeOwnerLabelName + "=" + string(machine.UID)) - if err != nil { - return nil, fmt.Errorf("failed to parse label selector: %v", err) - } - listOpts := &ctrlruntimeclient.ListOptions{LabelSelector: selector} - nodes := &corev1.NodeList{} - if err := r.client.List(ctx, nodes, listOpts); err != nil { - return nil, fmt.Errorf("failed to list nodes: %v", err) - } - if len(nodes.Items) == 0 { - // We just want log that we didn't found the node. We don't want to - // return here, as we want to remove finalizers at the end. - klog.V(3).Infof("No node found for the machine %s", machine.Spec.Name) - } - - for _, node := range nodes.Items { - for _, va := range volumeAttachments.Items { - if va.Spec.NodeName == node.Name { - klog.V(3).Infof("waiting for the volumeAttachment %s to be deleted before deleting node %s", va.Name, node.Name) - return &reconcile.Result{RequeueAfter: 10 * time.Second}, nil - } - } - if err := r.client.Delete(ctx, &node); err != nil { - return nil, err - } - } } - return nil, r.updateMachine(machine, func(m *clusterv1alpha1.Machine) { + return r.updateMachine(machine, func(m *clusterv1alpha1.Machine) { finalizers := sets.NewString(m.Finalizers...) if finalizers.Has(FinalizerDeleteNode) { finalizers := sets.NewString(m.Finalizers...) diff --git a/pkg/controller/machine/machine_test.go b/pkg/controller/machine/machine_test.go index a3913daf8..a9322ca32 100644 --- a/pkg/controller/machine/machine_test.go +++ b/pkg/controller/machine/machine_test.go @@ -474,7 +474,7 @@ func TestControllerDeleteNodeForMachine(t *testing.T) { tests := []struct { name string machine *clusterv1alpha1.Machine - nodes []runtime.Object + nodes []*corev1.Node err error shouldDeleteNode string }{ @@ -489,13 +489,17 @@ func TestControllerDeleteNodeForMachine(t *testing.T) { NodeRef: &corev1.ObjectReference{Name: "node-1"}, }, }, - nodes: []runtime.Object{&corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node-0", - }}, &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }}, + nodes: []*corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node-0", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node-1", + }, + }, }, err: nil, shouldDeleteNode: "node-1", @@ -510,8 +514,8 @@ func TestControllerDeleteNodeForMachine(t *testing.T) { }, Status: clusterv1alpha1.MachineStatus{}, }, - nodes: []runtime.Object{ - &corev1.Node{ + nodes: []*corev1.Node{ + { ObjectMeta: metav1.ObjectMeta{ Name: "node-0", Labels: map[string]string{ @@ -519,7 +523,7 @@ func TestControllerDeleteNodeForMachine(t *testing.T) { }, }, }, - &corev1.Node{ + { ObjectMeta: metav1.ObjectMeta{ Name: "node-1", }, @@ -538,13 +542,13 @@ func TestControllerDeleteNodeForMachine(t *testing.T) { }, Status: clusterv1alpha1.MachineStatus{}, }, - nodes: []runtime.Object{ - &corev1.Node{ + nodes: []*corev1.Node{ + { ObjectMeta: metav1.ObjectMeta{ Name: "node-0", }, }, - &corev1.Node{ + { ObjectMeta: metav1.ObjectMeta{ Name: "node-1", }, @@ -564,10 +568,12 @@ func TestControllerDeleteNodeForMachine(t *testing.T) { NodeRef: &corev1.ObjectReference{Name: "node-1"}, }, }, - nodes: []runtime.Object{&corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node-0", - }}, + nodes: []*corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node-0", + }, + }, }, err: nil, shouldDeleteNode: "", @@ -579,7 +585,9 @@ func TestControllerDeleteNodeForMachine(t *testing.T) { ctx := context.Background() objects := []runtime.Object{test.machine} - objects = append(objects, test.nodes...) + for _, n := range test.nodes { + objects = append(objects, n) + } client := ctrlruntimefake.NewFakeClient(objects...) @@ -595,7 +603,12 @@ func TestControllerDeleteNodeForMachine(t *testing.T) { providerData: providerData, } - _, err := reconciler.deleteNodeForMachine(ctx, test.machine) + nodes, err := reconciler.retrieveNodesRelatedToMachine(ctx, test.machine) + if err != nil { + return + } + + err = reconciler.deleteNodeForMachine(ctx, nodes, test.machine) if diff := deep.Equal(err, test.err); diff != nil { t.Errorf("expected to get %v instead got: %v", test.err, err) } From 167ce7d4e691b71e190aef9c355e318cd939c93a Mon Sep 17 00:00:00 2001 From: mlavacca Date: Wed, 23 Feb 2022 13:59:47 +0100 Subject: [PATCH 3/9] ClusterRole updated Signed-off-by: Mattia Lavacca --- examples/machine-controller.yaml | 9 +++++++++ pkg/controller/machine/machine_controller.go | 11 ++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/examples/machine-controller.yaml b/examples/machine-controller.yaml index 084d92d20..ae24f6845 100644 --- a/examples/machine-controller.yaml +++ b/examples/machine-controller.yaml @@ -536,6 +536,15 @@ rules: - "list" - "get" - "watch" +# storageAttachments permissions are needed in vSphere to solve an the following issue: https://github.com/kubermatic/machine-controller/issues/1189 +- apiGroups: + - "storage.k8s.io" + resources: + - "volumeattachments" + verbs: + - "list" + - "get" + - "watch" - apiGroups: - "" resources: diff --git a/pkg/controller/machine/machine_controller.go b/pkg/controller/machine/machine_controller.go index 862baf9a7..f9fcd92e4 100644 --- a/pkg/controller/machine/machine_controller.go +++ b/pkg/controller/machine/machine_controller.go @@ -557,8 +557,8 @@ func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes. } if providerName == providerconfigtypes.CloudProviderVsphere { - // List all the volumeAttachments in the cluster; we must be sure that all - // of them will be deleted before deleting the node + // Under vSphere, list all the volumeAttachments in the cluster; + // we must be sure that all of them will be deleted before deleting the node volumeAttachments := &storagev1.VolumeAttachmentList{} if err := r.client.List(ctx, volumeAttachments); err != nil { return nil, fmt.Errorf("failed to list volumeAttachments: %v", err) @@ -579,7 +579,7 @@ func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes. func (r *Reconciler) retrieveNodesRelatedToMachine(ctx context.Context, machine *clusterv1alpha1.Machine) ([]*corev1.Node, error) { nodes := make([]*corev1.Node, 0) - // If there's NodeRef on the Machine object, remove the Node by using the + // If there's NodeRef on the Machine object, retrieve the node by using the // value of the NodeRef. If there's no NodeRef, try to find the Node by // listing nodes using the NodeOwner label selector. if machine.Status.NodeRef != nil { @@ -687,10 +687,7 @@ func (r *Reconciler) deleteCloudProviderInstance(prov cloudprovidertypes.Provide } func (r *Reconciler) deleteNodeForMachine(ctx context.Context, nodes []*corev1.Node, machine *clusterv1alpha1.Machine) error { - // If there's NodeRef on the Machine object, remove the Node by using the - // value of the NodeRef. If there's no NodeRef, try to find the Node by - // listing nodes using the NodeOwner label selector. - + // iterates on all nodes and delete them. Finally, remove the finalizer on the machine for _, node := range nodes { if err := r.client.Delete(ctx, node); err != nil { if !kerrors.IsNotFound(err) { From 439716ec15298b51f5185cb9933aa6d50650c171 Mon Sep 17 00:00:00 2001 From: mlavacca Date: Wed, 23 Feb 2022 14:06:47 +0100 Subject: [PATCH 4/9] yaml linter fixed Signed-off-by: Mattia Lavacca --- examples/machine-controller.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/machine-controller.yaml b/examples/machine-controller.yaml index ae24f6845..b34ef8b0f 100644 --- a/examples/machine-controller.yaml +++ b/examples/machine-controller.yaml @@ -536,7 +536,7 @@ rules: - "list" - "get" - "watch" -# storageAttachments permissions are needed in vSphere to solve an the following issue: https://github.com/kubermatic/machine-controller/issues/1189 +# storageAttachments permissions are needed in vSphere to solve an the following issue: https://github.com/kubermatic/machine-controller/issues/1189 - apiGroups: - "storage.k8s.io" resources: From 5d5644f924816d6d28650f8f1c273211ede97cf6 Mon Sep 17 00:00:00 2001 From: mlavacca Date: Mon, 28 Feb 2022 21:45:24 +0100 Subject: [PATCH 5/9] VolumeAttachments correctly handled Signed-off-by: Mattia Lavacca --- cmd/machine-controller/main.go | 9 +- pkg/controller/machine/machine_controller.go | 59 ++-- pkg/node/poddeletion/pod_deletion.go | 267 +++++++++++++++++++ 3 files changed, 313 insertions(+), 22 deletions(-) create mode 100644 pkg/node/poddeletion/pod_deletion.go diff --git a/cmd/machine-controller/main.go b/cmd/machine-controller/main.go index ebb821e5b..3435fb22c 100644 --- a/cmd/machine-controller/main.go +++ b/cmd/machine-controller/main.go @@ -71,7 +71,8 @@ var ( skipEvictionAfter time.Duration caBundleFile string - useOSM bool + useOSM bool + externalCSI bool nodeCSRApprover bool nodeHTTPProxy string @@ -128,6 +129,8 @@ type controllerRunOptions struct { useOSM bool + externalCSI bool + // Assigns the POD networks that will be allocated. podCidr string @@ -170,6 +173,8 @@ func main() { flag.StringVar(&podCidr, "pod-cidr", "172.25.0.0/16", "The network ranges from which POD networks are allocated") flag.StringVar(&nodePortRange, "node-port-range", "30000-32767", "A port range to reserve for services with NodePort visibility") flag.StringVar(&nodeRegistryCredentialsSecret, "node-registry-credentials-secret", "", "A Secret object reference, that containt auth info for image registry in namespace/secret-name form, example: kube-system/registry-credentials. See doc at https://github.com/kubermaric/machine-controller/blob/master/docs/registry-authentication.md") + + flag.BoolVar(&externalCSI, "external-csi", false, "the Kubernetese cluster uses the external CSI driver") flag.BoolVar(&useOSM, "use-osm", false, "use osm controller for node bootstrap") flag.Parse() @@ -267,6 +272,7 @@ func main() { ContainerRuntime: containerRuntimeConfig, }, useOSM: useOSM, + externalCSI: externalCSI, podCidr: podCidr, nodePortRange: nodePortRange, } @@ -403,6 +409,7 @@ func (bs *controllerBootstrap) Start(ctx context.Context) error { bs.opt.skipEvictionAfter, bs.opt.node, bs.opt.useOSM, + bs.opt.externalCSI, bs.opt.podCidr, bs.opt.nodePortRange, ); err != nil { diff --git a/pkg/controller/machine/machine_controller.go b/pkg/controller/machine/machine_controller.go index f9fcd92e4..593a5475a 100644 --- a/pkg/controller/machine/machine_controller.go +++ b/pkg/controller/machine/machine_controller.go @@ -39,6 +39,7 @@ import ( "github.com/kubermatic/machine-controller/pkg/containerruntime" kuberneteshelper "github.com/kubermatic/machine-controller/pkg/kubernetes" "github.com/kubermatic/machine-controller/pkg/node/eviction" + "github.com/kubermatic/machine-controller/pkg/node/poddeletion" "github.com/kubermatic/machine-controller/pkg/providerconfig" providerconfigtypes "github.com/kubermatic/machine-controller/pkg/providerconfig/types" "github.com/kubermatic/machine-controller/pkg/rhsm" @@ -47,7 +48,6 @@ import ( "github.com/kubermatic/machine-controller/pkg/userdata/rhel" corev1 "k8s.io/api/core/v1" - storagev1 "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/api/equality" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -117,6 +117,7 @@ type Reconciler struct { satelliteSubscriptionManager rhsm.SatelliteSubscriptionManager useOSM bool + externalCSI bool podCIDR string nodePortRange string } @@ -175,6 +176,7 @@ func Add( skipEvictionAfter time.Duration, nodeSettings NodeSettings, useOSM bool, + externalCSI bool, podCIDR string, nodePortRange string, ) error { @@ -194,6 +196,7 @@ func Add( satelliteSubscriptionManager: rhsm.NewSatelliteSubscriptionManager(), useOSM: useOSM, + externalCSI: externalCSI, podCIDR: podCIDR, nodePortRange: nodePortRange, } @@ -463,6 +466,25 @@ func (r *Reconciler) ensureMachineHasNodeReadyCondition(machine *clusterv1alpha1 }) } +func (r *Reconciler) shouldCleanupVolumes(ctx context.Context, machine *clusterv1alpha1.Machine) (bool, error) { + // No node - No volumeAttachments to be collected + if machine.Status.NodeRef == nil { + klog.V(4).Infof("Skipping eviction for machine %q since it does not have a node", machine.Name) + return false, nil + } + + node := &corev1.Node{} + if err := r.client.Get(ctx, types.NamespacedName{Name: machine.Status.NodeRef.Name}, node); err != nil { + // Node does not exist - No volumeAttachments to be collected + if kerrors.IsNotFound(err) { + klog.V(4).Infof("Skipping eviction for machine %q since it does not have a node", machine.Name) + return false, nil + } + return false, fmt.Errorf("failed to get node %q", machine.Status.NodeRef.Name) + } + return true, nil +} + // evictIfNecessary checks if the machine has a node and evicts it if necessary func (r *Reconciler) shouldEvict(ctx context.Context, machine *clusterv1alpha1.Machine) (bool, error) { // If the deletion got triggered a few hours ago, skip eviction. @@ -527,17 +549,29 @@ func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes. if err != nil { return nil, err } + shouldCleanUpVolumes, err := r.shouldCleanupVolumes(ctx, machine) + if err != nil { + return nil, err + } + var evictedSomething, deletedSomething, volumesFree bool if shouldEvict { - evictedSomething, err := eviction.New(ctx, machine.Status.NodeRef.Name, r.client, r.kubeClient).Run() + evictedSomething, err = eviction.New(ctx, machine.Status.NodeRef.Name, r.client, r.kubeClient).Run() if err != nil { return nil, fmt.Errorf("failed to evict node %s: %v", machine.Status.NodeRef.Name, err) } - if evictedSomething { - return &reconcile.Result{RequeueAfter: 10 * time.Second}, nil + } + if shouldCleanUpVolumes && r.externalCSI { + deletedSomething, volumesFree, err = poddeletion.New(ctx, machine.Status.NodeRef.Name, r.client, r.kubeClient).Run() + if err != nil { + return nil, fmt.Errorf("failed to delete pods bound to volumes running on node %s: %v", machine.Status.NodeRef.Name, err) } } + if evictedSomething || deletedSomething || (r.externalCSI && !volumesFree) { + return &reconcile.Result{RequeueAfter: 10 * time.Second}, nil + } + if result, err := r.deleteCloudProviderInstance(prov, machine); result != nil || err != nil { return result, err } @@ -556,23 +590,6 @@ func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes. return nil, err } - if providerName == providerconfigtypes.CloudProviderVsphere { - // Under vSphere, list all the volumeAttachments in the cluster; - // we must be sure that all of them will be deleted before deleting the node - volumeAttachments := &storagev1.VolumeAttachmentList{} - if err := r.client.List(ctx, volumeAttachments); err != nil { - return nil, fmt.Errorf("failed to list volumeAttachments: %v", err) - } - for _, va := range volumeAttachments.Items { - for _, node := range nodes { - if va.Spec.NodeName == machine.Status.NodeRef.Name { - klog.V(3).Infof("waiting for the volumeAttachment %s to be deleted before deleting node %s", va.Name, node.Name) - return &reconcile.Result{RequeueAfter: 10 * time.Second}, nil - } - } - } - } - return nil, r.deleteNodeForMachine(ctx, nodes, machine) } diff --git a/pkg/node/poddeletion/pod_deletion.go b/pkg/node/poddeletion/pod_deletion.go new file mode 100644 index 000000000..0465b027d --- /dev/null +++ b/pkg/node/poddeletion/pod_deletion.go @@ -0,0 +1,267 @@ +/* +Copyright 2019 The Machine Controller Authors. + +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. +*/ + +package poddeletion + +import ( + "context" + "fmt" + "sync" + "time" + + corev1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "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/util/retry" + "k8s.io/klog" + ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + ErrorQueueLen = 1000 +) + +type NodeVolumeAttachmentsCleanup struct { + ctx context.Context + nodeName string + client ctrlruntimeclient.Client + kubeClient kubernetes.Interface +} + +// New returns a new NodeVolumeAttachmentsCleanup +func New(ctx context.Context, nodeName string, client ctrlruntimeclient.Client, kubeClient kubernetes.Interface) *NodeVolumeAttachmentsCleanup { + return &NodeVolumeAttachmentsCleanup{ + ctx: ctx, + nodeName: nodeName, + client: client, + kubeClient: kubeClient, + } +} + +// Run executes the eviction +func (vc *NodeVolumeAttachmentsCleanup) Run() (bool, bool, error) { + node, err := vc.getNode() + if err != nil { + return false, false, fmt.Errorf("failed to get node from lister: %v", err) + } + klog.V(3).Infof("Starting to cleanup node %s", vc.nodeName) + + volumeAttachmentsDeleted, err := vc.nodeCanBeDeleted() + if err != nil { + return false, false, fmt.Errorf("failed to check volumeAttachments deletion: %v", err) + } + if volumeAttachmentsDeleted { + return false, true, nil + } + + if err := vc.CordonNode(node); err != nil { + return false, false, fmt.Errorf("failed to cordon node %s: %v", vc.nodeName, err) + } + klog.V(6).Infof("Successfully cordoned node %s", vc.nodeName) + + podsToDelete, errors := vc.getFilteredPods() + if len(errors) > 0 { + return false, false, fmt.Errorf("failed to get Pods to delete for node %s, errors encountered: %v", vc.nodeName, err) + } + klog.V(6).Infof("Found %v pods to delete for node %s", len(podsToDelete), vc.nodeName) + + if len(podsToDelete) == 0 { + return false, false, nil + } + + // If we arrived here we have pods to evict, so tell the controller to retry later + if errs := vc.deletePods(podsToDelete); len(errs) > 0 { + return false, false, fmt.Errorf("failed to delete pods, errors encountered: %v", errs) + } + klog.V(6).Infof("Successfully deleted all pods mounting persistent volumes on node %s", vc.nodeName) + return true, false, err +} + +func (vc *NodeVolumeAttachmentsCleanup) getNode() (*corev1.Node, error) { + node := &corev1.Node{} + if err := vc.client.Get(vc.ctx, types.NamespacedName{Name: vc.nodeName}, node); err != nil { + return nil, fmt.Errorf("failed to get node from lister: %v", err) + } + return node, nil +} + +func (vc *NodeVolumeAttachmentsCleanup) CordonNode(node *corev1.Node) error { + if !node.Spec.Unschedulable { + _, err := vc.updateNode(func(n *corev1.Node) { + n.Spec.Unschedulable = true + }) + if err != nil { + return err + } + } + + // Be paranoid and wait until the change got propagated to the lister + // This assumes that the delay between our lister and the APIserver + // is smaller or equal to the delay the schedulers lister has - If + // that is not the case, there is a small chance the scheduler schedules + // pods in between, those will then get deleted upon node deletion and + // not evicted + return wait.Poll(1*time.Second, 10*time.Second, func() (bool, error) { + node := &corev1.Node{} + if err := vc.client.Get(vc.ctx, types.NamespacedName{Name: vc.nodeName}, node); err != nil { + return false, err + } + if node.Spec.Unschedulable { + return true, nil + } + return false, nil + }) +} + +func (vc *NodeVolumeAttachmentsCleanup) getFilteredPods() ([]corev1.Pod, []error) { + filteredPods := []corev1.Pod{} + lock := sync.Mutex{} + retErrs := []error{} + + volumeAttachments, err := vc.kubeClient.StorageV1().VolumeAttachments().List(vc.ctx, metav1.ListOptions{}) + if err != nil { + retErrs = append(retErrs, fmt.Errorf("failed to list pods: %v", err)) + return nil, retErrs + } + + persistentVolumeClaims, err := vc.kubeClient.CoreV1().PersistentVolumeClaims(metav1.NamespaceAll).List(vc.ctx, metav1.ListOptions{}) + if err != nil { + retErrs = append(retErrs, fmt.Errorf("failed to list persistent volumes: %v", err)) + return nil, retErrs + } + + errCh := make(chan error, ErrorQueueLen) + wg := sync.WaitGroup{} + for _, va := range volumeAttachments.Items { + if va.Spec.NodeName == vc.nodeName { + for _, pvc := range persistentVolumeClaims.Items { + if va.Spec.Source.PersistentVolumeName != nil && *va.Spec.Source.PersistentVolumeName == pvc.Spec.VolumeName { + wg.Add(1) + go func(pvc corev1.PersistentVolumeClaim) { + defer wg.Done() + pods, err := vc.kubeClient.CoreV1().Pods(pvc.Namespace).List(vc.ctx, metav1.ListOptions{}) + switch { + case kerrors.IsTooManyRequests(err): + return + case err != nil: + errCh <- fmt.Errorf("failed to list pod: %v", err) + default: + for _, pod := range pods.Items { + if doesPodClaimVolume(pod, pvc.Name) { + lock.Lock() + filteredPods = append(filteredPods, pod) + lock.Unlock() + } + } + } + }(pvc) + } + } + } + } + wg.Wait() + close(errCh) + + for err := range errCh { + retErrs = append(retErrs, err) + } + + return filteredPods, nil +} + +// doesPodClaimVolume checks if the volume is mounted by the pod +func doesPodClaimVolume(pod corev1.Pod, pvcName string) bool { + for _, volumeMount := range pod.Spec.Volumes { + if volumeMount.PersistentVolumeClaim != nil && volumeMount.PersistentVolumeClaim.ClaimName == pvcName { + return true + } + } + return false +} + +// nodeCanBeDeleted checks if all the volumeAttachments related to the node have already been collected by the external CSI driver +func (vc *NodeVolumeAttachmentsCleanup) nodeCanBeDeleted() (bool, error) { + volumeAttachments, err := vc.kubeClient.StorageV1().VolumeAttachments().List(vc.ctx, metav1.ListOptions{}) + if err != nil { + return false, fmt.Errorf("error while listing volumeAttachments: %v", err) + } + for _, va := range volumeAttachments.Items { + if va.Spec.NodeName == vc.nodeName { + klog.V(3).Infof("waiting for the volumeAttachment %s to be deleted before deleting node %s", va.Name, vc.nodeName) + return false, nil + } + } + return true, nil +} + +func (vc *NodeVolumeAttachmentsCleanup) deletePods(pods []corev1.Pod) []error { + + errCh := make(chan error, len(pods)) + retErrs := []error{} + + var wg sync.WaitGroup + var isDone bool + defer func() { isDone = true }() + + wg.Add(len(pods)) + for _, pod := range pods { + go func(p corev1.Pod) { + defer wg.Done() + for { + if isDone { + return + } + err := vc.kubeClient.CoreV1().Pods(p.Namespace).Delete(vc.ctx, p.Name, metav1.DeleteOptions{}) + if err == nil || kerrors.IsNotFound(err) { + klog.V(6).Infof("Successfully evicted pod %s/%s on node %s", p.Namespace, p.Name, vc.nodeName) + return + } else if kerrors.IsTooManyRequests(err) { + // PDB prevents eviction, return and make the controller retry later + return + } else { + errCh <- fmt.Errorf("error evicting pod %s/%s on node %s: %v", p.Namespace, p.Name, vc.nodeName, err) + return + } + } + }(pod) + } + wg.Wait() + close(errCh) + + for err := range errCh { + retErrs = append(retErrs, err) + } + + return retErrs +} + +func (vc *NodeVolumeAttachmentsCleanup) updateNode(modify func(*corev1.Node)) (*corev1.Node, error) { + node := &corev1.Node{} + err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { + if err := vc.client.Get(vc.ctx, types.NamespacedName{Name: vc.nodeName}, node); err != nil { + return err + } + // Apply modifications + modify(node) + // Update the node + return vc.client.Update(vc.ctx, node) + }) + + return node, err +} From 68d3cc069377eb86e346e405acf8277ff8be3811 Mon Sep 17 00:00:00 2001 From: mlavacca Date: Wed, 9 Mar 2022 09:58:47 +0100 Subject: [PATCH 6/9] Code factorized Signed-off-by: Mattia Lavacca --- cmd/machine-controller/main.go | 8 +- pkg/controller/machine/helpers.go | 33 +++++++ pkg/controller/machine/machine_controller.go | 16 ++-- pkg/node/eviction/eviction.go | 70 +++------------ pkg/node/manager/node_manager.go | 94 ++++++++++++++++++++ pkg/node/poddeletion/pod_deletion.go | 94 +++++--------------- 6 files changed, 169 insertions(+), 146 deletions(-) create mode 100644 pkg/controller/machine/helpers.go create mode 100644 pkg/node/manager/node_manager.go diff --git a/cmd/machine-controller/main.go b/cmd/machine-controller/main.go index 3435fb22c..1689bd075 100644 --- a/cmd/machine-controller/main.go +++ b/cmd/machine-controller/main.go @@ -71,8 +71,7 @@ var ( skipEvictionAfter time.Duration caBundleFile string - useOSM bool - externalCSI bool + useOSM bool nodeCSRApprover bool nodeHTTPProxy string @@ -129,8 +128,6 @@ type controllerRunOptions struct { useOSM bool - externalCSI bool - // Assigns the POD networks that will be allocated. podCidr string @@ -174,7 +171,6 @@ func main() { flag.StringVar(&nodePortRange, "node-port-range", "30000-32767", "A port range to reserve for services with NodePort visibility") flag.StringVar(&nodeRegistryCredentialsSecret, "node-registry-credentials-secret", "", "A Secret object reference, that containt auth info for image registry in namespace/secret-name form, example: kube-system/registry-credentials. See doc at https://github.com/kubermaric/machine-controller/blob/master/docs/registry-authentication.md") - flag.BoolVar(&externalCSI, "external-csi", false, "the Kubernetese cluster uses the external CSI driver") flag.BoolVar(&useOSM, "use-osm", false, "use osm controller for node bootstrap") flag.Parse() @@ -272,7 +268,6 @@ func main() { ContainerRuntime: containerRuntimeConfig, }, useOSM: useOSM, - externalCSI: externalCSI, podCidr: podCidr, nodePortRange: nodePortRange, } @@ -409,7 +404,6 @@ func (bs *controllerBootstrap) Start(ctx context.Context) error { bs.opt.skipEvictionAfter, bs.opt.node, bs.opt.useOSM, - bs.opt.externalCSI, bs.opt.podCidr, bs.opt.nodePortRange, ); err != nil { diff --git a/pkg/controller/machine/helpers.go b/pkg/controller/machine/helpers.go new file mode 100644 index 000000000..8d3b63acf --- /dev/null +++ b/pkg/controller/machine/helpers.go @@ -0,0 +1,33 @@ +/* +Copyright 2019 The Machine Controller Authors. + +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. +*/ + +package controller + +import ( + "strconv" + + "github.com/kubermatic/machine-controller/pkg/apis/cluster/common" + clusterv1alpha1 "github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1" +) + +func useExternalCSI(machine *clusterv1alpha1.Machine) (bool, error) { + kubeletFeatureGates := common.GetKubeletFlags(machine.Annotations) + val, ok := kubeletFeatureGates[common.ExternalCloudProviderKubeletFlag] + if !ok { + return false, nil + } + return strconv.ParseBool(val) +} diff --git a/pkg/controller/machine/machine_controller.go b/pkg/controller/machine/machine_controller.go index 593a5475a..0cc396cb2 100644 --- a/pkg/controller/machine/machine_controller.go +++ b/pkg/controller/machine/machine_controller.go @@ -117,7 +117,6 @@ type Reconciler struct { satelliteSubscriptionManager rhsm.SatelliteSubscriptionManager useOSM bool - externalCSI bool podCIDR string nodePortRange string } @@ -176,7 +175,6 @@ func Add( skipEvictionAfter time.Duration, nodeSettings NodeSettings, useOSM bool, - externalCSI bool, podCIDR string, nodePortRange string, ) error { @@ -196,7 +194,6 @@ func Add( satelliteSubscriptionManager: rhsm.NewSatelliteSubscriptionManager(), useOSM: useOSM, - externalCSI: externalCSI, podCIDR: podCIDR, nodePortRange: nodePortRange, } @@ -412,7 +409,7 @@ func (r *Reconciler) reconcile(ctx context.Context, machine *clusterv1alpha1.Mac // step 2: check if a user requested to delete the machine if machine.DeletionTimestamp != nil { - return r.deleteMachine(ctx, prov, providerConfig.CloudProvider, machine) + return r.deleteMachine(ctx, prov, machine) } // Step 3: Essentially creates an instance for the given machine. @@ -544,7 +541,7 @@ func (r *Reconciler) shouldEvict(ctx context.Context, machine *clusterv1alpha1.M } // deleteMachine makes sure that an instance has gone in a series of steps. -func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes.Provider, providerName providerconfigtypes.CloudProvider, machine *clusterv1alpha1.Machine) (*reconcile.Result, error) { +func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes.Provider, machine *clusterv1alpha1.Machine) (*reconcile.Result, error) { shouldEvict, err := r.shouldEvict(ctx, machine) if err != nil { return nil, err @@ -554,6 +551,11 @@ func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes. return nil, err } + externalCSI, err := useExternalCSI(machine) + if err != nil { + return nil, err + } + var evictedSomething, deletedSomething, volumesFree bool if shouldEvict { evictedSomething, err = eviction.New(ctx, machine.Status.NodeRef.Name, r.client, r.kubeClient).Run() @@ -561,14 +563,14 @@ func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes. return nil, fmt.Errorf("failed to evict node %s: %v", machine.Status.NodeRef.Name, err) } } - if shouldCleanUpVolumes && r.externalCSI { + if shouldCleanUpVolumes && externalCSI { deletedSomething, volumesFree, err = poddeletion.New(ctx, machine.Status.NodeRef.Name, r.client, r.kubeClient).Run() if err != nil { return nil, fmt.Errorf("failed to delete pods bound to volumes running on node %s: %v", machine.Status.NodeRef.Name, err) } } - if evictedSomething || deletedSomething || (r.externalCSI && !volumesFree) { + if evictedSomething || deletedSomething || (externalCSI && !volumesFree) { return &reconcile.Result{RequeueAfter: 10 * time.Second}, nil } diff --git a/pkg/node/eviction/eviction.go b/pkg/node/eviction/eviction.go index 4962770ec..41507fb32 100644 --- a/pkg/node/eviction/eviction.go +++ b/pkg/node/eviction/eviction.go @@ -20,44 +20,41 @@ import ( "context" "fmt" "sync" - "time" evictiontypes "github.com/kubermatic/machine-controller/pkg/node/eviction/types" + "github.com/kubermatic/machine-controller/pkg/node/manager" corev1 "k8s.io/api/core/v1" policy "k8s.io/api/policy/v1beta1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" - "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" - "k8s.io/client-go/util/retry" "k8s.io/klog" ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" ) type NodeEviction struct { - ctx context.Context - nodeName string - client ctrlruntimeclient.Client - kubeClient kubernetes.Interface + nodeManager *manager.NodeManager + ctx context.Context + nodeName string + kubeClient kubernetes.Interface } // New returns a new NodeEviction func New(ctx context.Context, nodeName string, client ctrlruntimeclient.Client, kubeClient kubernetes.Interface) *NodeEviction { return &NodeEviction{ - ctx: ctx, - nodeName: nodeName, - client: client, - kubeClient: kubeClient, + nodeManager: manager.NewNodeManager(ctx, client, nodeName), + ctx: ctx, + nodeName: nodeName, + kubeClient: kubeClient, } } // Run executes the eviction func (ne *NodeEviction) Run() (bool, error) { - node := &corev1.Node{} - if err := ne.client.Get(ne.ctx, types.NamespacedName{Name: ne.nodeName}, node); err != nil { + node, err := ne.nodeManager.GetNode() + if err != nil { return false, fmt.Errorf("failed to get node from lister: %v", err) } if _, exists := node.Annotations[evictiontypes.SkipEvictionAnnotationKey]; exists { @@ -66,7 +63,7 @@ func (ne *NodeEviction) Run() (bool, error) { } klog.V(3).Infof("Starting to evict node %s", ne.nodeName) - if err := ne.cordonNode(node); err != nil { + if err := ne.nodeManager.CordonNode(node); err != nil { return false, fmt.Errorf("failed to cordon node %s: %v", ne.nodeName, err) } klog.V(6).Infof("Successfully cordoned node %s", ne.nodeName) @@ -90,34 +87,6 @@ func (ne *NodeEviction) Run() (bool, error) { return true, nil } -func (ne *NodeEviction) cordonNode(node *corev1.Node) error { - if !node.Spec.Unschedulable { - _, err := ne.updateNode(func(n *corev1.Node) { - n.Spec.Unschedulable = true - }) - if err != nil { - return err - } - } - - // Be paranoid and wait until the change got propagated to the lister - // This assumes that the delay between our lister and the APIserver - // is smaller or equal to the delay the schedulers lister has - If - // that is not the case, there is a small chance the scheduler schedules - // pods in between, those will then get deleted upon node deletion and - // not evicted - return wait.Poll(1*time.Second, 10*time.Second, func() (bool, error) { - node := &corev1.Node{} - if err := ne.client.Get(ne.ctx, types.NamespacedName{Name: ne.nodeName}, node); err != nil { - return false, err - } - if node.Spec.Unschedulable { - return true, nil - } - return false, nil - }) -} - func (ne *NodeEviction) getFilteredPods() ([]corev1.Pod, error) { // The lister-backed client from the mgr automatically creates a lister for all objects requested through it. // We explicitly do not want that for pods, hence we have to use the kubernetes core client @@ -202,18 +171,3 @@ func (ne *NodeEviction) evictPod(pod *corev1.Pod) error { } return ne.kubeClient.PolicyV1beta1().Evictions(eviction.Namespace).Evict(ne.ctx, eviction) } - -func (ne *NodeEviction) updateNode(modify func(*corev1.Node)) (*corev1.Node, error) { - node := &corev1.Node{} - err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { - if err := ne.client.Get(ne.ctx, types.NamespacedName{Name: ne.nodeName}, node); err != nil { - return err - } - // Apply modifications - modify(node) - // Update the node - return ne.client.Update(ne.ctx, node) - }) - - return node, err -} diff --git a/pkg/node/manager/node_manager.go b/pkg/node/manager/node_manager.go new file mode 100644 index 000000000..86d111d82 --- /dev/null +++ b/pkg/node/manager/node_manager.go @@ -0,0 +1,94 @@ +/* +Copyright 2019 The Machine Controller Authors. + +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. +*/ + +package manager + +import ( + "context" + "fmt" + "time" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/util/retry" + ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +type NodeManager struct { + ctx context.Context + client ctrlruntimeclient.Client + nodeName string +} + +func NewNodeManager(ctx context.Context, client ctrlruntimeclient.Client, nodeName string) *NodeManager { + return &NodeManager{ + ctx: ctx, + client: client, + nodeName: nodeName, + } +} + +func (nm *NodeManager) GetNode() (*corev1.Node, error) { + node := &corev1.Node{} + if err := nm.client.Get(nm.ctx, types.NamespacedName{Name: nm.nodeName}, node); err != nil { + return nil, fmt.Errorf("failed to get node from lister: %v", err) + } + return node, nil +} + +func (nm *NodeManager) CordonNode(node *corev1.Node) error { + if !node.Spec.Unschedulable { + _, err := nm.updateNode(func(n *corev1.Node) { + n.Spec.Unschedulable = true + }) + if err != nil { + return err + } + } + + // Be paranoid and wait until the change got propagated to the lister + // This assumes that the delay between our lister and the APIserver + // is smaller or equal to the delay the schedulers lister has - If + // that is not the case, there is a small chance the scheduler schedules + // pods in between, those will then get deleted upon node deletion and + // not evicted + return wait.Poll(1*time.Second, 10*time.Second, func() (bool, error) { + node := &corev1.Node{} + if err := nm.client.Get(nm.ctx, types.NamespacedName{Name: nm.nodeName}, node); err != nil { + return false, err + } + if node.Spec.Unschedulable { + return true, nil + } + return false, nil + }) +} + +func (nm *NodeManager) updateNode(modify func(*corev1.Node)) (*corev1.Node, error) { + node := &corev1.Node{} + err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { + if err := nm.client.Get(nm.ctx, types.NamespacedName{Name: nm.nodeName}, node); err != nil { + return err + } + // Apply modifications + modify(node) + // Update the node + return nm.client.Update(nm.ctx, node) + }) + + return node, err +} diff --git a/pkg/node/poddeletion/pod_deletion.go b/pkg/node/poddeletion/pod_deletion.go index 0465b027d..b64a3f038 100644 --- a/pkg/node/poddeletion/pod_deletion.go +++ b/pkg/node/poddeletion/pod_deletion.go @@ -20,43 +20,40 @@ import ( "context" "fmt" "sync" - "time" + "github.com/kubermatic/machine-controller/pkg/node/manager" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "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/util/retry" "k8s.io/klog" ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" ) const ( - ErrorQueueLen = 1000 + errorQueueLen = 100 ) type NodeVolumeAttachmentsCleanup struct { - ctx context.Context - nodeName string - client ctrlruntimeclient.Client - kubeClient kubernetes.Interface + nodeManager *manager.NodeManager + ctx context.Context + nodeName string + kubeClient kubernetes.Interface } // New returns a new NodeVolumeAttachmentsCleanup func New(ctx context.Context, nodeName string, client ctrlruntimeclient.Client, kubeClient kubernetes.Interface) *NodeVolumeAttachmentsCleanup { return &NodeVolumeAttachmentsCleanup{ - ctx: ctx, - nodeName: nodeName, - client: client, - kubeClient: kubeClient, + nodeManager: manager.NewNodeManager(ctx, client, nodeName), + ctx: ctx, + nodeName: nodeName, + kubeClient: kubeClient, } } // Run executes the eviction func (vc *NodeVolumeAttachmentsCleanup) Run() (bool, bool, error) { - node, err := vc.getNode() + node, err := vc.nodeManager.GetNode() if err != nil { return false, false, fmt.Errorf("failed to get node from lister: %v", err) } @@ -70,7 +67,7 @@ func (vc *NodeVolumeAttachmentsCleanup) Run() (bool, bool, error) { return false, true, nil } - if err := vc.CordonNode(node); err != nil { + if err := vc.nodeManager.CordonNode(node); err != nil { return false, false, fmt.Errorf("failed to cordon node %s: %v", vc.nodeName, err) } klog.V(6).Infof("Successfully cordoned node %s", vc.nodeName) @@ -93,42 +90,6 @@ func (vc *NodeVolumeAttachmentsCleanup) Run() (bool, bool, error) { return true, false, err } -func (vc *NodeVolumeAttachmentsCleanup) getNode() (*corev1.Node, error) { - node := &corev1.Node{} - if err := vc.client.Get(vc.ctx, types.NamespacedName{Name: vc.nodeName}, node); err != nil { - return nil, fmt.Errorf("failed to get node from lister: %v", err) - } - return node, nil -} - -func (vc *NodeVolumeAttachmentsCleanup) CordonNode(node *corev1.Node) error { - if !node.Spec.Unschedulable { - _, err := vc.updateNode(func(n *corev1.Node) { - n.Spec.Unschedulable = true - }) - if err != nil { - return err - } - } - - // Be paranoid and wait until the change got propagated to the lister - // This assumes that the delay between our lister and the APIserver - // is smaller or equal to the delay the schedulers lister has - If - // that is not the case, there is a small chance the scheduler schedules - // pods in between, those will then get deleted upon node deletion and - // not evicted - return wait.Poll(1*time.Second, 10*time.Second, func() (bool, error) { - node := &corev1.Node{} - if err := vc.client.Get(vc.ctx, types.NamespacedName{Name: vc.nodeName}, node); err != nil { - return false, err - } - if node.Spec.Unschedulable { - return true, nil - } - return false, nil - }) -} - func (vc *NodeVolumeAttachmentsCleanup) getFilteredPods() ([]corev1.Pod, []error) { filteredPods := []corev1.Pod{} lock := sync.Mutex{} @@ -146,7 +107,7 @@ func (vc *NodeVolumeAttachmentsCleanup) getFilteredPods() ([]corev1.Pod, []error return nil, retErrs } - errCh := make(chan error, ErrorQueueLen) + errCh := make(chan error, errorQueueLen) wg := sync.WaitGroup{} for _, va := range volumeAttachments.Items { if va.Spec.NodeName == vc.nodeName { @@ -185,16 +146,6 @@ func (vc *NodeVolumeAttachmentsCleanup) getFilteredPods() ([]corev1.Pod, []error return filteredPods, nil } -// doesPodClaimVolume checks if the volume is mounted by the pod -func doesPodClaimVolume(pod corev1.Pod, pvcName string) bool { - for _, volumeMount := range pod.Spec.Volumes { - if volumeMount.PersistentVolumeClaim != nil && volumeMount.PersistentVolumeClaim.ClaimName == pvcName { - return true - } - } - return false -} - // nodeCanBeDeleted checks if all the volumeAttachments related to the node have already been collected by the external CSI driver func (vc *NodeVolumeAttachmentsCleanup) nodeCanBeDeleted() (bool, error) { volumeAttachments, err := vc.kubeClient.StorageV1().VolumeAttachments().List(vc.ctx, metav1.ListOptions{}) @@ -251,17 +202,12 @@ func (vc *NodeVolumeAttachmentsCleanup) deletePods(pods []corev1.Pod) []error { return retErrs } -func (vc *NodeVolumeAttachmentsCleanup) updateNode(modify func(*corev1.Node)) (*corev1.Node, error) { - node := &corev1.Node{} - err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { - if err := vc.client.Get(vc.ctx, types.NamespacedName{Name: vc.nodeName}, node); err != nil { - return err +// doesPodClaimVolume checks if the volume is mounted by the pod +func doesPodClaimVolume(pod corev1.Pod, pvcName string) bool { + for _, volumeMount := range pod.Spec.Volumes { + if volumeMount.PersistentVolumeClaim != nil && volumeMount.PersistentVolumeClaim.ClaimName == pvcName { + return true } - // Apply modifications - modify(node) - // Update the node - return vc.client.Update(vc.ctx, node) - }) - - return node, err + } + return false } From fe399d15bbb70e6f3bb915169cfc7aa8928af762 Mon Sep 17 00:00:00 2001 From: mlavacca Date: Wed, 9 Mar 2022 11:17:24 +0100 Subject: [PATCH 7/9] renaming Signed-off-by: Mattia Lavacca --- cmd/machine-controller/main.go | 1 - examples/machine-controller.yaml | 2 +- pkg/node/eviction/eviction.go | 6 ++--- .../{manager => nodemanager}/node_manager.go | 4 ++-- pkg/node/poddeletion/pod_deletion.go | 23 +++++++++++-------- 5 files changed, 19 insertions(+), 17 deletions(-) rename pkg/node/{manager => nodemanager}/node_manager.go (95%) diff --git a/cmd/machine-controller/main.go b/cmd/machine-controller/main.go index 1689bd075..ebb821e5b 100644 --- a/cmd/machine-controller/main.go +++ b/cmd/machine-controller/main.go @@ -170,7 +170,6 @@ func main() { flag.StringVar(&podCidr, "pod-cidr", "172.25.0.0/16", "The network ranges from which POD networks are allocated") flag.StringVar(&nodePortRange, "node-port-range", "30000-32767", "A port range to reserve for services with NodePort visibility") flag.StringVar(&nodeRegistryCredentialsSecret, "node-registry-credentials-secret", "", "A Secret object reference, that containt auth info for image registry in namespace/secret-name form, example: kube-system/registry-credentials. See doc at https://github.com/kubermaric/machine-controller/blob/master/docs/registry-authentication.md") - flag.BoolVar(&useOSM, "use-osm", false, "use osm controller for node bootstrap") flag.Parse() diff --git a/examples/machine-controller.yaml b/examples/machine-controller.yaml index b34ef8b0f..a61fb745d 100644 --- a/examples/machine-controller.yaml +++ b/examples/machine-controller.yaml @@ -536,7 +536,7 @@ rules: - "list" - "get" - "watch" -# storageAttachments permissions are needed in vSphere to solve an the following issue: https://github.com/kubermatic/machine-controller/issues/1189 +# volumeAttachments permissions are needed by clusters using external CSI - apiGroups: - "storage.k8s.io" resources: diff --git a/pkg/node/eviction/eviction.go b/pkg/node/eviction/eviction.go index 41507fb32..e6d1a2024 100644 --- a/pkg/node/eviction/eviction.go +++ b/pkg/node/eviction/eviction.go @@ -22,7 +22,7 @@ import ( "sync" evictiontypes "github.com/kubermatic/machine-controller/pkg/node/eviction/types" - "github.com/kubermatic/machine-controller/pkg/node/manager" + "github.com/kubermatic/machine-controller/pkg/node/nodemanager" corev1 "k8s.io/api/core/v1" policy "k8s.io/api/policy/v1beta1" @@ -35,7 +35,7 @@ import ( ) type NodeEviction struct { - nodeManager *manager.NodeManager + nodeManager *nodemanager.NodeManager ctx context.Context nodeName string kubeClient kubernetes.Interface @@ -44,7 +44,7 @@ type NodeEviction struct { // New returns a new NodeEviction func New(ctx context.Context, nodeName string, client ctrlruntimeclient.Client, kubeClient kubernetes.Interface) *NodeEviction { return &NodeEviction{ - nodeManager: manager.NewNodeManager(ctx, client, nodeName), + nodeManager: nodemanager.New(ctx, client, nodeName), ctx: ctx, nodeName: nodeName, kubeClient: kubeClient, diff --git a/pkg/node/manager/node_manager.go b/pkg/node/nodemanager/node_manager.go similarity index 95% rename from pkg/node/manager/node_manager.go rename to pkg/node/nodemanager/node_manager.go index 86d111d82..342a2bf57 100644 --- a/pkg/node/manager/node_manager.go +++ b/pkg/node/nodemanager/node_manager.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package manager +package nodemanager import ( "context" @@ -34,7 +34,7 @@ type NodeManager struct { nodeName string } -func NewNodeManager(ctx context.Context, client ctrlruntimeclient.Client, nodeName string) *NodeManager { +func New(ctx context.Context, client ctrlruntimeclient.Client, nodeName string) *NodeManager { return &NodeManager{ ctx: ctx, client: client, diff --git a/pkg/node/poddeletion/pod_deletion.go b/pkg/node/poddeletion/pod_deletion.go index b64a3f038..1b9874aa8 100644 --- a/pkg/node/poddeletion/pod_deletion.go +++ b/pkg/node/poddeletion/pod_deletion.go @@ -21,7 +21,7 @@ import ( "fmt" "sync" - "github.com/kubermatic/machine-controller/pkg/node/manager" + "github.com/kubermatic/machine-controller/pkg/node/nodemanager" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -35,7 +35,7 @@ const ( ) type NodeVolumeAttachmentsCleanup struct { - nodeManager *manager.NodeManager + nodeManager *nodemanager.NodeManager ctx context.Context nodeName string kubeClient kubernetes.Interface @@ -44,14 +44,14 @@ type NodeVolumeAttachmentsCleanup struct { // New returns a new NodeVolumeAttachmentsCleanup func New(ctx context.Context, nodeName string, client ctrlruntimeclient.Client, kubeClient kubernetes.Interface) *NodeVolumeAttachmentsCleanup { return &NodeVolumeAttachmentsCleanup{ - nodeManager: manager.NewNodeManager(ctx, client, nodeName), + nodeManager: nodemanager.New(ctx, client, nodeName), ctx: ctx, nodeName: nodeName, kubeClient: kubeClient, } } -// Run executes the eviction +// Run executes the pod deletion func (vc *NodeVolumeAttachmentsCleanup) Run() (bool, bool, error) { node, err := vc.nodeManager.GetNode() if err != nil { @@ -59,6 +59,7 @@ func (vc *NodeVolumeAttachmentsCleanup) Run() (bool, bool, error) { } klog.V(3).Infof("Starting to cleanup node %s", vc.nodeName) + // if there are no more volumeAttachments related to the node, then it can be deleted volumeAttachmentsDeleted, err := vc.nodeCanBeDeleted() if err != nil { return false, false, fmt.Errorf("failed to check volumeAttachments deletion: %v", err) @@ -67,11 +68,13 @@ func (vc *NodeVolumeAttachmentsCleanup) Run() (bool, bool, error) { return false, true, nil } + // cordon the node to be sure that the deleted pods are re-scheduled in the same node if err := vc.nodeManager.CordonNode(node); err != nil { return false, false, fmt.Errorf("failed to cordon node %s: %v", vc.nodeName, err) } klog.V(6).Infof("Successfully cordoned node %s", vc.nodeName) + // get all the pods that needs to be deleted (i.e. those mounting volumes attached to the node that is going to be deleted) podsToDelete, errors := vc.getFilteredPods() if len(errors) > 0 { return false, false, fmt.Errorf("failed to get Pods to delete for node %s, errors encountered: %v", vc.nodeName, err) @@ -82,11 +85,11 @@ func (vc *NodeVolumeAttachmentsCleanup) Run() (bool, bool, error) { return false, false, nil } - // If we arrived here we have pods to evict, so tell the controller to retry later + // delete the previously filtered pods, then tells the controller to retry later if errs := vc.deletePods(podsToDelete); len(errs) > 0 { return false, false, fmt.Errorf("failed to delete pods, errors encountered: %v", errs) } - klog.V(6).Infof("Successfully deleted all pods mounting persistent volumes on node %s", vc.nodeName) + klog.V(6).Infof("Successfully deleted all pods mounting persistent volumes attached on node %s", vc.nodeName) return true, false, err } @@ -124,7 +127,7 @@ func (vc *NodeVolumeAttachmentsCleanup) getFilteredPods() ([]corev1.Pod, []error errCh <- fmt.Errorf("failed to list pod: %v", err) default: for _, pod := range pods.Items { - if doesPodClaimVolume(pod, pvc.Name) { + if doesPodClaimVolume(pod, pvc.Name) && pod.Spec.NodeName == vc.nodeName { lock.Lock() filteredPods = append(filteredPods, pod) lock.Unlock() @@ -180,13 +183,13 @@ func (vc *NodeVolumeAttachmentsCleanup) deletePods(pods []corev1.Pod) []error { } err := vc.kubeClient.CoreV1().Pods(p.Namespace).Delete(vc.ctx, p.Name, metav1.DeleteOptions{}) if err == nil || kerrors.IsNotFound(err) { - klog.V(6).Infof("Successfully evicted pod %s/%s on node %s", p.Namespace, p.Name, vc.nodeName) + klog.V(6).Infof("Successfully deleted pod %s/%s on node %s", p.Namespace, p.Name, vc.nodeName) return } else if kerrors.IsTooManyRequests(err) { - // PDB prevents eviction, return and make the controller retry later + // PDB prevents pod deletion, return and make the controller retry later return } else { - errCh <- fmt.Errorf("error evicting pod %s/%s on node %s: %v", p.Namespace, p.Name, vc.nodeName, err) + errCh <- fmt.Errorf("error deleting pod %s/%s on node %s: %v", p.Namespace, p.Name, vc.nodeName, err) return } } From bc139e474aa8d184f8d6eda1e88829a140453de3 Mon Sep 17 00:00:00 2001 From: mlavacca Date: Wed, 9 Mar 2022 11:52:55 +0100 Subject: [PATCH 8/9] fix yamllint Signed-off-by: Mattia Lavacca --- examples/machine-controller.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/machine-controller.yaml b/examples/machine-controller.yaml index a61fb745d..4812246c4 100644 --- a/examples/machine-controller.yaml +++ b/examples/machine-controller.yaml @@ -536,7 +536,7 @@ rules: - "list" - "get" - "watch" -# volumeAttachments permissions are needed by clusters using external CSI +# volumeAttachments permissions are needed by clusters using external CSI - apiGroups: - "storage.k8s.io" resources: From e635e1bcb2caab8aebd884e0594617e8a68f79e5 Mon Sep 17 00:00:00 2001 From: mlavacca Date: Wed, 9 Mar 2022 17:27:22 +0100 Subject: [PATCH 9/9] Logic applied only to vSphere Signed-off-by: Mattia Lavacca --- examples/machine-controller.yaml | 2 +- pkg/controller/machine/helpers.go | 33 -------------------- pkg/controller/machine/machine_controller.go | 25 ++++++++------- 3 files changed, 14 insertions(+), 46 deletions(-) delete mode 100644 pkg/controller/machine/helpers.go diff --git a/examples/machine-controller.yaml b/examples/machine-controller.yaml index 4812246c4..d3ee84c21 100644 --- a/examples/machine-controller.yaml +++ b/examples/machine-controller.yaml @@ -536,7 +536,7 @@ rules: - "list" - "get" - "watch" -# volumeAttachments permissions are needed by clusters using external CSI +# volumeAttachments permissions are needed by vsphere clusters - apiGroups: - "storage.k8s.io" resources: diff --git a/pkg/controller/machine/helpers.go b/pkg/controller/machine/helpers.go deleted file mode 100644 index 8d3b63acf..000000000 --- a/pkg/controller/machine/helpers.go +++ /dev/null @@ -1,33 +0,0 @@ -/* -Copyright 2019 The Machine Controller Authors. - -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. -*/ - -package controller - -import ( - "strconv" - - "github.com/kubermatic/machine-controller/pkg/apis/cluster/common" - clusterv1alpha1 "github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1" -) - -func useExternalCSI(machine *clusterv1alpha1.Machine) (bool, error) { - kubeletFeatureGates := common.GetKubeletFlags(machine.Annotations) - val, ok := kubeletFeatureGates[common.ExternalCloudProviderKubeletFlag] - if !ok { - return false, nil - } - return strconv.ParseBool(val) -} diff --git a/pkg/controller/machine/machine_controller.go b/pkg/controller/machine/machine_controller.go index 0cc396cb2..d7977bf4d 100644 --- a/pkg/controller/machine/machine_controller.go +++ b/pkg/controller/machine/machine_controller.go @@ -409,7 +409,7 @@ func (r *Reconciler) reconcile(ctx context.Context, machine *clusterv1alpha1.Mac // step 2: check if a user requested to delete the machine if machine.DeletionTimestamp != nil { - return r.deleteMachine(ctx, prov, machine) + return r.deleteMachine(ctx, prov, providerConfig.CloudProvider, machine) } // Step 3: Essentially creates an instance for the given machine. @@ -463,7 +463,12 @@ func (r *Reconciler) ensureMachineHasNodeReadyCondition(machine *clusterv1alpha1 }) } -func (r *Reconciler) shouldCleanupVolumes(ctx context.Context, machine *clusterv1alpha1.Machine) (bool, error) { +func (r *Reconciler) shouldCleanupVolumes(ctx context.Context, machine *clusterv1alpha1.Machine, providerName providerconfigtypes.CloudProvider) (bool, error) { + // we need to wait for volumeAttachments clean up only for vSphere + if providerName != providerconfigtypes.CloudProviderVsphere { + return false, nil + } + // No node - No volumeAttachments to be collected if machine.Status.NodeRef == nil { klog.V(4).Infof("Skipping eviction for machine %q since it does not have a node", machine.Name) @@ -541,36 +546,32 @@ func (r *Reconciler) shouldEvict(ctx context.Context, machine *clusterv1alpha1.M } // deleteMachine makes sure that an instance has gone in a series of steps. -func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes.Provider, machine *clusterv1alpha1.Machine) (*reconcile.Result, error) { +func (r *Reconciler) deleteMachine(ctx context.Context, prov cloudprovidertypes.Provider, providerName providerconfigtypes.CloudProvider, machine *clusterv1alpha1.Machine) (*reconcile.Result, error) { shouldEvict, err := r.shouldEvict(ctx, machine) if err != nil { return nil, err } - shouldCleanUpVolumes, err := r.shouldCleanupVolumes(ctx, machine) - if err != nil { - return nil, err - } - - externalCSI, err := useExternalCSI(machine) + shouldCleanUpVolumes, err := r.shouldCleanupVolumes(ctx, machine, providerName) if err != nil { return nil, err } - var evictedSomething, deletedSomething, volumesFree bool + var evictedSomething, deletedSomething bool + var volumesFree = true if shouldEvict { evictedSomething, err = eviction.New(ctx, machine.Status.NodeRef.Name, r.client, r.kubeClient).Run() if err != nil { return nil, fmt.Errorf("failed to evict node %s: %v", machine.Status.NodeRef.Name, err) } } - if shouldCleanUpVolumes && externalCSI { + if shouldCleanUpVolumes { deletedSomething, volumesFree, err = poddeletion.New(ctx, machine.Status.NodeRef.Name, r.client, r.kubeClient).Run() if err != nil { return nil, fmt.Errorf("failed to delete pods bound to volumes running on node %s: %v", machine.Status.NodeRef.Name, err) } } - if evictedSomething || deletedSomething || (externalCSI && !volumesFree) { + if evictedSomething || deletedSomething || !volumesFree { return &reconcile.Result{RequeueAfter: 10 * time.Second}, nil }