Merge pull request #72250 from sbezverk/AllowVolumeExpansion

AllowVolumeExpansion validation and tests
pull/564/head
Kubernetes Prow Robot 2018-12-26 16:20:24 -08:00 committed by GitHub
commit 52b6b4086f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 95 additions and 49 deletions

View File

@ -42,6 +42,7 @@ go_test(
"//pkg/apis/core:go_default_library",
"//pkg/apis/storage:go_default_library",
"//pkg/features:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature/testing:go_default_library",
],

View File

@ -32,4 +32,17 @@ func DropDisabledFields(class, oldClass *storage.StorageClass) {
oldClass.AllowedTopologies = nil
}
}
if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) && !allowVolumeExpansionInUse(oldClass) {
class.AllowVolumeExpansion = nil
}
}
func allowVolumeExpansionInUse(oldClass *storage.StorageClass) bool {
if oldClass == nil {
return false
}
if oldClass.AllowVolumeExpansion != nil {
return true
}
return false
}

View File

@ -17,9 +17,11 @@ limitations under the License.
package util
import (
"fmt"
"reflect"
"testing"
"k8s.io/apimachinery/pkg/util/diff"
utilfeature "k8s.io/apiserver/pkg/util/feature"
utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing"
api "k8s.io/kubernetes/pkg/apis/core"
@ -68,3 +70,82 @@ func TestDropAlphaFields(t *testing.T) {
t.Errorf("AllowedTopologies field got unexpectantly modified: %+v", class.AllowedTopologies)
}
}
func TestDropAllowVolumeExpansion(t *testing.T) {
allowVolumeExpansion := false
scWithoutAllowVolumeExpansion := func() *storage.StorageClass {
return &storage.StorageClass{}
}
scWithAllowVolumeExpansion := func() *storage.StorageClass {
return &storage.StorageClass{
AllowVolumeExpansion: &allowVolumeExpansion,
}
}
scInfo := []struct {
description string
hasAllowVolumeExpansion bool
sc func() *storage.StorageClass
}{
{
description: "StorageClass Without AllowVolumeExpansion",
hasAllowVolumeExpansion: false,
sc: scWithoutAllowVolumeExpansion,
},
{
description: "StorageClass With AllowVolumeExpansion",
hasAllowVolumeExpansion: true,
sc: scWithAllowVolumeExpansion,
},
{
description: "is nil",
hasAllowVolumeExpansion: false,
sc: func() *storage.StorageClass { return nil },
},
}
for _, enabled := range []bool{true, false} {
for _, oldStorageClassInfo := range scInfo {
for _, newStorageClassInfo := range scInfo {
oldStorageClassHasAllowVolumeExpansion, oldStorageClass := oldStorageClassInfo.hasAllowVolumeExpansion, oldStorageClassInfo.sc()
newStorageClassHasAllowVolumeExpansion, newStorageClass := newStorageClassInfo.hasAllowVolumeExpansion, newStorageClassInfo.sc()
if newStorageClass == nil {
continue
}
t.Run(fmt.Sprintf("feature enabled=%v, old StorageClass %v, new StorageClass %v", enabled, oldStorageClassInfo.description, newStorageClassInfo.description), func(t *testing.T) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, enabled)()
DropDisabledFields(newStorageClass, oldStorageClass)
// old StorageClass should never be changed
if !reflect.DeepEqual(oldStorageClass, oldStorageClassInfo.sc()) {
t.Errorf("old StorageClass changed: %v", diff.ObjectReflectDiff(oldStorageClass, oldStorageClassInfo.sc()))
}
switch {
case enabled || oldStorageClassHasAllowVolumeExpansion:
// new StorageClass should not be changed if the feature is enabled, or if the old StorageClass had AllowVolumeExpansion
if !reflect.DeepEqual(newStorageClass, newStorageClassInfo.sc()) {
t.Errorf("new StorageClass changed: %v", diff.ObjectReflectDiff(newStorageClass, newStorageClassInfo.sc()))
}
case newStorageClassHasAllowVolumeExpansion:
// new StorageClass should be changed
if reflect.DeepEqual(newStorageClass, newStorageClassInfo.sc()) {
t.Errorf("new StorageClass was not changed")
}
// new StorageClass should not have AllowVolumeExpansion
if !reflect.DeepEqual(newStorageClass, scWithoutAllowVolumeExpansion()) {
t.Errorf("new StorageClass had StorageClassAllowVolumeExpansion: %v", diff.ObjectReflectDiff(newStorageClass, scWithoutAllowVolumeExpansion()))
}
default:
// new StorageClass should not need to be changed
if !reflect.DeepEqual(newStorageClass, newStorageClassInfo.sc()) {
t.Errorf("new StorageClass changed: %v", diff.ObjectReflectDiff(newStorageClass, newStorageClassInfo.sc()))
}
}
})
}
}
}
}

View File

@ -46,7 +46,6 @@ func ValidateStorageClass(storageClass *storage.StorageClass) field.ErrorList {
allErrs = append(allErrs, validateProvisioner(storageClass.Provisioner, field.NewPath("provisioner"))...)
allErrs = append(allErrs, validateParameters(storageClass.Parameters, field.NewPath("parameters"))...)
allErrs = append(allErrs, validateReclaimPolicy(storageClass.ReclaimPolicy, field.NewPath("reclaimPolicy"))...)
allErrs = append(allErrs, validateAllowVolumeExpansion(storageClass.AllowVolumeExpansion, field.NewPath("allowVolumeExpansion"))...)
allErrs = append(allErrs, validateVolumeBindingMode(storageClass.VolumeBindingMode, field.NewPath("volumeBindingMode"))...)
allErrs = append(allErrs, validateAllowedTopologies(storageClass.AllowedTopologies, field.NewPath("allowedTopologies"))...)
@ -123,16 +122,6 @@ func validateReclaimPolicy(reclaimPolicy *api.PersistentVolumeReclaimPolicy, fld
return allErrs
}
// validateAllowVolumeExpansion tests that if ExpandPersistentVolumes feature gate is disabled, whether the AllowVolumeExpansion filed
// of storage class is set
func validateAllowVolumeExpansion(allowExpand *bool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if allowExpand != nil && !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) {
allErrs = append(allErrs, field.Forbidden(fldPath, "field is disabled by feature-gate ExpandPersistentVolumes"))
}
return allErrs
}
// ValidateVolumeAttachment validates a VolumeAttachment. This function is common for v1 and v1beta1 objects,
func ValidateVolumeAttachment(volumeAttachment *storage.VolumeAttachment) field.ErrorList {
allErrs := apivalidation.ValidateObjectMeta(&volumeAttachment.ObjectMeta, false, apivalidation.ValidateClassName, field.NewPath("metadata"))

View File

@ -140,32 +140,6 @@ func TestValidateStorageClass(t *testing.T) {
}
}
func TestAlphaExpandPersistentVolumesFeatureValidation(t *testing.T) {
deleteReclaimPolicy := api.PersistentVolumeReclaimPolicy("Delete")
falseVar := false
testSC := &storage.StorageClass{
// empty parameters
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
Provisioner: "kubernetes.io/foo-provisioner",
Parameters: map[string]string{},
ReclaimPolicy: &deleteReclaimPolicy,
AllowVolumeExpansion: &falseVar,
VolumeBindingMode: &immediateMode1,
}
// Enable feature ExpandPersistentVolumes
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, true)()
if errs := ValidateStorageClass(testSC); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
// Disable feature ExpandPersistentVolumes
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, false)()
if errs := ValidateStorageClass(testSC); len(errs) == 0 {
t.Errorf("expected failure, but got no error")
}
}
func TestVolumeAttachmentValidation(t *testing.T) {
volumeName := "pv-name"
empty := ""

View File

@ -18,11 +18,9 @@ go_library(
"//pkg/apis/storage:go_default_library",
"//pkg/apis/storage/util:go_default_library",
"//pkg/apis/storage/validation:go_default_library",
"//pkg/features:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
],
)

View File

@ -22,12 +22,10 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api/legacyscheme"
"k8s.io/kubernetes/pkg/apis/storage"
storageutil "k8s.io/kubernetes/pkg/apis/storage/util"
"k8s.io/kubernetes/pkg/apis/storage/validation"
"k8s.io/kubernetes/pkg/features"
)
// storageClassStrategy implements behavior for StorageClass objects
@ -48,10 +46,6 @@ func (storageClassStrategy) NamespaceScoped() bool {
func (storageClassStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
class := obj.(*storage.StorageClass)
if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) {
class.AllowVolumeExpansion = nil
}
storageutil.DropDisabledFields(class, nil)
}
@ -73,10 +67,6 @@ func (storageClassStrategy) PrepareForUpdate(ctx context.Context, obj, old runti
newClass := obj.(*storage.StorageClass)
oldClass := old.(*storage.StorageClass)
if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) {
newClass.AllowVolumeExpansion = nil
oldClass.AllowVolumeExpansion = nil
}
storageutil.DropDisabledFields(oldClass, newClass)
}