diff --git a/hack/.golint_failures b/hack/.golint_failures index 64049a4ef4c..8014e1ea08d 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -71,10 +71,8 @@ pkg/apis/settings pkg/apis/settings/v1alpha1 pkg/apis/storage pkg/apis/storage/util -pkg/apis/storage/v1 pkg/apis/storage/v1/util pkg/apis/storage/v1alpha1 -pkg/apis/storage/v1beta1 pkg/apis/storage/v1beta1/util pkg/auth/authorizer/abac pkg/capabilities diff --git a/pkg/apis/storage/v1/BUILD b/pkg/apis/storage/v1/BUILD index e9ce505c90f..aeb7e17d2a7 100644 --- a/pkg/apis/storage/v1/BUILD +++ b/pkg/apis/storage/v1/BUILD @@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"]) load( "@io_bazel_rules_go//go:def.bzl", "go_library", + "go_test", ) go_library( @@ -18,11 +19,13 @@ go_library( deps = [ "//pkg/apis/core:go_default_library", "//pkg/apis/storage:go_default_library", + "//pkg/features:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/api/storage/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/conversion:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", ], ) @@ -41,3 +44,16 @@ filegroup( ], tags = ["automanaged"], ) + +go_test( + name = "go_default_xtest", + srcs = ["defaults_test.go"], + importpath = "k8s.io/kubernetes/pkg/apis/storage/v1_test", + deps = [ + "//pkg/api/legacyscheme:go_default_library", + "//pkg/apis/storage/install:go_default_library", + "//vendor/k8s.io/api/storage/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", + ], +) diff --git a/pkg/apis/storage/v1/defaults.go b/pkg/apis/storage/v1/defaults.go index 2e7c51c632e..6f574f4867c 100644 --- a/pkg/apis/storage/v1/defaults.go +++ b/pkg/apis/storage/v1/defaults.go @@ -20,6 +20,8 @@ import ( "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/kubernetes/pkg/features" ) func addDefaultingFuncs(scheme *runtime.Scheme) error { @@ -31,4 +33,9 @@ func SetDefaults_StorageClass(obj *storagev1.StorageClass) { obj.ReclaimPolicy = new(v1.PersistentVolumeReclaimPolicy) *obj.ReclaimPolicy = v1.PersistentVolumeReclaimDelete } + + if obj.VolumeBindingMode == nil && utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) { + obj.VolumeBindingMode = new(storagev1.VolumeBindingMode) + *obj.VolumeBindingMode = storagev1.VolumeBindingImmediate + } } diff --git a/pkg/apis/storage/v1/defaults_test.go b/pkg/apis/storage/v1/defaults_test.go new file mode 100644 index 00000000000..4fb6304f773 --- /dev/null +++ b/pkg/apis/storage/v1/defaults_test.go @@ -0,0 +1,81 @@ +/* +Copyright 2017 The Kubernetes 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 v1_test + +import ( + "reflect" + "testing" + + storagev1 "k8s.io/api/storage/v1" + "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/kubernetes/pkg/api/legacyscheme" + _ "k8s.io/kubernetes/pkg/apis/storage/install" +) + +func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { + codec := legacyscheme.Codecs.LegacyCodec(storagev1.SchemeGroupVersion) + data, err := runtime.Encode(codec, obj) + if err != nil { + t.Errorf("%v\n %#v", err, obj) + return nil + } + obj2, err := runtime.Decode(codec, data) + if err != nil { + t.Errorf("%v\nData: %s\nSource: %#v", err, string(data), obj) + return nil + } + obj3 := reflect.New(reflect.TypeOf(obj).Elem()).Interface().(runtime.Object) + err = legacyscheme.Scheme.Convert(obj2, obj3, nil) + if err != nil { + t.Errorf("%v\nSource: %#v", err, obj2) + return nil + } + return obj3 +} + +func TestSetDefaultVolumeBindingMode(t *testing.T) { + class := &storagev1.StorageClass{} + + // When feature gate is disabled, field should not be defaulted + output := roundTrip(t, runtime.Object(class)).(*storagev1.StorageClass) + if output.VolumeBindingMode != nil { + t.Errorf("Expected VolumeBindingMode to not be defaulted, got: %+v", output.VolumeBindingMode) + } + + class = &storagev1.StorageClass{} + + err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true") + if err != nil { + t.Fatalf("Failed to enable feature gate for VolumeScheduling: %v", err) + } + + // When feature gate is enabled, field should be defaulted + defaultMode := storagev1.VolumeBindingImmediate + output = roundTrip(t, runtime.Object(class)).(*storagev1.StorageClass) + outMode := output.VolumeBindingMode + if outMode == nil { + t.Errorf("Expected VolumeBindingMode to be defaulted to: %+v, got: nil", defaultMode) + } else if *outMode != defaultMode { + t.Errorf("Expected VolumeBindingMode to be defaulted to: %+v, got: %+v", defaultMode, outMode) + } + + err = utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false") + if err != nil { + t.Fatalf("Failed to disable feature gate for VolumeScheduling: %v", err) + } +} diff --git a/pkg/apis/storage/v1/util/BUILD b/pkg/apis/storage/v1/util/BUILD index 86106067c32..c6603b8e9c2 100644 --- a/pkg/apis/storage/v1/util/BUILD +++ b/pkg/apis/storage/v1/util/BUILD @@ -3,17 +3,13 @@ package(default_visibility = ["//visibility:public"]) load( "@io_bazel_rules_go//go:def.bzl", "go_library", - "go_test", ) go_library( name = "go_default_library", srcs = ["helpers.go"], importpath = "k8s.io/kubernetes/pkg/apis/storage/v1/util", - deps = [ - "//vendor/k8s.io/api/storage/v1:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - ], + deps = ["//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library"], ) filegroup( @@ -28,11 +24,3 @@ filegroup( srcs = [":package-srcs"], tags = ["automanaged"], ) - -go_test( - name = "go_default_test", - srcs = ["helpers_test.go"], - importpath = "k8s.io/kubernetes/pkg/apis/storage/v1/util", - library = ":go_default_library", - deps = ["//vendor/k8s.io/api/storage/v1:go_default_library"], -) diff --git a/pkg/apis/storage/v1/util/helpers.go b/pkg/apis/storage/v1/util/helpers.go index 6e81cfe35ec..d0d5ad7ba99 100644 --- a/pkg/apis/storage/v1/util/helpers.go +++ b/pkg/apis/storage/v1/util/helpers.go @@ -17,7 +17,6 @@ limitations under the License. package util import ( - storagev1 "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -54,10 +53,3 @@ func IsDefaultAnnotation(obj metav1.ObjectMeta) bool { return false } - -// IsBindingModeWaitForFirstConsumer returns true if the VolumeBindingMode is set -// to VolumeBindingWaitForFirstConsumer -func IsBindingModeWaitForFirstConsumer(class *storagev1.StorageClass) bool { - mode := class.VolumeBindingMode - return mode != nil && *mode == storagev1.VolumeBindingWaitForFirstConsumer -} diff --git a/pkg/apis/storage/v1/util/helpers_test.go b/pkg/apis/storage/v1/util/helpers_test.go deleted file mode 100644 index 4b01e41ece1..00000000000 --- a/pkg/apis/storage/v1/util/helpers_test.go +++ /dev/null @@ -1,55 +0,0 @@ -/* -Copyright 2017 The Kubernetes 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 util - -import ( - "testing" - - storagev1 "k8s.io/api/storage/v1" -) - -type bindingTest struct { - class *storagev1.StorageClass - expected bool -} - -func TestIsBindingModeWaitForFirstConsumer(t *testing.T) { - immediateMode := storagev1.VolumeBindingImmediate - waitingMode := storagev1.VolumeBindingWaitForFirstConsumer - cases := map[string]bindingTest{ - "nil binding mode": { - &storagev1.StorageClass{}, - false, - }, - "immediate binding mode": { - &storagev1.StorageClass{VolumeBindingMode: &immediateMode}, - false, - }, - "waiting binding mode": { - &storagev1.StorageClass{VolumeBindingMode: &waitingMode}, - true, - }, - } - - for testName, testCase := range cases { - result := IsBindingModeWaitForFirstConsumer(testCase.class) - if result != testCase.expected { - t.Errorf("Test %q failed. Expected %v, got %v", testName, testCase.expected, result) - } - } - -} diff --git a/pkg/apis/storage/v1beta1/BUILD b/pkg/apis/storage/v1beta1/BUILD index b734d4d6d0d..368302812eb 100644 --- a/pkg/apis/storage/v1beta1/BUILD +++ b/pkg/apis/storage/v1beta1/BUILD @@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"]) load( "@io_bazel_rules_go//go:def.bzl", "go_library", + "go_test", ) go_library( @@ -18,11 +19,13 @@ go_library( deps = [ "//pkg/apis/core:go_default_library", "//pkg/apis/storage:go_default_library", + "//pkg/features:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/api/storage/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/conversion:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", ], ) @@ -41,3 +44,16 @@ filegroup( ], tags = ["automanaged"], ) + +go_test( + name = "go_default_xtest", + srcs = ["defaults_test.go"], + importpath = "k8s.io/kubernetes/pkg/apis/storage/v1beta1_test", + deps = [ + "//pkg/api/legacyscheme:go_default_library", + "//pkg/apis/storage/install:go_default_library", + "//vendor/k8s.io/api/storage/v1beta1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", + ], +) diff --git a/pkg/apis/storage/v1beta1/defaults.go b/pkg/apis/storage/v1beta1/defaults.go index e50599bf273..97dbff2f378 100644 --- a/pkg/apis/storage/v1beta1/defaults.go +++ b/pkg/apis/storage/v1beta1/defaults.go @@ -20,6 +20,8 @@ import ( "k8s.io/api/core/v1" storagev1beta1 "k8s.io/api/storage/v1beta1" "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/kubernetes/pkg/features" ) func addDefaultingFuncs(scheme *runtime.Scheme) error { @@ -31,4 +33,9 @@ func SetDefaults_StorageClass(obj *storagev1beta1.StorageClass) { obj.ReclaimPolicy = new(v1.PersistentVolumeReclaimPolicy) *obj.ReclaimPolicy = v1.PersistentVolumeReclaimDelete } + + if obj.VolumeBindingMode == nil && utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) { + obj.VolumeBindingMode = new(storagev1beta1.VolumeBindingMode) + *obj.VolumeBindingMode = storagev1beta1.VolumeBindingImmediate + } } diff --git a/pkg/apis/storage/v1beta1/defaults_test.go b/pkg/apis/storage/v1beta1/defaults_test.go new file mode 100644 index 00000000000..34aff74b8e0 --- /dev/null +++ b/pkg/apis/storage/v1beta1/defaults_test.go @@ -0,0 +1,81 @@ +/* +Copyright 2017 The Kubernetes 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 v1beta1_test + +import ( + "reflect" + "testing" + + storagev1beta1 "k8s.io/api/storage/v1beta1" + "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/kubernetes/pkg/api/legacyscheme" + _ "k8s.io/kubernetes/pkg/apis/storage/install" +) + +func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { + codec := legacyscheme.Codecs.LegacyCodec(storagev1beta1.SchemeGroupVersion) + data, err := runtime.Encode(codec, obj) + if err != nil { + t.Errorf("%v\n %#v", err, obj) + return nil + } + obj2, err := runtime.Decode(codec, data) + if err != nil { + t.Errorf("%v\nData: %s\nSource: %#v", err, string(data), obj) + return nil + } + obj3 := reflect.New(reflect.TypeOf(obj).Elem()).Interface().(runtime.Object) + err = legacyscheme.Scheme.Convert(obj2, obj3, nil) + if err != nil { + t.Errorf("%v\nSource: %#v", err, obj2) + return nil + } + return obj3 +} + +func TestSetDefaultVolumeBindingMode(t *testing.T) { + class := &storagev1beta1.StorageClass{} + + // When feature gate is disabled, field should not be defaulted + output := roundTrip(t, runtime.Object(class)).(*storagev1beta1.StorageClass) + if output.VolumeBindingMode != nil { + t.Errorf("Expected VolumeBindingMode to not be defaulted, got: %+v", output.VolumeBindingMode) + } + + class = &storagev1beta1.StorageClass{} + + err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true") + if err != nil { + t.Fatalf("Failed to enable feature gate for VolumeScheduling: %v", err) + } + + // When feature gate is enabled, field should be defaulted + defaultMode := storagev1beta1.VolumeBindingImmediate + output = roundTrip(t, runtime.Object(class)).(*storagev1beta1.StorageClass) + outMode := output.VolumeBindingMode + if outMode == nil { + t.Errorf("Expected VolumeBindingMode to be defaulted to: %+v, got: nil", defaultMode) + } else if *outMode != defaultMode { + t.Errorf("Expected VolumeBindingMode to be defaulted to: %+v, got: %+v", defaultMode, outMode) + } + + err = utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false") + if err != nil { + t.Fatalf("Failed to disable feature gate for VolumeScheduling: %v", err) + } +} diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go index bcfc960e8aa..e9f66e938e8 100644 --- a/pkg/apis/storage/validation/validation.go +++ b/pkg/apis/storage/validation/validation.go @@ -227,13 +227,14 @@ var supportedVolumeBindingModes = sets.NewString(string(storage.VolumeBindingImm // validateVolumeBindingMode tests that VolumeBindingMode specifies valid values. func validateVolumeBindingMode(mode *storage.VolumeBindingMode, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if mode != nil { - if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) { - allErrs = append(allErrs, field.Forbidden(fldPath, "field is disabled by feature-gate VolumeScheduling")) - } - if !supportedVolumeBindingModes.Has(string(*mode)) { + if utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) { + if mode == nil { + allErrs = append(allErrs, field.Required(fldPath, "")) + } else if !supportedVolumeBindingModes.Has(string(*mode)) { allErrs = append(allErrs, field.NotSupported(fldPath, mode, supportedVolumeBindingModes.List())) } + } else if mode != nil { + allErrs = append(allErrs, field.Forbidden(fldPath, "field is disabled by feature-gate VolumeScheduling")) } return allErrs diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index c1701688780..2cb0a61a414 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -478,7 +478,7 @@ func TestValidateVolumeBindingMode(t *testing.T) { cases := map[string]bindingTest{ "no mode": { class: makeClassWithBinding(nil), - shouldSucceed: true, + shouldSucceed: false, }, "immediate mode": { class: makeClassWithBinding(&immediateMode1),