- Update feature gates to support key={true,false} syntax (#677)

- Enable StableScheduling by default

charts/tidb-operator/values.yaml

@@ -46,7 +46,7 @@ scheduler:
   replicas: 1
   schedulerName: tidb-scheduler
   # features:
-  # - StableScheduling
+  # - StableScheduling=true
   resources:
     limits:
       cpu: 250m

docs/design-proposals/tidb-stable-scheduling.md

@@ -111,7 +111,7 @@ Add a new flag in tidb-scheduler which accepts a comma-separated list of
 string.
 
 ```
-tidb-scheduler --features StableScheduling
+tidb-scheduler --features StableScheduling=true
 ```
 
 tidb-scheduler will enable this functionality only when `StableScheduling`

pkg/features/features.go

@@ -23,7 +23,10 @@ import (
 )
 
 var (
-	allFeatures = sets.NewString(StableScheduling)
+	allFeatures     = sets.NewString(StableScheduling)
+	defaultFeatures = map[string]bool{
+		StableScheduling: true,
+	}
 	// DefaultFeatureGate is a shared global FeatureGate.
 	DefaultFeatureGate FeatureGate = NewFeatureGate()
 )
@@ -41,21 +44,26 @@ type FeatureGate interface {
 }
 
 type featureGate struct {
-	defaultFeatures sets.String
-	enabledFeatures sets.String
+	enabledFeatures map[string]bool
 }
 
 func (f *featureGate) AddFlag(flagset *flag.FlagSet) {
-	flag.Var(utilflags.NewStringSetValue(f.defaultFeatures, &f.enabledFeatures), "features", fmt.Sprintf("features to enable, comma-separated list of string, available: %s", strings.Join(allFeatures.List(), ",")))
+	flag.Var(utilflags.NewMapStringBool(&f.enabledFeatures), "features", fmt.Sprintf("A set of key={true,false} pairs to enable/disable features, available features: %s", strings.Join(allFeatures.List(), ",")))
 }
 
 func (f *featureGate) Enabled(key string) bool {
-	return f.enabledFeatures.Has(key)
+	if b, ok := f.enabledFeatures[key]; ok {
+		return b
+	}
+	return false
 }
 
 func NewFeatureGate() FeatureGate {
-	return &featureGate{
-		defaultFeatures: sets.NewString(),
-		enabledFeatures: sets.NewString(),
+	f := &featureGate{
+		enabledFeatures: make(map[string]bool),
+	}
+	for k, v := range defaultFeatures {
+		f.enabledFeatures[k] = v
 	}
+	return f
 }

pkg/util/flags/map_string_bool.go

@@ -0,0 +1,106 @@
+// Copyright 2019 PingCAP, Inc.
+//
+// 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,
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// This is copied from https://raw.githubusercontent.com/kubernetes/component-base/release-1.14/cli/flag/map_string_bool.go.
+// TODO Vendor it when our dependencies has been upgraded to 1.14+.
+
+/*
+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 flags
+
+import (
+	"fmt"
+	"sort"
+	"strconv"
+	"strings"
+)
+
+// MapStringBool can be set from the command line with the format `--flag "string=bool"`.
+// Multiple comma-separated key-value pairs in a single invocation are supported. For example: `--flag "a=true,b=false"`.
+// Multiple flag invocations are supported. For example: `--flag "a=true" --flag "b=false"`.
+type MapStringBool struct {
+	Map         *map[string]bool
+	initialized bool
+}
+
+// NewMapStringBool takes a pointer to a map[string]string and returns the
+// MapStringBool flag parsing shim for that map
+func NewMapStringBool(m *map[string]bool) *MapStringBool {
+	return &MapStringBool{Map: m}
+}
+
+// String implements github.com/spf13/pflag.Value
+func (m *MapStringBool) String() string {
+	if m == nil || m.Map == nil {
+		return ""
+	}
+	pairs := []string{}
+	for k, v := range *m.Map {
+		pairs = append(pairs, fmt.Sprintf("%s=%t", k, v))
+	}
+	sort.Strings(pairs)
+	return strings.Join(pairs, ",")
+}
+
+// Set implements github.com/spf13/pflag.Value
+func (m *MapStringBool) Set(value string) error {
+	if m.Map == nil {
+		return fmt.Errorf("no target (nil pointer to map[string]bool)")
+	}
+	if !m.initialized || *m.Map == nil {
+		// clear default values, or allocate if no existing map
+		*m.Map = make(map[string]bool)
+		m.initialized = true
+	}
+	for _, s := range strings.Split(value, ",") {
+		if len(s) == 0 {
+			continue
+		}
+		arr := strings.SplitN(s, "=", 2)
+		if len(arr) != 2 {
+			return fmt.Errorf("malformed pair, expect string=bool")
+		}
+		k := strings.TrimSpace(arr[0])
+		v := strings.TrimSpace(arr[1])
+		boolValue, err := strconv.ParseBool(v)
+		if err != nil {
+			return fmt.Errorf("invalid value of %s: %s, err: %v", k, v, err)
+		}
+		(*m.Map)[k] = boolValue
+	}
+	return nil
+}
+
+// Type implements github.com/spf13/pflag.Value
+func (*MapStringBool) Type() string {
+	return "mapStringBool"
+}
+
+// Empty implements OmitEmpty
+func (m *MapStringBool) Empty() bool {
+	return len(*m.Map) == 0
+}

tests/cmd/e2e/main.go

@@ -21,7 +21,7 @@ import (
 
 	"github.com/pingcap/tidb-operator/tests/pkg/apimachinery"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 
 	"github.com/golang/glog"
 	"github.com/jinzhu/copier"
@@ -49,7 +49,7 @@ func main() {
 		SchedulerImage: "mirantis/hypokube",
 		SchedulerTag:   "final",
 		SchedulerFeatures: []string{
-			"StableScheduling",
+			"StableScheduling=true",
 		},
 		LogLevel:           "2",
 		WebhookServiceName: "webhook-service",

tests/cmd/stability/stability.go

@@ -4,7 +4,7 @@ import (
 	"fmt"
 
 	"github.com/pingcap/tidb-operator/tests"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 )
 
 func newOperatorConfig() *tests.OperatorConfig {
@@ -15,7 +15,7 @@ func newOperatorConfig() *tests.OperatorConfig {
 		Tag:            cfg.OperatorTag,
 		SchedulerImage: "gcr.io/google-containers/hyperkube",
 		SchedulerFeatures: []string{
-			"StableScheduling",
+			"StableScheduling=true",
 		},
 		LogLevel:           "2",
 		WebhookServiceName: tests.WebhookServiceName,