From f216c6081e1b38f3316b553b1df1735f7cf5ec03 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Wed, 6 Nov 2024 13:59:00 +0100 Subject: [PATCH] Add feature gate for considering VolumeAttachments when waiting for volume detach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stefan Büringer buringerst@vmware.com --- config/manager/manager.yaml | 2 +- .../experimental-features.md | 12 +++++++ feature/feature.go | 13 ++++++-- .../controllers/machine/machine_controller.go | 21 +++++++------ .../machine/machine_controller_test.go | 31 +++++++++++++++++++ 5 files changed, 66 insertions(+), 13 deletions(-) diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 26ad15d063cb..50219acd802b 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -24,7 +24,7 @@ spec: - "--diagnostics-address=${CAPI_DIAGNOSTICS_ADDRESS:=:8443}" - "--insecure-diagnostics=${CAPI_INSECURE_DIAGNOSTICS:=false}" - "--use-deprecated-infra-machine-naming=${CAPI_USE_DEPRECATED_INFRA_MACHINE_NAMING:=false}" - - "--feature-gates=MachinePool=${EXP_MACHINE_POOL:=true},ClusterResourceSet=${EXP_CLUSTER_RESOURCE_SET:=true},ClusterTopology=${CLUSTER_TOPOLOGY:=false},RuntimeSDK=${EXP_RUNTIME_SDK:=false},MachineSetPreflightChecks=${EXP_MACHINE_SET_PREFLIGHT_CHECKS:=true}" + - "--feature-gates=MachinePool=${EXP_MACHINE_POOL:=true},ClusterResourceSet=${EXP_CLUSTER_RESOURCE_SET:=true},ClusterTopology=${CLUSTER_TOPOLOGY:=false},RuntimeSDK=${EXP_RUNTIME_SDK:=false},MachineSetPreflightChecks=${EXP_MACHINE_SET_PREFLIGHT_CHECKS:=true},MachineWaitForVolumeDetachConsiderVolumeAttachments=${EXP_MACHINE_WAITFORVOLUMEDETACH_CONSIDER_VOLUMEATTACHMENTS:=true}" image: controller:latest name: manager env: diff --git a/docs/book/src/tasks/experimental-features/experimental-features.md b/docs/book/src/tasks/experimental-features/experimental-features.md index 36ee84a14e00..ba7149888787 100644 --- a/docs/book/src/tasks/experimental-features/experimental-features.md +++ b/docs/book/src/tasks/experimental-features/experimental-features.md @@ -3,6 +3,18 @@ Cluster API now ships with a new experimental package that lives under the `exp/` directory. This is a temporary location for features which will be moved to their permanent locations after graduation. Users can experiment with these features by enabling them using feature gates. +Currently Cluster API has the following experimental features: +* `ClusterResourceSet` (`EXP_CLUSTER_RESOURCE_SET`): [ClusterResourceSet](./cluster-resource-set.md) +* `MachinePool` (`EXP_MACHINE_POOL`): [MachinePools](./machine-pools.md) +* `MachineSetPreflightChecks` (`EXP_MACHINE_SET_PREFLIGHT_CHECKS`): [MachineSetPreflightChecks](./machineset-preflight-checks.md) +* `MachineWaitForVolumeDetachConsiderVolumeAttachments` (`EXP_MACHINE_WAITFORVOLUMEDETACH_CONSIDER_VOLUMEATTACHMENTS`): + * During Machine drain the Machine controller waits for volumes to be detached. Per default, the controller considers + `Nodes.status.volumesAttached` and `VolumesAttachments`. This feature flag controls if the `VolumeAttachments` should + be considered. The feature gate was added to allow to opt-out if unexpected problems occur. +* `ClusterTopology` (`CLUSTER_TOPOLOGY`): [ClusterClass](./cluster-class/index.md) +* `RuntimeSDK` (`EXP_RUNTIME_SDK`): [RuntimeSDK](./runtime-sdk/index.md) +* `KubeadmBootstrapFormatIgnition` (`EXP_KUBEADM_BOOTSTRAP_FORMAT_IGNITION`): [Ignition](./ignition.md) + ## Enabling Experimental Features for Management Clusters Started with clusterctl Users can enable/disable features by setting OS environment variables before running `clusterctl init`, e.g.: diff --git a/feature/feature.go b/feature/feature.go index 4253ae98f509..87fbab48dc2d 100644 --- a/feature/feature.go +++ b/feature/feature.go @@ -62,6 +62,12 @@ const ( // alpha: v1.5 // beta: v1.9 MachineSetPreflightChecks featuregate.Feature = "MachineSetPreflightChecks" + + // MachineWaitForVolumeDetachConsiderVolumeAttachments is a feature gate that controls if the Machine controller + // considers VolumeAttachments when waiting for volumes to be detached. + // + // beta: v1.9 + MachineWaitForVolumeDetachConsiderVolumeAttachments featuregate.Feature = "MachineWaitForVolumeDetachConsiderVolumeAttachments" ) func init() { @@ -72,9 +78,10 @@ func init() { // To add a new feature, define a key for it above and add it here. var defaultClusterAPIFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ // Every feature should be initiated here: - ClusterResourceSet: {Default: true, PreRelease: featuregate.Beta}, - MachinePool: {Default: true, PreRelease: featuregate.Beta}, - MachineSetPreflightChecks: {Default: true, PreRelease: featuregate.Beta}, + ClusterResourceSet: {Default: true, PreRelease: featuregate.Beta}, + MachinePool: {Default: true, PreRelease: featuregate.Beta}, + MachineSetPreflightChecks: {Default: true, PreRelease: featuregate.Beta}, + MachineWaitForVolumeDetachConsiderVolumeAttachments: {Default: true, PreRelease: featuregate.Beta}, ClusterTopology: {Default: false, PreRelease: featuregate.Alpha}, KubeadmBootstrapFormatIgnition: {Default: false, PreRelease: featuregate.Alpha}, RuntimeSDK: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/internal/controllers/machine/machine_controller.go b/internal/controllers/machine/machine_controller.go index fcb3891c6bff..a462490d956f 100644 --- a/internal/controllers/machine/machine_controller.go +++ b/internal/controllers/machine/machine_controller.go @@ -49,6 +49,7 @@ import ( "sigs.k8s.io/cluster-api/controllers/clustercache" "sigs.k8s.io/cluster-api/controllers/external" "sigs.k8s.io/cluster-api/controllers/noderefutil" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/controllers/machine/drain" "sigs.k8s.io/cluster-api/internal/util/cache" "sigs.k8s.io/cluster-api/internal/util/ssa" @@ -1125,17 +1126,19 @@ func getAttachedVolumeInformation(ctx context.Context, remoteClient client.Clien attachedVolumeName.Insert(string(attachedVolume.Name)) } - volumeAttachments, err := getVolumeAttachmentForNode(ctx, remoteClient, node.GetName()) - if err != nil { - return nil, nil, errors.Wrap(err, "failed to list VolumeAttachments") - } + if feature.Gates.Enabled(feature.MachineWaitForVolumeDetachConsiderVolumeAttachments) { + volumeAttachments, err := getVolumeAttachmentForNode(ctx, remoteClient, node.GetName()) + if err != nil { + return nil, nil, errors.Wrap(err, "failed to list VolumeAttachments") + } - for _, va := range volumeAttachments { - // Return an error if a VolumeAttachments does not refer a PersistentVolume. - if va.Spec.Source.PersistentVolumeName == nil { - return nil, nil, errors.Errorf("spec.source.persistentVolumeName for VolumeAttachment %s is not set", va.GetName()) + for _, va := range volumeAttachments { + // Return an error if a VolumeAttachments does not refer a PersistentVolume. + if va.Spec.Source.PersistentVolumeName == nil { + return nil, nil, errors.Errorf("spec.source.persistentVolumeName for VolumeAttachment %s is not set", va.GetName()) + } + attachedPVNames.Insert(*va.Spec.Source.PersistentVolumeName) } - attachedPVNames.Insert(*va.Spec.Source.PersistentVolumeName) } return attachedVolumeName, attachedPVNames, nil diff --git a/internal/controllers/machine/machine_controller_test.go b/internal/controllers/machine/machine_controller_test.go index 7445a10a2d6e..5a8506ac1b31 100644 --- a/internal/controllers/machine/machine_controller_test.go +++ b/internal/controllers/machine/machine_controller_test.go @@ -30,6 +30,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/tools/record" + utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache/informertest" @@ -44,6 +45,7 @@ import ( "sigs.k8s.io/cluster-api/controllers/clustercache" "sigs.k8s.io/cluster-api/controllers/external" externalfake "sigs.k8s.io/cluster-api/controllers/external/fake" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/util/cache" "sigs.k8s.io/cluster-api/internal/util/ssa" "sigs.k8s.io/cluster-api/util" @@ -2007,6 +2009,7 @@ func TestShouldWaitForNodeVolumes(t *testing.T) { name string node *corev1.Node remoteObjects []client.Object + featureGateDisabled bool expected ctrl.Result expectedDeletingReason string expectedDeletingMessage string @@ -2158,6 +2161,28 @@ func TestShouldWaitForNodeVolumes(t *testing.T) { expectedDeletingMessage: `Waiting for Node volumes to be detached (started at 2024-10-09T16:13:59Z) * PersistentVolumeClaims: default/test-pvc`, }, + { + name: "Node has volumes attached according to volumeattachments (but ignored because feature gate is disabled)", + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeName, + }, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + { + Type: corev1.NodeReady, + Status: corev1.ConditionTrue, + }, + }, + }, + }, + remoteObjects: []client.Object{ + volumeAttachment, + persistentVolume, + }, + featureGateDisabled: true, + expected: ctrl.Result{}, + }, { name: "Node has volumes attached according to volumeattachments but without a pv", node: &corev1.Node{ @@ -2339,6 +2364,12 @@ func TestShouldWaitForNodeVolumes(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) + if tt.featureGateDisabled { + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineWaitForVolumeDetachConsiderVolumeAttachments, false) + } else { + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineWaitForVolumeDetachConsiderVolumeAttachments, true) + } + fakeClient := fake.NewClientBuilder().WithObjects(testCluster).Build() var remoteObjects []client.Object