Merge pull request #2257 from DualSpark/kubelet-feature-gate

Adding feature gates flag for kubelet, and unit tests

docs/cluster_spec.md

@@ -118,6 +118,23 @@ spec:
 
 Will result in the flag `--runtime-config=batch/v2alpha1=true,apps/v1alpha1=true`. Note that `kube-apiserver` accepts `true` as a value for switch-like flags.
 
+### kubelet
+
+This block contains configurations for `kubelet`.  See https://kubernetes.io/docs/admin/kubelet/
+
+####  Feature Gates
+
+```yaml
+spec:
+  kubelet:
+    featureGates:
+      ExperimentalCriticalPodAnnotation: "true"
+      AllowExtTrafficLocalEndpoints: "false"
+```
+
+Will result in the flag `--feature-gates=ExperimentalCriticalPodAnnotation=true,AllowExtTrafficLocalEndpoints=false`
+
+
 ### networkID
 
 On AWS, this is the id of the VPC the cluster is created in. If creating a cluster from scratch, this field doesn't need to be specified at create time; `kops` will create a `VPC` for you.

nodeup/pkg/model/kubelet.go

@@ -44,39 +44,14 @@ func (b *KubeletBuilder) Build(c *fi.ModelBuilderContext) error {
 		return fmt.Errorf("error building kubelet config: %v", err)
 	}
 
-	// Add sysconfig file
-	{
-		// TODO: Dump this - just complexity!
-		flags, err := flagbuilder.BuildFlags(kubeletConfig)
-		if err != nil {
-			return fmt.Errorf("error building kubelet flags: %v", err)
-		}
-
-		// Add cloud config file if needed
-		// We build this flag differently because it depends on CloudConfig, and to expose it directly
-		// would be a degree of freedom we don't have (we'd have to write the config to different files)
-		// We can always add this later if it is needed.
-		if b.Cluster.Spec.CloudConfig != nil {
-			flags += " --cloud-config=" + CloudConfigFilePath
-		}
-
-		flags += " --network-plugin-dir=" + b.NetworkPluginDir()
-
-		sysconfig := "DAEMON_ARGS=\"" + flags + "\"\n"
-
-		t := &nodetasks.File{
-			Path:     "/etc/sysconfig/kubelet",
-			Contents: fi.NewStringResource(sysconfig),
-			Type:     nodetasks.FileType_File,
-		}
-		c.AddTask(t)
-	}
+	b.buildSysConfig(c, kubeletConfig)
 
 	// Add kubelet file itself (as an asset)
 	{
 		// TODO: Extract to common function?
 		assetName := "kubelet"
 		assetPath := ""
+		// TODO make Find call to an interface, we cannot mock out this function because it finds a file on disk
 		asset, err := b.Assets.Find(assetName, assetPath)
 		if err != nil {
 			return fmt.Errorf("error trying to locate asset %q: %v", assetName, err)
@@ -128,6 +103,36 @@ func (b *KubeletBuilder) Build(c *fi.ModelBuilderContext) error {
 	return nil
 }
 
+// buildSysConfig adds a task to create a sysconfig file for kubelet
+func (b *KubeletBuilder) buildSysConfig(c *fi.ModelBuilderContext, kubeletConfig *kops.KubeletConfigSpec) error {
+
+	// TODO: Dump this - just complexity!
+	flags, err := flagbuilder.BuildFlags(kubeletConfig)
+	if err != nil {
+		return fmt.Errorf("error building kubelet flags: %v", err)
+	}
+
+	// Add cloud config file if needed
+	// We build this flag differently because it depends on CloudConfig, and to expose it directly
+	// would be a degree of freedom we don't have (we'd have to write the config to different files)
+	// We can always add this later if it is needed.
+	if b.Cluster.Spec.CloudConfig != nil {
+		flags += " --cloud-config=" + CloudConfigFilePath
+	}
+
+	flags += " --network-plugin-dir=" + b.NetworkPluginDir()
+
+	sysconfig := "DAEMON_ARGS=\"" + flags + "\"\n"
+
+	t := &nodetasks.File{
+		Path:     "/etc/sysconfig/kubelet",
+		Contents: fi.NewStringResource(sysconfig),
+		Type:     nodetasks.FileType_File,
+	}
+	c.AddTask(t)
+	return nil
+}
+
 func (b *KubeletBuilder) kubeletPath() string {
 	kubeletCommand := "/usr/local/bin/kubelet"
 	if b.Distribution == distros.DistributionCoreOS {

nodeup/pkg/model/kubelet_test.go

@@ -17,9 +17,19 @@ limitations under the License.
 package model
 
 import (
-	"k8s.io/kops/pkg/apis/kops"
-	"k8s.io/kops/upup/pkg/fi"
+	"bytes"
+	"io/ioutil"
+	"path"
+	"sort"
+	"strings"
 	"testing"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/kops/nodeup/pkg/distros"
+	"k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/pkg/apis/kops/v1alpha2"
+	"k8s.io/kops/pkg/diff"
+	"k8s.io/kops/upup/pkg/fi"
 )
 
 func Test_InstanceGroupKubeletMerge(t *testing.T) {
@@ -142,3 +152,107 @@ func stringSlicesEqual(exp, other []string) bool {
 
 	return true
 }
+
+func Test_RunKubeletBuilder(t *testing.T) {
+	runKubeletBuilderTest(t, "featuregates")
+}
+
+func runKubeletBuilderTest(t *testing.T, key string) {
+	basedir := path.Join("tests/kubelet/", key)
+
+	clusterYamlPath := path.Join(basedir, "cluster.yaml")
+	clusterYaml, err := ioutil.ReadFile(clusterYamlPath)
+	if err != nil {
+		t.Fatalf("error reading cluster yaml file %q: %v", clusterYamlPath, err)
+	}
+
+	var cluster *kops.Cluster
+	var instanceGroup *kops.InstanceGroup
+
+	// Codecs provides access to encoding and decoding for the scheme
+	codecs := kops.Codecs
+
+	codec := codecs.UniversalDecoder(kops.SchemeGroupVersion)
+
+	sections := bytes.Split(clusterYaml, []byte("\n---\n"))
+	for _, section := range sections {
+		defaults := &schema.GroupVersionKind{
+			Group:   v1alpha2.SchemeGroupVersion.Group,
+			Version: v1alpha2.SchemeGroupVersion.Version,
+		}
+		o, gvk, err := codec.Decode(section, defaults, nil)
+		if err != nil {
+			t.Errorf("error parsing file %v", err)
+		}
+
+		switch v := o.(type) {
+		case *kops.Cluster:
+			cluster = v
+		case *kops.InstanceGroup:
+			instanceGroup = v
+		default:
+			t.Errorf("Unhandled kind %q", gvk)
+		}
+	}
+
+	context := &fi.ModelBuilderContext{
+		Tasks: make(map[string]fi.Task),
+	}
+	nodeUpModelContext := &NodeupModelContext{
+		Cluster:       cluster,
+		Architecture:  "amd64",
+		Distribution:  distros.DistributionXenial,
+		InstanceGroup: instanceGroup,
+	}
+
+	builder := KubeletBuilder{NodeupModelContext: nodeUpModelContext}
+
+	kubeletConfig, err := builder.buildKubeletConfig()
+	if err != nil {
+		t.Errorf("error building kubelet config: %v", err)
+	}
+
+	// because of the diff we cannot test maps that include multiple values
+	// as maps are not sorted and will change
+	kubeletConfig.NodeLabels = make(map[string]string)
+	kubeletConfig.NodeLabels["kubernetes.io/role"] = "node"
+
+	err = builder.buildSysConfig(context, kubeletConfig)
+	if err != nil {
+		t.Fatalf("error from KubeletBuilder Build: %v", err)
+	}
+
+	var keys []string
+	for key := range context.Tasks {
+		keys = append(keys, key)
+	}
+	sort.Strings(keys)
+
+	var yamls []string
+	for _, key := range keys {
+		task := context.Tasks[key]
+		yaml, err := kops.ToRawYaml(task)
+		if err != nil {
+			t.Fatalf("error serializing task: %v", err)
+		}
+		yamls = append(yamls, strings.TrimSpace(string(yaml)))
+	}
+
+	actualTasksYaml := strings.Join(yamls, "\n---\n")
+
+	tasksYamlPath := path.Join(basedir, "tasks.yaml")
+	expectedTasksYamlBytes, err := ioutil.ReadFile(tasksYamlPath)
+	if err != nil {
+		t.Fatalf("error reading file %q: %v", tasksYamlPath, err)
+	}
+
+	actualTasksYaml = strings.TrimSpace(actualTasksYaml)
+	expectedTasksYaml := strings.TrimSpace(string(expectedTasksYamlBytes))
+
+	if expectedTasksYaml != actualTasksYaml {
+		diffString := diff.FormatDiff(expectedTasksYaml, actualTasksYaml)
+		t.Logf("diff:\n%s\n", diffString)
+
+		t.Fatalf("tasks differed from expected for test %q", key)
+	}
+}

nodeup/pkg/model/tests/kubelet/featuregates/cluster.yaml

@@ -0,0 +1,74 @@
+apiVersion: kops/v1alpha2
+kind: Cluster
+metadata:
+  creationTimestamp: "2016-12-10T22:42:27Z"
+  name: minimal.example.com
+spec:
+  kubernetesApiAccess:
+  - 0.0.0.0/0
+  channel: stable
+  cloudProvider: aws
+  configBase: memfs://clusters.example.com/minimal.example.com
+  etcdClusters:
+  - etcdMembers:
+    - instanceGroup: master-us-test-1a
+      name: master-us-test-1a
+    name: main
+  - etcdMembers:
+    - instanceGroup: master-us-test-1a
+      name: master-us-test-1a
+    name: events
+  kubelet:
+    allowPrivileged: true
+    apiServers: https://api.internal.minimal.example.com
+    babysitDaemons: true
+    cgroupRoot: docker
+    cloudProvider: aws
+    clusterDNS: 100.64.0.10
+    clusterDomain: cluster.local
+    enableDebuggingHandlers: true
+    evictionHard: memory.available<100Mi,nodefs.available<10%,nodefs.inodesFree<5%,imagefs.available<10%,imagefs.inodesFree<5%
+    hostnameOverride: '@aws'
+    logLevel: 2
+    networkPluginName: cni
+    nonMasqueradeCIDR: 100.64.0.0/10
+    podManifestPath: /etc/kubernetes/manifests
+    featureGates:
+      ExperimentalCriticalPodAnnotation: "true"
+# You cannot test two since in the output the order is not guaranteed
+#     AllowExtTrafficLocalEndpoints: "false"
+  kubernetesVersion: v1.5.0
+  masterInternalName: api.internal.minimal.example.com
+  masterPublicName: api.minimal.example.com
+  networkCIDR: 172.20.0.0/16
+  networking:
+    kubenet: {}
+  nonMasqueradeCIDR: 100.64.0.0/10
+  sshAccess:
+    - 0.0.0.0/0
+  topology:
+    masters: public
+    nodes: public
+  subnets:
+  - cidr: 172.20.32.0/19
+    name: us-test-1a
+    type: Public
+    zone: us-test-1a
+---
+
+apiVersion: kops/v1alpha2
+kind: InstanceGroup
+metadata:
+  creationTimestamp: "2016-12-10T22:42:28Z"
+  name: nodes
+  labels:
+    kops.k8s.io/cluster: minimal.example.com
+spec:
+  associatePublicIp: true
+  image: kope.io/k8s-1.4-debian-jessie-amd64-hvm-ebs-2016-10-21
+  machineType: t2.medium
+  maxSize: 2
+  minSize: 2
+  role: Node
+  subnets:
+  - us-test-1a

nodeup/pkg/model/tests/kubelet/featuregates/tasks.yaml

@@ -0,0 +1,4 @@
+contents: |
+  DAEMON_ARGS="--allow-privileged=true --api-servers=https://api.internal.minimal.example.com --babysit-daemons=true --cgroup-root=docker --cloud-provider=aws --cluster-dns=100.64.0.10 --cluster-domain=cluster.local --enable-debugging-handlers=true --eviction-hard=memory.available<100Mi,nodefs.available<10%,nodefs.inodesFree<5%,imagefs.available<10%,imagefs.inodesFree<5% --feature-gates=ExperimentalCriticalPodAnnotation=true --hostname-override=@aws --network-plugin=cni --node-labels=kubernetes.io/role=node --non-masquerade-cidr=100.64.0.0/10 --pod-manifest-path=/etc/kubernetes/manifests --v=2 --network-plugin-dir=/opt/cni/bin/"
+path: /etc/sysconfig/kubelet
+type: file

pkg/apis/kops/componentconfig.go

@@ -319,6 +319,9 @@ type KubeletConfigSpec struct {
 
 	// Taints to add when registering a node in the cluster
 	Taints []string `json:"taints,omitempty" flag:"register-with-taints"`
+
+	// FeatureGates is set of key=value pairs that describe feature gates for alpha/experimental features.
+	FeatureGates map[string]string `json:"featureGates,omitempty" flag:"feature-gates"`
 }
 
 type KubeProxyConfig struct {

pkg/apis/kops/v1alpha1/componentconfig.go

@@ -318,6 +318,9 @@ type KubeletConfigSpec struct {
 
 	// Taints to add when registering a node in the cluster
 	Taints []string `json:"taints,omitempty" flag:"register-with-taints"`
+
+	// FeatureGates is set of key=value pairs that describe feature gates for alpha/experimental features.
+	FeatureGates map[string]string `json:"featureGates,omitempty" flag:"feature-gates"`
 }
 
 type KubeProxyConfig struct {

pkg/apis/kops/v1alpha1/zz_generated.conversion.go

@@ -1365,6 +1365,7 @@ func autoConvert_v1alpha1_KubeletConfigSpec_To_kops_KubeletConfigSpec(in *Kubele
 	out.EvictionMinimumReclaim = in.EvictionMinimumReclaim
 	out.VolumePluginDirectory = in.VolumePluginDirectory
 	out.Taints = in.Taints
+	out.FeatureGates = in.FeatureGates
 	return nil
 }
 
@@ -1411,6 +1412,7 @@ func autoConvert_kops_KubeletConfigSpec_To_v1alpha1_KubeletConfigSpec(in *kops.K
 	out.EvictionMinimumReclaim = in.EvictionMinimumReclaim
 	out.VolumePluginDirectory = in.VolumePluginDirectory
 	out.Taints = in.Taints
+	out.FeatureGates = in.FeatureGates
 	return nil
 }
 

pkg/apis/kops/v1alpha2/componentconfig.go

@@ -140,6 +140,9 @@ type KubeletConfigSpec struct {
 
 	// Taints to add when registering a node in the cluster
 	Taints []string `json:"taints,omitempty" flag:"register-with-taints"`
+
+	// FeatureGates is set of key=value pairs that describe feature gates for alpha/experimental features.
+	FeatureGates map[string]string `json:"featureGates,omitempty" flag:"feature-gates"`
 }
 
 type KubeProxyConfig struct {

pkg/apis/kops/v1alpha2/zz_generated.conversion.go

@@ -1463,6 +1463,7 @@ func autoConvert_v1alpha2_KubeletConfigSpec_To_kops_KubeletConfigSpec(in *Kubele
 	out.EvictionMinimumReclaim = in.EvictionMinimumReclaim
 	out.VolumePluginDirectory = in.VolumePluginDirectory
 	out.Taints = in.Taints
+	out.FeatureGates = in.FeatureGates
 	return nil
 }
 
@@ -1509,6 +1510,7 @@ func autoConvert_kops_KubeletConfigSpec_To_v1alpha2_KubeletConfigSpec(in *kops.K
 	out.EvictionMinimumReclaim = in.EvictionMinimumReclaim
 	out.VolumePluginDirectory = in.VolumePluginDirectory
 	out.Taints = in.Taints
+	out.FeatureGates = in.FeatureGates
 	return nil
 }