Merge pull request #50976 from fabriziopandini/smallRefactor-kubeadm-featureGates

Automatic merge from submit-queue (batch tested with PRs 50229, 50973, 50976, 51085, 51084)

Small improvements to kubeadm feature-gates

**What this PR does / why we need it**:
This PR implements two small improvements for kubeadm feature-gates:
- Move the generic features package from `cmd/kubeadm/app/cmd/features`to ` cmd/kubeadm/app/features`
- Add CLI flag for `cfg.FeatureFlags`

**Which issue this PR fixes**: 
fixes [#393](https://github.com/kubernetes/kubeadm/issues/393)
fixes [#394](https://github.com/kubernetes/kubeadm/issues/394)

**Special notes for your reviewer**:
Main work of this PR is grouped into two commits, one for each issue + a separated commit for autogenerated bazel files.
cc @luxas 
cc @jamiehannaford

cmd/kubeadm/app/BUILD

@@ -31,6 +31,7 @@ filegroup(
         "//cmd/kubeadm/app/cmd:all-srcs",
         "//cmd/kubeadm/app/constants:all-srcs",
         "//cmd/kubeadm/app/discovery:all-srcs",
+        "//cmd/kubeadm/app/features:all-srcs",
         "//cmd/kubeadm/app/images:all-srcs",
         "//cmd/kubeadm/app/node:all-srcs",
         "//cmd/kubeadm/app/phases/addons/dns:all-srcs",

cmd/kubeadm/app/apis/kubeadm/validation/BUILD

@@ -22,8 +22,8 @@ go_library(
     srcs = ["validation.go"],
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
-        "//cmd/kubeadm/app/cmd/features:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
+        "//cmd/kubeadm/app/features:go_default_library",
         "//cmd/kubeadm/app/util:go_default_library",
         "//cmd/kubeadm/app/util/token:go_default_library",
         "//pkg/api/validation:go_default_library",

cmd/kubeadm/app/apis/kubeadm/validation/validation.go

@@ -29,8 +29,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
-	"k8s.io/kubernetes/cmd/kubeadm/app/cmd/features"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
+	"k8s.io/kubernetes/cmd/kubeadm/app/features"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	tokenutil "k8s.io/kubernetes/cmd/kubeadm/app/util/token"
 	apivalidation "k8s.io/kubernetes/pkg/api/validation"

cmd/kubeadm/app/cmd/BUILD

@@ -22,10 +22,10 @@ go_library(
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
         "//cmd/kubeadm/app/apis/kubeadm/v1alpha1:go_default_library",
         "//cmd/kubeadm/app/apis/kubeadm/validation:go_default_library",
-        "//cmd/kubeadm/app/cmd/features:go_default_library",
         "//cmd/kubeadm/app/cmd/phases:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
         "//cmd/kubeadm/app/discovery:go_default_library",
+        "//cmd/kubeadm/app/features:go_default_library",
         "//cmd/kubeadm/app/phases/addons/dns:go_default_library",
         "//cmd/kubeadm/app/phases/addons/proxy:go_default_library",
         "//cmd/kubeadm/app/phases/apiconfig:go_default_library",
@@ -94,7 +94,6 @@ filegroup(
     name = "all-srcs",
     srcs = [
         ":package-srcs",
-        "//cmd/kubeadm/app/cmd/features:all-srcs",
         "//cmd/kubeadm/app/cmd/phases:all-srcs",
     ],
     tags = ["automanaged"],

cmd/kubeadm/app/cmd/config.go

@@ -27,6 +27,7 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
+	"k8s.io/kubernetes/cmd/kubeadm/app/features"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/uploadconfig"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config"
@@ -128,6 +129,8 @@ func NewCmdConfigUploadFromFlags(out io.Writer, kubeConfigFile *string) *cobra.C
 	cfg := &kubeadmapiext.MasterConfiguration{}
 	api.Scheme.Default(cfg)
 
+	var featureFlagsString string
+
 	cmd := &cobra.Command{
 		Use:   "from-flags",
 		Short: "Create the in-cluster configuration file for the first time from using flags",
@@ -137,6 +140,11 @@ func NewCmdConfigUploadFromFlags(out io.Writer, kubeConfigFile *string) *cobra.C
 			same flags before upgrading to v1.8 using 'kubeadm upgrade'.
 		`), metav1.NamespaceSystem, constants.MasterConfigurationConfigMap),
 		Run: func(cmd *cobra.Command, args []string) {
+			var err error
+			if cfg.FeatureFlags, err = features.NewFeatureGate(&features.InitFeatureGates, featureFlagsString); err != nil {
+				kubeadmutil.CheckErr(err)
+			}
+
 			client, err := kubeconfigutil.ClientSetFromFile(*kubeConfigFile)
 			kubeadmutil.CheckErr(err)
 
@@ -146,7 +154,7 @@ func NewCmdConfigUploadFromFlags(out io.Writer, kubeConfigFile *string) *cobra.C
 			kubeadmutil.CheckErr(err)
 		},
 	}
-	AddInitConfigFlags(cmd.PersistentFlags(), cfg)
+	AddInitConfigFlags(cmd.PersistentFlags(), cfg, &featureFlagsString)
 	return cmd
 }
 

cmd/kubeadm/app/cmd/init.go

@@ -21,6 +21,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"strings"
 	"text/template"
 	"time"
 
@@ -33,8 +34,8 @@ import (
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
 	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation"
-	"k8s.io/kubernetes/cmd/kubeadm/app/cmd/features"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
+	"k8s.io/kubernetes/cmd/kubeadm/app/features"
 	dnsaddonphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/dns"
 	proxyaddonphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/proxy"
 	apiconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/apiconfig"
@@ -89,10 +90,17 @@ func NewCmdInit(out io.Writer) *cobra.Command {
 	var skipPreFlight bool
 	var skipTokenPrint bool
 	var dryRun bool
+	var featureFlagsString string
+
 	cmd := &cobra.Command{
 		Use:   "init",
 		Short: "Run this in order to set up the Kubernetes master",
 		Run: func(cmd *cobra.Command, args []string) {
+			var err error
+			if cfg.FeatureFlags, err = features.NewFeatureGate(&features.InitFeatureGates, featureFlagsString); err != nil {
+				kubeadmutil.CheckErr(err)
+			}
+
 			api.Scheme.Default(cfg)
 			internalcfg := &kubeadmapi.MasterConfiguration{}
 			api.Scheme.Convert(cfg, internalcfg, nil)
@@ -110,14 +118,14 @@ func NewCmdInit(out io.Writer) *cobra.Command {
 		},
 	}
 
-	AddInitConfigFlags(cmd.PersistentFlags(), cfg)
+	AddInitConfigFlags(cmd.PersistentFlags(), cfg, &featureFlagsString)
 	AddInitOtherFlags(cmd.PersistentFlags(), &cfgPath, &skipPreFlight, &skipTokenPrint, &dryRun)
 
 	return cmd
 }
 
 // AddInitConfigFlags adds init flags bound to the config to the specified flagset
-func AddInitConfigFlags(flagSet *flag.FlagSet, cfg *kubeadmapiext.MasterConfiguration) {
+func AddInitConfigFlags(flagSet *flag.FlagSet, cfg *kubeadmapiext.MasterConfiguration, featureFlagsString *string) {
 	flagSet.StringVar(
 		&cfg.API.AdvertiseAddress, "apiserver-advertise-address", cfg.API.AdvertiseAddress,
 		"The IP address the API Server will advertise it's listening on. 0.0.0.0 means the default network interface's address.",
@@ -162,6 +170,8 @@ func AddInitConfigFlags(flagSet *flag.FlagSet, cfg *kubeadmapiext.MasterConfigur
 		&cfg.TokenTTL, "token-ttl", cfg.TokenTTL,
 		"The duration before the bootstrap token is automatically deleted. 0 means 'never expires'.",
 	)
+	flagSet.StringVar(featureFlagsString, "feature-gates", *featureFlagsString, "A set of key=value pairs that describe feature gates for various features. "+
+		"Options are:\n"+strings.Join(features.KnownFeatures(&features.InitFeatureGates), "\n"))
 }
 
 // AddInitOtherFlags adds init flags that are not bound to a configuration file to the given flagset

cmd/kubeadm/app/cmd/phases/BUILD

@@ -26,6 +26,7 @@ go_library(
         "//cmd/kubeadm/app/apis/kubeadm/v1alpha1:go_default_library",
         "//cmd/kubeadm/app/apis/kubeadm/validation:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
+        "//cmd/kubeadm/app/features:go_default_library",
         "//cmd/kubeadm/app/phases/bootstraptoken/clusterinfo:go_default_library",
         "//cmd/kubeadm/app/phases/bootstraptoken/node:go_default_library",
         "//cmd/kubeadm/app/phases/certs:go_default_library",

cmd/kubeadm/app/cmd/phases/selfhosting.go

@@ -17,10 +17,13 @@ limitations under the License.
 package phases
 
 import (
+	"strings"
+
 	"github.com/spf13/cobra"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
+	"k8s.io/kubernetes/cmd/kubeadm/app/features"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/selfhosting"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
@@ -29,13 +32,19 @@ import (
 
 // NewCmdSelfhosting returns the self-hosting Cobra command
 func NewCmdSelfhosting() *cobra.Command {
-	var kubeConfigFile string
+	var kubeConfigFile, featureFlagsString string
 	cfg := &kubeadmapiext.MasterConfiguration{}
 	cmd := &cobra.Command{
 		Use:     "selfhosting",
 		Aliases: []string{"selfhosted"},
 		Short:   "Make a kubeadm cluster self-hosted.",
 		Run: func(cmd *cobra.Command, args []string) {
+
+			var err error
+			if cfg.FeatureFlags, err = features.NewFeatureGate(&features.InitFeatureGates, featureFlagsString); err != nil {
+				kubeadmutil.CheckErr(err)
+			}
+
 			api.Scheme.Default(cfg)
 			internalcfg := &kubeadmapi.MasterConfiguration{}
 			api.Scheme.Convert(cfg, internalcfg, nil)
@@ -48,5 +57,8 @@ func NewCmdSelfhosting() *cobra.Command {
 	}
 
 	cmd.Flags().StringVar(&kubeConfigFile, "kubeconfig", "/etc/kubernetes/admin.conf", "The KubeConfig file to use for talking to the cluster")
+	cmd.Flags().StringVar(&featureFlagsString, "feature-gates", featureFlagsString, "A set of key=value pairs that describe feature gates for various features."+
+		"Options are:\n"+strings.Join(features.KnownFeatures(&features.InitFeatureGates), "\n"))
+
 	return cmd
 }

cmd/kubeadm/app/features/BUILD

@@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
+    "go_test",
 )
 
 go_library(
@@ -23,3 +24,10 @@ filegroup(
     srcs = [":package-srcs"],
     tags = ["automanaged"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["features_test.go"],
+    library = ":go_default_library",
+    deps = ["//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library"],
+)

cmd/kubeadm/app/features/features.go

@@ -17,6 +17,11 @@ limitations under the License.
 package features
 
 import (
+	"fmt"
+	"sort"
+	"strconv"
+	"strings"
+
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 )
 
@@ -61,3 +66,48 @@ var InitFeatureGates = FeatureList{
 	SelfHosting:         {Default: false, PreRelease: utilfeature.Beta},
 	StoreCertsInSecrets: {Default: false, PreRelease: utilfeature.Alpha},
 }
+
+// KnownFeatures returns a slice of strings describing the FeatureList features.
+func KnownFeatures(f *FeatureList) []string {
+	var known []string
+	for k, v := range *f {
+		pre := ""
+		if v.PreRelease != utilfeature.GA {
+			pre = fmt.Sprintf("%s - ", v.PreRelease)
+		}
+		known = append(known, fmt.Sprintf("%s=true|false (%sdefault=%t)", k, pre, v.Default))
+	}
+	sort.Strings(known)
+	return known
+}
+
+// NewFeatureGate parse a string of the form "key1=value1,key2=value2,..." into a
+// map[string]bool of known keys or returns an error.
+func NewFeatureGate(f *FeatureList, value string) (map[string]bool, error) {
+	featureGate := map[string]bool{}
+	for _, s := range strings.Split(value, ",") {
+		if len(s) == 0 {
+			continue
+		}
+
+		arr := strings.SplitN(s, "=", 2)
+		if len(arr) != 2 {
+			return nil, fmt.Errorf("missing bool value for feature-gate key:%s", s)
+		}
+
+		k := strings.TrimSpace(arr[0])
+		v := strings.TrimSpace(arr[1])
+
+		if !Supports(*f, k) {
+			return nil, fmt.Errorf("unrecognized feature-gate key: %s", k)
+		}
+
+		boolValue, err := strconv.ParseBool(v)
+		if err != nil {
+			return nil, fmt.Errorf("invalid value %v for feature-gate key: %s, use true|false instead", v, k)
+		}
+		featureGate[k] = boolValue
+	}
+
+	return featureGate, nil
+}

cmd/kubeadm/app/features/features_test.go

@@ -0,0 +1,119 @@
+/*
+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 features
+
+import (
+	"reflect"
+	"testing"
+
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+)
+
+func TestKnownFeatures(t *testing.T) {
+	var someFeatures = FeatureList{
+		"feature2": {Default: true, PreRelease: utilfeature.Alpha},
+		"feature1": {Default: false, PreRelease: utilfeature.Beta},
+		"feature3": {Default: false, PreRelease: utilfeature.GA},
+	}
+
+	r := KnownFeatures(&someFeatures)
+
+	if len(r) != 3 {
+		t.Errorf("KnownFeatures returned %d values, expected 3", len(r))
+	}
+
+	// check the first value is feature1 (the list should be sorted); prerelease and default should be present
+	f1 := "feature1=true|false (BETA - default=false)"
+	if r[0] != f1 {
+		t.Errorf("KnownFeatures returned %s values, expected %s", r[0], f1)
+	}
+	// check the second value is feature2; prerelease and default should be present
+	f2 := "feature2=true|false (ALPHA - default=true)"
+	if r[1] != f2 {
+		t.Errorf("KnownFeatures returned %s values, expected %s", r[1], f2)
+	}
+	// check the second value is feature3; prerelease should not shown fo GA features; default should be present
+	f3 := "feature3=true|false (default=false)"
+	if r[2] != f3 {
+		t.Errorf("KnownFeatures returned %s values, expected %s", r[2], f3)
+	}
+}
+
+func TestNewFeatureGate(t *testing.T) {
+	var someFeatures = FeatureList{
+		"feature1": {Default: false, PreRelease: utilfeature.Beta},
+		"feature2": {Default: true, PreRelease: utilfeature.Alpha},
+	}
+
+	var tests = []struct {
+		value                string
+		expectedError        bool
+		expectedFeaturesGate map[string]bool
+	}{
+		{ //invalid value (missing =)
+			value:         "invalidValue",
+			expectedError: true,
+		},
+		{ //invalid value (missing =)
+			value:         "feature1=true,invalidValue",
+			expectedError: true,
+		},
+		{ //invalid value (not a boolean)
+			value:         "feature1=notABoolean",
+			expectedError: true,
+		},
+		{ //invalid value (not a boolean)
+			value:         "feature1=true,feature2=notABoolean",
+			expectedError: true,
+		},
+		{ //unrecognized feature-gate key
+			value:         "unknownFeature=false",
+			expectedError: true,
+		},
+		{ //unrecognized feature-gate key
+			value:         "feature1=true,unknownFeature=false",
+			expectedError: true,
+		},
+		{ //one feature
+			value:                "feature1=true",
+			expectedError:        false,
+			expectedFeaturesGate: map[string]bool{"feature1": true},
+		},
+		{ //two features
+			value:                "feature1=true,feature2=false",
+			expectedError:        false,
+			expectedFeaturesGate: map[string]bool{"feature1": true, "feature2": false},
+		},
+	}
+
+	for _, test := range tests {
+
+		r, err := NewFeatureGate(&someFeatures, test.value)
+
+		if !test.expectedError && err != nil {
+			t.Errorf("NewFeatureGate failed when not expected: %v", err)
+			continue
+		} else if test.expectedError && err == nil {
+			t.Error("NewFeatureGate didn't failed when expected")
+			continue
+		}
+
+		if !reflect.DeepEqual(r, test.expectedFeaturesGate) {
+			t.Errorf("NewFeatureGate returned a unexpected value")
+		}
+	}
+}

cmd/kubeadm/app/phases/selfhosting/BUILD

@@ -30,8 +30,8 @@ go_library(
     ],
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
-        "//cmd/kubeadm/app/cmd/features:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
+        "//cmd/kubeadm/app/features:go_default_library",
         "//cmd/kubeadm/app/util:go_default_library",
         "//cmd/kubeadm/app/util/apiclient:go_default_library",
         "//pkg/api:go_default_library",

cmd/kubeadm/app/phases/selfhosting/selfhosting.go

@@ -28,8 +28,8 @@ import (
 	kuberuntime "k8s.io/apimachinery/pkg/runtime"
 	clientset "k8s.io/client-go/kubernetes"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
-	"k8s.io/kubernetes/cmd/kubeadm/app/cmd/features"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
+	"k8s.io/kubernetes/cmd/kubeadm/app/features"
 	"k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient"
 	"k8s.io/kubernetes/pkg/api"
 )