From a9d71bd6e63617ca19b2679793b7d0519007c751 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Fri, 25 Oct 2024 13:39:43 -0400 Subject: [PATCH 1/2] Move RecoverVolumeExpansionFailure feature to beta --- pkg/apis/core/validation/validation_test.go | 8 +++++--- pkg/features/kube_features.go | 1 + pkg/features/versioned_kube_features.go | 1 + .../util/operationexecutor/node_expander.go | 18 +++++++++++++++++- .../operationexecutor/node_expander_test.go | 13 ++++++++++++- .../operation_generator_test.go | 5 +++++ 6 files changed, 41 insertions(+), 5 deletions(-) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 54dd4ad7328..02ae1b19428 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -3066,19 +3066,21 @@ func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) { "nil pv": { oldPvc: nil, expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ - EnableRecoverFromExpansionFailure: false, + EnableRecoverFromExpansionFailure: true, EnableVolumeAttributesClass: false, }, }, "invaild apiGroup in dataSource allowed because the old pvc is used": { oldPvc: pvcWithDataSource(&core.TypedLocalObjectReference{APIGroup: &invaildAPIGroup}), expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ + EnableRecoverFromExpansionFailure: true, AllowInvalidAPIGroupInDataSourceOrRef: true, }, }, "invaild apiGroup in dataSourceRef allowed because the old pvc is used": { oldPvc: pvcWithDataSourceRef(&core.TypedObjectReference{APIGroup: &invaildAPIGroup}), expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ + EnableRecoverFromExpansionFailure: true, AllowInvalidAPIGroupInDataSourceOrRef: true, }, }, @@ -3086,7 +3088,7 @@ func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) { oldPvc: pvcWithVolumeAttributesClassName(utilpointer.String("foo")), enableVolumeAttributesClass: true, expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ - EnableRecoverFromExpansionFailure: false, + EnableRecoverFromExpansionFailure: true, EnableVolumeAttributesClass: true, }, }, @@ -3094,7 +3096,7 @@ func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) { oldPvc: pvcWithVolumeAttributesClassName(utilpointer.String("foo")), enableVolumeAttributesClass: false, expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ - EnableRecoverFromExpansionFailure: false, + EnableRecoverFromExpansionFailure: true, EnableVolumeAttributesClass: true, }, }, diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index de48eb83d42..339538f14d3 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -488,6 +488,7 @@ const ( // owner: @gnufied // kep: https://kep.k8s.io/1790 + // beta - v1.32 // // Allow users to recover from volume expansion failure RecoverVolumeExpansionFailure featuregate.Feature = "RecoverVolumeExpansionFailure" diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go index 5fb54c41247..bbbc8aabb9f 100644 --- a/pkg/features/versioned_kube_features.go +++ b/pkg/features/versioned_kube_features.go @@ -592,6 +592,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate RecoverVolumeExpansionFailure: { {Version: version.MustParse("1.23"), Default: false, PreRelease: featuregate.Alpha}, + {Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.Beta}, }, RecursiveReadOnlyMounts: { {Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Alpha}, diff --git a/pkg/volume/util/operationexecutor/node_expander.go b/pkg/volume/util/operationexecutor/node_expander.go index 9cd2b5878a1..b55d63bb243 100644 --- a/pkg/volume/util/operationexecutor/node_expander.go +++ b/pkg/volume/util/operationexecutor/node_expander.go @@ -97,7 +97,9 @@ func (ne *NodeExpander) runPreCheck() bool { // PVC is already expanded but we are still trying to expand the volume because // last recorded size in ASOW is older. This can happen for RWX volume types. - if ne.pvcStatusCap.Cmp(ne.pluginResizeOpts.NewSize) >= 0 && ne.resizeStatus == "" { + if ne.pvcStatusCap.Cmp(ne.pluginResizeOpts.NewSize) >= 0 && + ne.resizeStatus == "" && + ne.hasReadWriteMany() { ne.pvcAlreadyUpdated = true return true } @@ -118,6 +120,20 @@ func (ne *NodeExpander) runPreCheck() bool { return false } +func (ne *NodeExpander) hasReadWriteMany() bool { + accessModes := ne.pvc.Spec.AccessModes + if accessModes == nil { + return false + } + + for _, mode := range accessModes { + if mode == v1.ReadWriteMany { + return true + } + } + return false +} + func (ne *NodeExpander) expandOnPlugin() (bool, resource.Quantity, error) { allowExpansion := ne.runPreCheck() if !allowExpansion { diff --git a/pkg/volume/util/operationexecutor/node_expander_test.go b/pkg/volume/util/operationexecutor/node_expander_test.go index 4ac020005e1..d3ed12b673f 100644 --- a/pkg/volume/util/operationexecutor/node_expander_test.go +++ b/pkg/volume/util/operationexecutor/node_expander_test.go @@ -110,10 +110,21 @@ func TestNodeExpander(t *testing.T) { expectedStatusSize: resource.MustParse("1G"), }, { - name: "pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize", + name: "RWO volumes, pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize", pvc: getTestPVC("test-vol0", "2G", "2G", "2G", nil), pv: getTestPV("test-vol0", "2G"), + expectedResizeStatus: "", + expectResizeCall: false, + assumeResizeOpAsFinished: true, + expectFinalErrors: false, + expectedStatusSize: resource.MustParse("2G"), + }, + { + name: "RWOP volumes, pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize", + pvc: addAccessMode(getTestPVC("test-vol0", "2G", "2G", "2G", nil), v1.ReadWriteMany), + pv: getTestPV("test-vol0", "2G"), + expectedResizeStatus: "", expectResizeCall: true, assumeResizeOpAsFinished: true, diff --git a/pkg/volume/util/operationexecutor/operation_generator_test.go b/pkg/volume/util/operationexecutor/operation_generator_test.go index 79cf22bfc5d..9d628b779ca 100644 --- a/pkg/volume/util/operationexecutor/operation_generator_test.go +++ b/pkg/volume/util/operationexecutor/operation_generator_test.go @@ -455,6 +455,11 @@ func getTestPVC(volumeName string, specSize, statusSize, allocatedSize string, r return pvc } +func addAccessMode(pvc *v1.PersistentVolumeClaim, mode v1.PersistentVolumeAccessMode) *v1.PersistentVolumeClaim { + pvc.Spec.AccessModes = append(pvc.Spec.AccessModes, mode) + return pvc +} + func getTestPV(volumeName string, specSize string) *v1.PersistentVolume { return &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ From 2d58d4ef529ddc0673114f546fd658e511f5c4f6 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Mon, 28 Oct 2024 16:04:54 -0400 Subject: [PATCH 2/2] Fix unit tests and feature gate stuff --- .../util/operationexecutor/node_expander.go | 17 ++--------------- .../operationexecutor/node_expander_test.go | 2 +- .../operation_generator_test.go | 2 +- .../test_data/versioned_feature_list.yaml | 4 ++++ 4 files changed, 8 insertions(+), 17 deletions(-) diff --git a/pkg/volume/util/operationexecutor/node_expander.go b/pkg/volume/util/operationexecutor/node_expander.go index b55d63bb243..fff1760d0f5 100644 --- a/pkg/volume/util/operationexecutor/node_expander.go +++ b/pkg/volume/util/operationexecutor/node_expander.go @@ -24,6 +24,7 @@ import ( clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" + "k8s.io/kubectl/pkg/util/storage" kevents "k8s.io/kubernetes/pkg/kubelet/events" "k8s.io/kubernetes/pkg/volume/util" volumetypes "k8s.io/kubernetes/pkg/volume/util/types" @@ -99,7 +100,7 @@ func (ne *NodeExpander) runPreCheck() bool { // last recorded size in ASOW is older. This can happen for RWX volume types. if ne.pvcStatusCap.Cmp(ne.pluginResizeOpts.NewSize) >= 0 && ne.resizeStatus == "" && - ne.hasReadWriteMany() { + storage.ContainsAccessMode(ne.pvc.Spec.AccessModes, v1.ReadWriteMany) { ne.pvcAlreadyUpdated = true return true } @@ -120,20 +121,6 @@ func (ne *NodeExpander) runPreCheck() bool { return false } -func (ne *NodeExpander) hasReadWriteMany() bool { - accessModes := ne.pvc.Spec.AccessModes - if accessModes == nil { - return false - } - - for _, mode := range accessModes { - if mode == v1.ReadWriteMany { - return true - } - } - return false -} - func (ne *NodeExpander) expandOnPlugin() (bool, resource.Quantity, error) { allowExpansion := ne.runPreCheck() if !allowExpansion { diff --git a/pkg/volume/util/operationexecutor/node_expander_test.go b/pkg/volume/util/operationexecutor/node_expander_test.go index d3ed12b673f..e95a42d5ce3 100644 --- a/pkg/volume/util/operationexecutor/node_expander_test.go +++ b/pkg/volume/util/operationexecutor/node_expander_test.go @@ -121,7 +121,7 @@ func TestNodeExpander(t *testing.T) { expectedStatusSize: resource.MustParse("2G"), }, { - name: "RWOP volumes, pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize", + name: "RWX volumes, pv.spec.cap = pvc.status.cap, resizeStatus='', desiredSize > actualSize", pvc: addAccessMode(getTestPVC("test-vol0", "2G", "2G", "2G", nil), v1.ReadWriteMany), pv: getTestPV("test-vol0", "2G"), diff --git a/pkg/volume/util/operationexecutor/operation_generator_test.go b/pkg/volume/util/operationexecutor/operation_generator_test.go index 9d628b779ca..7bf0dabd38b 100644 --- a/pkg/volume/util/operationexecutor/operation_generator_test.go +++ b/pkg/volume/util/operationexecutor/operation_generator_test.go @@ -255,7 +255,7 @@ func TestOperationGenerator_nodeExpandVolume(t *testing.T) { actualSize: getSizeFunc("1G"), expectedResizeStatus: "", - resizeCallCount: 1, + resizeCallCount: 0, expectedStatusSize: resource.MustParse("2G"), }, { diff --git a/test/featuregates_linter/test_data/versioned_feature_list.yaml b/test/featuregates_linter/test_data/versioned_feature_list.yaml index 04636a22bb0..389acca3995 100644 --- a/test/featuregates_linter/test_data/versioned_feature_list.yaml +++ b/test/featuregates_linter/test_data/versioned_feature_list.yaml @@ -922,6 +922,10 @@ lockToDefault: false preRelease: Alpha version: "1.23" + - default: true + lockToDefault: false + preRelease: Beta + version: "1.32" - name: RecursiveReadOnlyMounts versionedSpecs: - default: false