Add defaulting to VolumeBindingMode

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

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",
+    ],
+)

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
+	}
 }

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)
+	}
+}

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 = [
+    deps = ["//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library"],
-        "//vendor/k8s.io/api/storage/v1:go_default_library",
-        "//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"],
-)

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
-}

pkg/apis/storage/v1/util/helpers_test.go

@@ -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)
-		}
-	}
-
-}

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",
+    ],
+)

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
+	}
 }

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)
+	}
+}

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) {
-		if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) {
+		if mode == nil {
-			allErrs = append(allErrs, field.Forbidden(fldPath, "field is disabled by feature-gate VolumeScheduling"))
+			allErrs = append(allErrs, field.Required(fldPath, ""))
-		}
+		} else if !supportedVolumeBindingModes.Has(string(*mode)) {
-		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

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),