Merge pull request #9415 from johngmyers/refactor-nodeup-2

Continue moving InstanceGroup data to NodeupConfig

cmd/kops-controller/controllers/node_controller.go

@@ -114,10 +114,7 @@ func (r *NodeReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		return ctrl.Result{}, fmt.Errorf("unable to load instance group object for node %s: %v", node.Name, err)
 	}
 
-	labels, err := nodelabels.BuildNodeLabels(cluster, ig)
-	if err != nil {
-		return ctrl.Result{}, fmt.Errorf("unable to build config for node %s: %v", node.Name, err)
-	}
+	labels := nodelabels.BuildNodeLabels(cluster, ig)
 
 	lifecycle, err := r.getInstanceLifecycle(ctx, node)
 	if err != nil {

nodeup/pkg/model/BUILD.bazel

@@ -63,7 +63,6 @@ go_library(
         "//util/pkg/architectures:go_default_library",
         "//util/pkg/exec:go_default_library",
         "//util/pkg/proxy:go_default_library",
-        "//util/pkg/reflectutils:go_default_library",
         "//util/pkg/vfs:go_default_library",
         "//vendor/github.com/aws/aws-sdk-go/aws:go_default_library",
         "//vendor/github.com/aws/aws-sdk-go/aws/ec2metadata:go_default_library",

nodeup/pkg/model/context.go

@@ -355,31 +355,9 @@ func (c *NodeupModelContext) UseBootstrapTokens() bool {
 	return c.Cluster.Spec.Kubelet != nil && c.Cluster.Spec.Kubelet.BootstrapKubeconfig != ""
 }
 
-// UseSecureKubelet checks if the kubelet api should be protected by a client certificate. Note: the settings are
-// in one of three section, master specific kubelet, cluster wide kubelet or the InstanceGroup. Though arguably is
-// doesn't make much sense to unset this on a per InstanceGroup level, but hey :)
+// UseSecureKubelet checks if the kubelet api should be protected by a client certificate.
 func (c *NodeupModelContext) UseSecureKubelet() bool {
-	cluster := &c.Cluster.Spec // just to shorten the typing
-	group := &c.InstanceGroup.Spec
-
-	// @check on the InstanceGroup itself
-	if group.Kubelet != nil && group.Kubelet.AnonymousAuth != nil && !*group.Kubelet.AnonymousAuth {
-		return true
-	}
-
-	// @check if we have anything specific to master kubelet
-	if c.IsMaster {
-		if cluster.MasterKubelet != nil && cluster.MasterKubelet.AnonymousAuth != nil && !*cluster.MasterKubelet.AnonymousAuth {
-			return true
-		}
-	}
-
-	// @check the default settings for master and kubelet
-	if cluster.Kubelet != nil && cluster.Kubelet.AnonymousAuth != nil && !*cluster.Kubelet.AnonymousAuth {
-		return true
-	}
-
-	return false
+	return c.NodeupConfig.KubeletConfig.AnonymousAuth != nil && !*c.NodeupConfig.KubeletConfig.AnonymousAuth
 }
 
 // KubectlPath returns distro based path for kubectl

nodeup/pkg/model/kubelet.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"path"
 	"path/filepath"
-	"strings"
 
 	"k8s.io/kops/pkg/model/components"
 
@@ -38,7 +37,6 @@ import (
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
 	"k8s.io/kops/upup/pkg/fi/nodeup/nodetasks"
-	"k8s.io/kops/util/pkg/reflectutils"
 )
 
 const (
@@ -422,12 +420,7 @@ func (b *KubeletBuilder) buildKubeletConfigSpec() (*kops.KubeletConfigSpec, erro
 	isMaster := b.IsMaster
 
 	// Merge KubeletConfig for NodeLabels
-	c := &kops.KubeletConfigSpec{}
-	if isMaster {
-		reflectutils.JSONMergeStruct(c, b.Cluster.Spec.MasterKubelet)
-	} else {
-		reflectutils.JSONMergeStruct(c, b.Cluster.Spec.Kubelet)
-	}
+	c := b.NodeupConfig.KubeletConfig
 
 	// check if we are using secure kubelet <-> api settings
 	if b.UseSecureKubelet() {
@@ -446,7 +439,7 @@ func (b *KubeletBuilder) buildKubeletConfigSpec() (*kops.KubeletConfigSpec, erro
 		instanceTypeName, err := metadata.GetMetadata("instance-type")
 		if err != nil {
 			// Otherwise, fall back to the Instance Group spec.
-			instanceTypeName = strings.Split(b.InstanceGroup.Spec.MachineType, ",")[0]
+			instanceTypeName = *b.NodeupConfig.DefaultMachineType
 		}
 
 		region, err := awsup.FindRegion(b.Cluster)
@@ -486,21 +479,10 @@ func (b *KubeletBuilder) buildKubeletConfigSpec() (*kops.KubeletConfigSpec, erro
 
 		// Write back values that could have changed
 		c.MaxPods = &maxPods
-		if b.InstanceGroup.Spec.Kubelet != nil {
-			if b.InstanceGroup.Spec.Kubelet.MaxPods == nil {
-				b.InstanceGroup.Spec.Kubelet.MaxPods = &maxPods
-			}
-		}
-	}
-
-	if b.InstanceGroup.Spec.Kubelet != nil {
-		reflectutils.JSONMergeStruct(c, b.InstanceGroup.Spec.Kubelet)
 	}
 
 	// Use --register-with-taints
 	{
-		c.Taints = append(c.Taints, b.InstanceGroup.Spec.Taints...)
-
 		if len(c.Taints) == 0 && isMaster {
 			// (Even though the value is empty, we still expect <Key>=<Value>:<Effect>)
 			c.Taints = append(c.Taints, nodelabels.RoleLabelMaster16+"=:"+string(v1.TaintEffectNoSchedule))
@@ -538,15 +520,9 @@ func (b *KubeletBuilder) buildKubeletConfigSpec() (*kops.KubeletConfigSpec, erro
 	// For bootstrapping reasons, protokube sets the critical labels for kops-controller to run.
 	if b.Cluster.IsKubernetesGTE("1.16") {
 		c.NodeLabels = nil
-	} else {
-		nodeLabels, err := nodelabels.BuildNodeLabels(b.Cluster, b.InstanceGroup)
-		if err != nil {
-			return nil, err
-		}
-		c.NodeLabels = nodeLabels
 	}
 
-	return c, nil
+	return &c, nil
 }
 
 // buildMasterKubeletKubeconfig builds a kubeconfig for the master kubelet, self-signing the kubelet cert

pkg/apis/nodeup/BUILD.bazel

@@ -7,6 +7,9 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/apis/kops:go_default_library",
+        "//pkg/nodelabels:go_default_library",
+        "//upup/pkg/fi:go_default_library",
         "//util/pkg/architectures:go_default_library",
+        "//util/pkg/reflectutils:go_default_library",
     ],
 )

pkg/apis/nodeup/config.go

@@ -17,8 +17,13 @@ limitations under the License.
 package nodeup
 
 import (
+	"strings"
+
 	"k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/pkg/nodelabels"
+	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/util/pkg/architectures"
+	"k8s.io/kops/util/pkg/reflectutils"
 )
 
 // Config is the configuration for the nodeup binary
@@ -48,9 +53,13 @@ type Config struct {
 	// Manifests for running etcd
 	EtcdManifests []string `json:"etcdManifests,omitempty"`
 
+	// DefaultMachineType is the first-listed instance machine type, used if querying instance metadata fails.
+	DefaultMachineType *string `json:",omitempty"`
 	// StaticManifests describes generic static manifests
 	// Using this allows us to keep complex logic out of nodeup
 	StaticManifests []*StaticManifest `json:"staticManifests,omitempty"`
+	// KubeletConfig defines the kubelet configuration.
+	KubeletConfig kops.KubeletConfigSpec
 	// SysctlParameters will configure kernel parameters using sysctl(8). When
 	// specified, each parameter must follow the form variable=value, the way
 	// it would appear in sysctl.conf.
@@ -78,9 +87,45 @@ type StaticManifest struct {
 }
 
 func NewConfig(cluster *kops.Cluster, instanceGroup *kops.InstanceGroup) *Config {
-	return &Config{
-		InstanceGroupRole: instanceGroup.Spec.Role,
+	role := instanceGroup.Spec.Role
+	isMaster := role == kops.InstanceGroupRoleMaster
+
+	config := Config{
+		InstanceGroupRole: role,
 		SysctlParameters:  instanceGroup.Spec.SysctlParameters,
 		VolumeMounts:      instanceGroup.Spec.VolumeMounts,
 	}
+
+	if isMaster {
+		reflectutils.JSONMergeStruct(&config.KubeletConfig, cluster.Spec.MasterKubelet)
+
+		// A few settings in Kubelet override those in MasterKubelet. I'm not sure why.
+		if cluster.Spec.Kubelet != nil && cluster.Spec.Kubelet.AnonymousAuth != nil && !*cluster.Spec.Kubelet.AnonymousAuth {
+			config.KubeletConfig.AnonymousAuth = fi.Bool(false)
+		}
+	} else {
+		reflectutils.JSONMergeStruct(&config.KubeletConfig, cluster.Spec.Kubelet)
+	}
+
+	if instanceGroup.Spec.Kubelet != nil {
+		useSecureKubelet := config.KubeletConfig.AnonymousAuth != nil && !*config.KubeletConfig.AnonymousAuth
+
+		reflectutils.JSONMergeStruct(&config.KubeletConfig, instanceGroup.Spec.Kubelet)
+
+		if useSecureKubelet {
+			config.KubeletConfig.AnonymousAuth = fi.Bool(false)
+		}
+	}
+
+	// We include the NodeLabels in the userdata even for Kubernetes 1.16 and later so that
+	// rolling update will still replace nodes when they change.
+	config.KubeletConfig.NodeLabels = nodelabels.BuildNodeLabels(cluster, instanceGroup)
+
+	config.KubeletConfig.Taints = append(config.KubeletConfig.Taints, instanceGroup.Spec.Taints...)
+
+	if cluster.Spec.Networking != nil && cluster.Spec.Networking.AmazonVPC != nil {
+		config.DefaultMachineType = fi.String(strings.Split(instanceGroup.Spec.MachineType, ",")[0])
+	}
+
+	return &config
 }

pkg/model/bootstrapscript.go

@@ -281,9 +281,6 @@ func (b *BootstrapScript) Run(c *fi.Context) error {
 
 		"IGSpec": func() (string, error) {
 			spec := make(map[string]interface{})
-			spec["kubelet"] = b.ig.Spec.Kubelet
-			spec["nodeLabels"] = b.ig.Spec.NodeLabels
-			spec["taints"] = b.ig.Spec.Taints
 
 			hooks, err := b.getRelevantHooks(b.ig.Spec.Hooks, b.ig.Spec.Role)
 			if err != nil {

pkg/model/tests/data/bootstrapscript_0.txt

@@ -205,19 +205,21 @@ fileAssets:
 hooks:
 - manifest: 8BN3anFUyDlkVF/JnaJqbwpq8ME= (fingerprint)
   name: apply-to-all.service
-kubelet:
-  kubeconfigPath: /etc/kubernetes/igconfig.txt
-nodeLabels:
-  label2: value2
-  labelname: labelvalue
-taints:
-- key1=value1:NoSchedule
-- key2=value2:NoExecute
 
 __EOF_IG_SPEC
 
 cat > conf/kube_env.yaml << '__EOF_KUBE_ENV'
 InstanceGroupRole: Master
+KubeletConfig:
+  kubeconfigPath: /etc/kubernetes/igconfig.txt
+  nodeLabels:
+    kubernetes.io/role: master
+    label2: value2
+    labelname: labelvalue
+    node-role.kubernetes.io/master: ""
+  taints:
+  - key1=value1:NoSchedule
+  - key2=value2:NoExecute
 
 __EOF_KUBE_ENV
 

pkg/model/tests/data/bootstrapscript_1.txt

@@ -222,19 +222,21 @@ hooks:
   name: disable-update-engine.service
 - manifest: 8BN3anFUyDlkVF/JnaJqbwpq8ME= (fingerprint)
   name: apply-to-all.service
-kubelet:
-  kubeconfigPath: /etc/kubernetes/igconfig.txt
-nodeLabels:
-  label2: value2
-  labelname: labelvalue
-taints:
-- key1=value1:NoSchedule
-- key2=value2:NoExecute
 
 __EOF_IG_SPEC
 
 cat > conf/kube_env.yaml << '__EOF_KUBE_ENV'
 InstanceGroupRole: Master
+KubeletConfig:
+  kubeconfigPath: /etc/kubernetes/igconfig.txt
+  nodeLabels:
+    kubernetes.io/role: master
+    label2: value2
+    labelname: labelvalue
+    node-role.kubernetes.io/master: ""
+  taints:
+  - key1=value1:NoSchedule
+  - key2=value2:NoExecute
 
 __EOF_KUBE_ENV
 

pkg/model/tests/data/bootstrapscript_2.txt

@@ -222,19 +222,21 @@ hooks:
   name: disable-update-engine.service
 - manifest: 8BN3anFUyDlkVF/JnaJqbwpq8ME= (fingerprint)
   name: apply-to-all.service
-kubelet:
-  kubeconfigPath: /etc/kubernetes/igconfig.txt
-nodeLabels:
-  label2: value2
-  labelname: labelvalue
-taints:
-- key1=value1:NoSchedule
-- key2=value2:NoExecute
 
 __EOF_IG_SPEC
 
 cat > conf/kube_env.yaml << '__EOF_KUBE_ENV'
 InstanceGroupRole: Master
+KubeletConfig:
+  kubeconfigPath: /etc/kubernetes/igconfig.txt
+  nodeLabels:
+    kubernetes.io/role: master
+    label2: value2
+    labelname: labelvalue
+    node-role.kubernetes.io/master: ""
+  taints:
+  - key1=value1:NoSchedule
+  - key2=value2:NoExecute
 
 __EOF_KUBE_ENV
 

pkg/model/tests/data/bootstrapscript_3.txt

@@ -190,19 +190,21 @@ fileAssets:
 hooks:
 - manifest: 8BN3anFUyDlkVF/JnaJqbwpq8ME= (fingerprint)
   name: apply-to-all.service
-kubelet:
-  kubeconfigPath: /etc/kubernetes/igconfig.txt
-nodeLabels:
-  label2: value2
-  labelname: labelvalue
-taints:
-- key1=value1:NoSchedule
-- key2=value2:NoExecute
 
 __EOF_IG_SPEC
 
 cat > conf/kube_env.yaml << '__EOF_KUBE_ENV'
 InstanceGroupRole: Node
+KubeletConfig:
+  kubeconfigPath: /etc/kubernetes/igconfig.txt
+  nodeLabels:
+    kubernetes.io/role: node
+    label2: value2
+    labelname: labelvalue
+    node-role.kubernetes.io/node: ""
+  taints:
+  - key1=value1:NoSchedule
+  - key2=value2:NoExecute
 
 __EOF_KUBE_ENV
 

pkg/model/tests/data/bootstrapscript_4.txt

@@ -207,19 +207,21 @@ hooks:
   name: disable-update-engine.service
 - manifest: 8BN3anFUyDlkVF/JnaJqbwpq8ME= (fingerprint)
   name: apply-to-all.service
-kubelet:
-  kubeconfigPath: /etc/kubernetes/igconfig.txt
-nodeLabels:
-  label2: value2
-  labelname: labelvalue
-taints:
-- key1=value1:NoSchedule
-- key2=value2:NoExecute
 
 __EOF_IG_SPEC
 
 cat > conf/kube_env.yaml << '__EOF_KUBE_ENV'
 InstanceGroupRole: Node
+KubeletConfig:
+  kubeconfigPath: /etc/kubernetes/igconfig.txt
+  nodeLabels:
+    kubernetes.io/role: node
+    label2: value2
+    labelname: labelvalue
+    node-role.kubernetes.io/node: ""
+  taints:
+  - key1=value1:NoSchedule
+  - key2=value2:NoExecute
 
 __EOF_KUBE_ENV