Merge pull request #55479 from ijc/kubeadm-optional-master-taint

Automatic merge from submit-queue (batch tested with PRs 59767, 56454, 59237, 59730, 55479). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: add configuration option to not taint master

**What this PR does / why we need it**:

Although tainting the master is normally a good and proper thing to do in some situations (docker for mac in our case, but I suppose minikube and such as well) having a single host configuration is desirable.

In linuxkit we have a [workaround](443e47c408/projects/kubernetes/kubernetes/kubeadm-init.sh (L19...L22)) to remove the taint after initialisation. With the change here we could simply populate `/etc/kubeadm/kubeadm.yaml` with `noTaintMaster: true` instead and have it never be tainted in the first place.

I have only added this to the config file and not to the CLI since AIUI the latter is somewhat deprecated.

The code also arranges to _remove_ an existing taint if it is unwanted. I'm unsure if this behaviour is correct or desirable, I think a reasonable argument could be made for leaving an existing taint in place too.

Signed-off-by: Ian Campbell <ijc@docker.com>

**Release note**:

Since the requirement for this option is rather niche and not best practice in the majority of cases I'm not sure if it warrants mentioning in the release notes? If it were then perhaps

```release-note
`kubeadm init` can now omit the tainting of the master node if configured to do so in `kubeadm.yaml`.
```

cmd/kubeadm/app/apis/kubeadm/types.go

@@ -51,6 +51,10 @@ type MasterConfiguration struct {
 	// If not specified, defaults to Node and RBAC, meaning both the node
 	// authorizer and RBAC are enabled.
 	AuthorizationModes []string
+	// NoTaintMaster will, if set, suppress the tainting of the
+	// master node allowing workloads to be run on it (e.g. in
+	// single node configurations).
+	NoTaintMaster bool
 
 	// Mark the controller and api server pods as privileged as some cloud
 	// controllers like openstack need escalated privileges under some conditions

cmd/kubeadm/app/apis/kubeadm/v1alpha1/types.go

@@ -51,6 +51,10 @@ type MasterConfiguration struct {
 	// If not specified, defaults to Node and RBAC, meaning both the node
 	// authorizer and RBAC are enabled.
 	AuthorizationModes []string `json:"authorizationModes,omitempty"`
+	// NoTaintMaster will, if set, suppress the tainting of the
+	// master node allowing workloads to be run on it (e.g. in
+	// single node configurations).
+	NoTaintMaster bool `json:"noTaintMaster,omitempty"`
 
 	// Mark the controller and api server pods as privileged as some cloud
 	// controllers like openstack need escalated privileges under some conditions

cmd/kubeadm/app/apis/kubeadm/v1alpha1/zz_generated.conversion.go

@@ -229,6 +229,7 @@ func autoConvert_v1alpha1_MasterConfiguration_To_kubeadm_MasterConfiguration(in
 	out.CloudProvider = in.CloudProvider
 	out.NodeName = in.NodeName
 	out.AuthorizationModes = *(*[]string)(unsafe.Pointer(&in.AuthorizationModes))
+	out.NoTaintMaster = in.NoTaintMaster
 	out.PrivilegedPods = in.PrivilegedPods
 	out.Token = in.Token
 	out.TokenTTL = (*v1.Duration)(unsafe.Pointer(in.TokenTTL))
@@ -275,6 +276,7 @@ func autoConvert_kubeadm_MasterConfiguration_To_v1alpha1_MasterConfiguration(in
 	out.CloudProvider = in.CloudProvider
 	out.NodeName = in.NodeName
 	out.AuthorizationModes = *(*[]string)(unsafe.Pointer(&in.AuthorizationModes))
+	out.NoTaintMaster = in.NoTaintMaster
 	out.PrivilegedPods = in.PrivilegedPods
 	out.Token = in.Token
 	out.TokenTTL = (*v1.Duration)(unsafe.Pointer(in.TokenTTL))

cmd/kubeadm/app/cmd/init.go

@@ -408,7 +408,7 @@ func (i *Init) Run(out io.Writer) error {
 	}
 
 	// PHASE 4: Mark the master with the right label/taint
-	if err := markmasterphase.MarkMaster(client, i.cfg.NodeName); err != nil {
+	if err := markmasterphase.MarkMaster(client, i.cfg.NodeName, !i.cfg.NoTaintMaster); err != nil {
 		return fmt.Errorf("error marking master: %v", err)
 	}
 

cmd/kubeadm/app/cmd/phases/markmaster.go

@@ -75,7 +75,7 @@ func NewCmdMarkMaster() *cobra.Command {
 			client, err := kubeconfigutil.ClientSetFromFile(kubeConfigFile)
 			kubeadmutil.CheckErr(err)
 
-			err = markmasterphase.MarkMaster(client, internalcfg.NodeName)
+			err = markmasterphase.MarkMaster(client, internalcfg.NodeName, !internalcfg.NoTaintMaster)
 			kubeadmutil.CheckErr(err)
 		},
 	}

cmd/kubeadm/app/phases/markmaster/markmaster.go

@@ -32,9 +32,13 @@ import (
 )
 
 // MarkMaster taints the master and sets the master label
-func MarkMaster(client clientset.Interface, masterName string) error {
+func MarkMaster(client clientset.Interface, masterName string, taint bool) error {
 
+	if taint {
 		fmt.Printf("[markmaster] Will mark node %s as master by adding a label and a taint\n", masterName)
+	} else {
+		fmt.Printf("[markmaster] Will mark node %s as master by adding a label\n", masterName)
+	}
 
 	// Loop on every falsy return. Return with an error if raised. Exit successfully if true is returned.
 	return wait.Poll(kubeadmconstants.APICallRetryInterval, kubeadmconstants.MarkMasterTimeout, func() (bool, error) {
@@ -56,7 +60,7 @@ func MarkMaster(client clientset.Interface, masterName string) error {
 		}
 
 		// The master node should be tainted and labelled accordingly
-		markMasterNode(n)
+		markMasterNode(n, taint)
 
 		newData, err := json.Marshal(n)
 		if err != nil {
@@ -76,15 +80,23 @@ func MarkMaster(client clientset.Interface, masterName string) error {
 			return false, err
 		}
 
+		if taint {
 			fmt.Printf("[markmaster] Master %s tainted and labelled with key/value: %s=%q\n", masterName, kubeadmconstants.LabelNodeRoleMaster, "")
+		} else {
+			fmt.Printf("[markmaster] Master %s labelled with key/value: %s=%q\n", masterName, kubeadmconstants.LabelNodeRoleMaster, "")
+		}
 
 		return true, nil
 	})
 }
 
-func markMasterNode(n *v1.Node) {
+func markMasterNode(n *v1.Node, taint bool) {
 	n.ObjectMeta.Labels[kubeadmconstants.LabelNodeRoleMaster] = ""
+	if taint {
 		addTaintIfNotExists(n, kubeadmconstants.MasterTaint)
+	} else {
+		delTaintIfExists(n, kubeadmconstants.MasterTaint)
+	}
 }
 
 func addTaintIfNotExists(n *v1.Node, t v1.Taint) {
@@ -96,3 +108,14 @@ func addTaintIfNotExists(n *v1.Node, t v1.Taint) {
 
 	n.Spec.Taints = append(n.Spec.Taints, t)
 }
+
+func delTaintIfExists(n *v1.Node, t v1.Taint) {
+	var taints []v1.Taint
+	for _, taint := range n.Spec.Taints {
+		if taint == t {
+			continue
+		}
+		taints = append(taints, t)
+	}
+	n.Spec.Taints = taints
+}

cmd/kubeadm/app/phases/markmaster/markmaster_test.go

@@ -43,32 +43,51 @@ func TestMarkMaster(t *testing.T) {
 		name          string
 		existingLabel string
 		existingTaint *v1.Taint
+		wantTaint     bool
 		expectedPatch string
 	}{
 		{
 			"master label and taint missing",
 			"",
 			nil,
+			true,
 			"{\"metadata\":{\"labels\":{\"node-role.kubernetes.io/master\":\"\"}},\"spec\":{\"taints\":[{\"effect\":\"NoSchedule\",\"key\":\"node-role.kubernetes.io/master\"}]}}",
 		},
+		{
+			"master label and taint missing but taint not wanted",
+			"",
+			nil,
+			false,
+			"{\"metadata\":{\"labels\":{\"node-role.kubernetes.io/master\":\"\"}}}",
+		},
 		{
 			"master label missing",
 			"",
 			&kubeadmconstants.MasterTaint,
+			true,
 			"{\"metadata\":{\"labels\":{\"node-role.kubernetes.io/master\":\"\"}}}",
 		},
 		{
 			"master taint missing",
 			kubeadmconstants.LabelNodeRoleMaster,
 			nil,
+			true,
 			"{\"spec\":{\"taints\":[{\"effect\":\"NoSchedule\",\"key\":\"node-role.kubernetes.io/master\"}]}}",
 		},
 		{
 			"nothing missing",
 			kubeadmconstants.LabelNodeRoleMaster,
 			&kubeadmconstants.MasterTaint,
+			true,
 			"{}",
 		},
+		{
+			"nothing missing but taint unwanted",
+			kubeadmconstants.LabelNodeRoleMaster,
+			&kubeadmconstants.MasterTaint,
+			false,
+			"{\"spec\":{\"taints\":null}}",
+		},
 	}
 
 	for _, tc := range tests {
@@ -125,7 +144,7 @@ func TestMarkMaster(t *testing.T) {
 			t.Fatalf("MarkMaster(%s): unexpected error building clientset: %v", tc.name, err)
 		}
 
-		err = MarkMaster(cs, hostname)
+		err = MarkMaster(cs, hostname, tc.wantTaint)
 		if err != nil {
 			t.Errorf("MarkMaster(%s) returned unexpected error: %v", tc.name, err)
 		}