Merge pull request #3345 from andreychernih/enable-critical-feature-gate

Automatic merge from submit-queue. .

Enable ExperimentalCriticalPodAnnotation feature gate

Otherwise, it is possible that critical system components will be evicted

https://github.com/kubernetes/kops/issues/3194
https://github.com/kubernetes/kubernetes/issues/51432

docs/cluster_spec.md

@@ -172,11 +172,13 @@ Will result in the flag `--resolv-conf=` being built.
 spec:
   kubelet:
     featureGates:
-      ExperimentalCriticalPodAnnotation: "true"
+      Accelerators: "true"
       AllowExtTrafficLocalEndpoints: "false"
 ```
 
-Will result in the flag `--feature-gates=ExperimentalCriticalPodAnnotation=true,AllowExtTrafficLocalEndpoints=false`
+Will result in the flag `--feature-gates=Accelerators=true,AllowExtTrafficLocalEndpoints=false`
+
+NOTE: Feature gate `ExperimentalCriticalPodAnnotation` is enabled by default because some critical components like `kube-proxy` depend on its presence.
 
 ####  Compute Resources Reservation
 

docs/releases/1.8-NOTES.md

@@ -6,3 +6,7 @@ _This is a WIP document describing changes to the upcoming kops 1.8 release_
 is not recommended, but will be the default value for existing clusters or clusters created via manifests.
 `kops create cluster` with `--networking flannel` will use `vxlan`, `--networking flannel-vxlan`
 or `--networking flannel-udp` can be specified to explicitly choose a backend mode.
+
+# Full changelist
+
+* ExperimentalCriticalPodAnnotation feature gate is now enabled by default in kubelet [@andreychernih](https://github.com/andreychernih) [#3345](https://github.com/kubernetes/kops/pull/3345)

pkg/apis/kops/util/versions.go

@@ -65,25 +65,10 @@ func ParseKubernetesVersion(version string) (*semver.Version, error) {
 // TODO: Convert to our own KubernetesVersion type?
 
 func IsKubernetesGTE(version string, k8sVersion semver.Version) bool {
-	// The string-arg is a little annoying, but simplifies the calling code!
-	switch version {
-	case "1.2":
-		return k8sVersion.Major > 1 || (k8sVersion.Major == 1 && k8sVersion.Minor >= 2)
-	case "1.3":
-		return k8sVersion.Major > 1 || (k8sVersion.Major == 1 && k8sVersion.Minor >= 3)
-	case "1.4":
-		return k8sVersion.Major > 1 || (k8sVersion.Major == 1 && k8sVersion.Minor >= 4)
-	case "1.5":
-		return k8sVersion.Major > 1 || (k8sVersion.Major == 1 && k8sVersion.Minor >= 5)
-	case "1.6":
-		return k8sVersion.Major > 1 || (k8sVersion.Major == 1 && k8sVersion.Minor >= 6)
-	case "1.7":
-		return k8sVersion.Major > 1 || (k8sVersion.Major == 1 && k8sVersion.Minor >= 7)
-	case "1.8":
-		return k8sVersion.Major > 1 || (k8sVersion.Major == 1 && k8sVersion.Minor >= 8)
-	case "1.9":
-		return k8sVersion.Major > 1 || (k8sVersion.Major == 1 && k8sVersion.Minor >= 9)
-	default:
-		panic(fmt.Sprintf("IsKubernetesGTE not supported with version %q", version))
+	parsedVersion, err := ParseKubernetesVersion(version)
+	if err != nil {
+		panic(fmt.Sprintf("Error parsing version %s: %v", version, err))
 	}
+
+	return k8sVersion.GTE(*parsedVersion)
 }

pkg/apis/kops/util/versions_test.go

@@ -40,3 +40,46 @@ func Test_ParseKubernetesVersion(t *testing.T) {
 	}
 
 }
+
+func Test_IsKubernetesGTEWithPatch(t *testing.T) {
+	currentVersion, err := ParseKubernetesVersion("1.6.2")
+	if err != nil {
+		t.Fatalf("Error parsing version: %v", err)
+	}
+
+	grid := map[string]bool{
+		"1.5.2": true,
+		"1.6.2": true,
+		"1.6.5": false,
+		"1.7.8": false,
+	}
+
+	for v, expected := range grid {
+		actual := IsKubernetesGTE(v, *currentVersion)
+		if actual != expected {
+			t.Errorf("expected %s to be >= than %s", v, currentVersion)
+		}
+	}
+}
+
+func Test_IsKubernetesGTEWithoutPatch(t *testing.T) {
+	currentVersion, err := ParseKubernetesVersion("1.6")
+	if err != nil {
+		t.Fatalf("Error parsing version: %v", err)
+	}
+
+	grid := map[string]bool{
+		"1.1": true,
+		"1.2": true,
+		"1.3": true,
+		"1.6": true,
+		"1.7": false,
+	}
+
+	for v, expected := range grid {
+		actual := IsKubernetesGTE(v, *currentVersion)
+		if actual != expected {
+			t.Errorf("expected %s to be >= than %s", v, currentVersion)
+		}
+	}
+}

pkg/model/components/kubelet.go

@@ -17,11 +17,12 @@ limitations under the License.
 package components
 
 import (
+	"strings"
+
 	"github.com/golang/glog"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/loader"
-	"strings"
 )
 
 // KubeletOptionsBuilder adds options for kubelets
@@ -191,5 +192,14 @@ func (b *KubeletOptionsBuilder) BuildOptions(o interface{}) error {
 	}
 	clusterSpec.Kubelet.PodInfraContainerImage = image
 
+	if clusterSpec.Kubelet.FeatureGates == nil {
+		clusterSpec.Kubelet.FeatureGates = make(map[string]string)
+	}
+	if _, found := clusterSpec.Kubelet.FeatureGates["ExperimentalCriticalPodAnnotation"]; !found {
+		if b.Context.IsKubernetesGTE("1.5.2") {
+			clusterSpec.Kubelet.FeatureGates["ExperimentalCriticalPodAnnotation"] = "true"
+		}
+	}
+
 	return nil
 }

pkg/model/components/kubelet_test.go

@@ -0,0 +1,101 @@
+/*
+Copyright 2016 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 components
+
+import (
+	"testing"
+
+	"k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/pkg/assets"
+)
+
+func buildSpec() *kops.ClusterSpec {
+	spec := kops.ClusterSpec{
+		KubernetesVersion:     "1.6.2",
+		ServiceClusterIPRange: "10.10.0.0/16",
+		Kubelet:               &kops.KubeletConfigSpec{},
+	}
+
+	return &spec
+}
+
+func buildOptions(spec *kops.ClusterSpec) error {
+	ab := assets.NewAssetBuilder(nil)
+
+	ver, err := KubernetesVersion(spec)
+	if err != nil {
+		return err
+	}
+
+	builder := KubeletOptionsBuilder{
+		Context: &OptionsContext{
+			AssetBuilder:      ab,
+			KubernetesVersion: *ver,
+		},
+	}
+
+	err = builder.BuildOptions(spec)
+	if err != nil {
+		return nil
+	}
+
+	return nil
+}
+
+func TestFeatureGates(t *testing.T) {
+	spec := buildSpec()
+	err := buildOptions(spec)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	gates := spec.Kubelet.FeatureGates
+	if gates["ExperimentalCriticalPodAnnotation"] != "true" {
+		t.Errorf("ExperimentalCriticalPodAnnotation feature gate should be enabled by default")
+	}
+}
+
+func TestFeatureGatesKubernetesVersion(t *testing.T) {
+	spec := buildSpec()
+	spec.KubernetesVersion = "1.4.0"
+	err := buildOptions(spec)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	gates := spec.Kubelet.FeatureGates
+	if _, found := gates["ExperimentalCriticalPodAnnotation"]; found {
+		t.Errorf("ExperimentalCriticalPodAnnotation feature gate should not be added on Kubernetes < 1.5.2")
+	}
+}
+
+func TestFeatureGatesOverride(t *testing.T) {
+	spec := buildSpec()
+	spec.Kubelet.FeatureGates = map[string]string{
+		"ExperimentalCriticalPodAnnotation": "false",
+	}
+
+	err := buildOptions(spec)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	gates := spec.Kubelet.FeatureGates
+	if gates["ExperimentalCriticalPodAnnotation"] != "false" {
+		t.Errorf("ExperimentalCriticalPodAnnotation feature should be disalbled")
+	}
+}