kubeadm: Split out markmaster to its own phase

cmd/kubeadm/app/BUILD

@@ -41,6 +41,7 @@ filegroup(
         "//cmd/kubeadm/app/phases/certs:all-srcs",
         "//cmd/kubeadm/app/phases/controlplane:all-srcs",
         "//cmd/kubeadm/app/phases/kubeconfig:all-srcs",
+        "//cmd/kubeadm/app/phases/markmaster:all-srcs",
         "//cmd/kubeadm/app/phases/selfhosting:all-srcs",
         "//cmd/kubeadm/app/phases/token:all-srcs",
         "//cmd/kubeadm/app/preflight:all-srcs",

cmd/kubeadm/app/cmd/BUILD

@@ -32,6 +32,7 @@ go_library(
         "//cmd/kubeadm/app/phases/apiconfig:go_default_library",
         "//cmd/kubeadm/app/phases/controlplane:go_default_library",
         "//cmd/kubeadm/app/phases/kubeconfig:go_default_library",
+        "//cmd/kubeadm/app/phases/markmaster:go_default_library",
         "//cmd/kubeadm/app/phases/selfhosting:go_default_library",
         "//cmd/kubeadm/app/phases/token:go_default_library",
         "//cmd/kubeadm/app/preflight:go_default_library",

cmd/kubeadm/app/cmd/init.go

@@ -37,6 +37,7 @@ import (
 	apiconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/apiconfig"
 	controlplanephase "k8s.io/kubernetes/cmd/kubeadm/app/phases/controlplane"
 	kubeconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubeconfig"
+	markmasterphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/markmaster"
 	selfhostingphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/selfhosting"
 	tokenphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/token"
 	"k8s.io/kubernetes/cmd/kubeadm/app/preflight"
@@ -246,7 +247,7 @@ func (i *Init) Run(out io.Writer) error {
 		return err
 	}
 
-	if err := apiconfigphase.UpdateMasterRoleLabelsAndTaints(client, i.cfg.NodeName); err != nil {
+	if err := markmasterphase.MarkMaster(client, i.cfg.NodeName); err != nil {
 		return err
 	}
 

cmd/kubeadm/app/cmd/phases/BUILD

@@ -13,6 +13,7 @@ go_library(
     srcs = [
         "certs.go",
         "kubeconfig.go",
+        "markmaster.go",
         "phase.go",
         "preflight.go",
         "selfhosting.go",
@@ -26,6 +27,7 @@ go_library(
         "//cmd/kubeadm/app/phases/certs:go_default_library",
         "//cmd/kubeadm/app/phases/certs/pkiutil:go_default_library",
         "//cmd/kubeadm/app/phases/kubeconfig:go_default_library",
+        "//cmd/kubeadm/app/phases/markmaster:go_default_library",
         "//cmd/kubeadm/app/phases/selfhosting:go_default_library",
         "//cmd/kubeadm/app/preflight:go_default_library",
         "//cmd/kubeadm/app/util:go_default_library",

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

+/*
+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 phases
+
+import (
+	"fmt"
+
+	"github.com/spf13/cobra"
+
+	markmasterphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/markmaster"
+	kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
+)
+
+// NewCmdMarkMaster returns the Cobra command for running the mark-master phase
+func NewCmdMarkMaster() *cobra.Command {
+	var kubeConfigFile string
+	cmd := &cobra.Command{
+		Use:     "mark-master <node-name>",
+		Short:   "Create KubeConfig files from given credentials.",
+		Aliases: []string{"markmaster"},
+		RunE: func(_ *cobra.Command, args []string) error {
+			if len(args) < 1 || len(args[0]) == 0 {
+				return fmt.Errorf("missing required argument node-name")
+			}
+			if len(args) > 1 {
+				return fmt.Errorf("too many arguments, only one argument supported: node-name")
+			}
+			client, err := kubeconfigutil.ClientSetFromFile(kubeConfigFile)
+			if err != nil {
+				return err
+			}
+
+			nodeName := args[0]
+			fmt.Printf("[markmaster] Will mark node %s as master by adding a label and a taint\n", nodeName)
+
+			return markmasterphase.MarkMaster(client, nodeName)
+		},
+	}
+
+	cmd.Flags().StringVar(&kubeConfigFile, "kubeconfig", "/etc/kubernetes/admin.conf", "The KubeConfig file to use for talking to the cluster")
+	return cmd
+}

cmd/kubeadm/app/cmd/phases/phase.go

@@ -34,6 +34,7 @@ func NewCmdPhase(out io.Writer) *cobra.Command {
 	cmd.AddCommand(NewCmdCerts())
 	cmd.AddCommand(NewCmdPreFlight())
 	cmd.AddCommand(NewCmdSelfhosting())
+	cmd.AddCommand(NewCmdMarkMaster())
 
 	return cmd
 }

cmd/kubeadm/app/constants/constants.go

@@ -79,6 +79,8 @@ const (
 	APICallRetryInterval = 500 * time.Millisecond
 	// DiscoveryRetryInterval specifies how long kubeadm should wait before retrying to connect to the master when doing discovery
 	DiscoveryRetryInterval = 5 * time.Second
+	// MarkMasterTimeout specifies how long kubeadm should wait for applying the label and taint on the master before timing out
+	MarkMasterTimeout = 2 * time.Minute
 
 	// Minimum amount of nodes the Service subnet should allow.
 	// We need at least ten, because the DNS service is always at the tenth cluster clusterIP
@@ -98,6 +100,12 @@ const (
 
 var (
 
+	// MasterTaint is the taint to apply on the PodSpec for being able to run that Pod on the master
+	MasterTaint = v1.Taint{
+		Key:    LabelNodeRoleMaster,
+		Effect: v1.TaintEffectNoSchedule,
+	}
+
 	// MasterToleration is the toleration to apply on the PodSpec for being able to run that Pod on the master
 	MasterToleration = v1.Toleration{
 		Key:    LabelNodeRoleMaster,

cmd/kubeadm/app/phases/apiconfig/BUILD

@@ -10,49 +10,31 @@ load(
 
 go_test(
     name = "go_default_test",
-    srcs = [
-        "clusterroles_test.go",
-        "setupmaster_test.go",
-    ],
+    srcs = ["clusterroles_test.go"],
     library = ":go_default_library",
     tags = ["automanaged"],
     deps = [
-        "//cmd/kubeadm/app/constants:go_default_library",
         "//pkg/api:go_default_library",
-        "//pkg/kubelet/apis:go_default_library",
-        "//pkg/util/node:go_default_library",
-        "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
-        "//vendor/k8s.io/client-go/kubernetes:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes/fake:go_default_library",
-        "//vendor/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
-        "//vendor/k8s.io/client-go/rest:go_default_library",
         "//vendor/k8s.io/client-go/testing:go_default_library",
     ],
 )
 
 go_library(
     name = "go_default_library",
-    srcs = [
-        "clusterroles.go",
-        "setupmaster.go",
-    ],
+    srcs = ["clusterroles.go"],
     tags = ["automanaged"],
     deps = [
         "//cmd/kubeadm/app/constants:go_default_library",
         "//pkg/apis/rbac/v1beta1:go_default_library",
         "//pkg/bootstrap/api:go_default_library",
-        "//pkg/kubelet/apis:go_default_library",
         "//pkg/util/version:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/api/rbac/v1beta1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes:go_default_library",
     ],
 )

cmd/kubeadm/app/phases/markmaster/BUILD

+package(default_visibility = ["//visibility:public"])
+
+licenses(["notice"])
+
+load(
+    "@io_bazel_rules_go//go:def.bzl",
+    "go_library",
+    "go_test",
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["markmaster_test.go"],
+    library = ":go_default_library",
+    tags = ["automanaged"],
+    deps = [
+        "//cmd/kubeadm/app/constants:go_default_library",
+        "//pkg/kubelet/apis:go_default_library",
+        "//pkg/util/node:go_default_library",
+        "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/client-go/kubernetes:go_default_library",
+        "//vendor/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
+        "//vendor/k8s.io/client-go/rest:go_default_library",
+    ],
+)
+
+go_library(
+    name = "go_default_library",
+    srcs = ["markmaster.go"],
+    tags = ["automanaged"],
+    deps = [
+        "//cmd/kubeadm/app/constants:go_default_library",
+        "//pkg/kubelet/apis:go_default_library",
+        "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
+        "//vendor/k8s.io/client-go/kubernetes:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+)

cmd/kubeadm/app/phases/apiconfig/setupmaster.go → cmd/kubeadm/app/phases/markmaster/markmaster.go

@@ -14,12 +14,11 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package apiconfig
+package markmaster
 
 import (
 	"encoding/json"
 	"fmt"
-	"time"
 
 	"k8s.io/api/core/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
@@ -32,55 +31,58 @@ import (
 	kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
 )
 
-const apiCallRetryInterval = 500 * time.Millisecond
+// MarkMaster taints the master and sets the master label
+func MarkMaster(client clientset.Interface, masterName string) error {
 
-// TODO: Can we think of any unit tests here? Or should this code just be covered through integration/e2e tests?
-
-func attemptToUpdateMasterRoleLabelsAndTaints(client *clientset.Clientset, nodeName string) error {
-	var n *v1.Node
-
-	// Wait for current node registration
-	wait.PollInfinite(kubeadmconstants.APICallRetryInterval, func() (bool, error) {
-		var err error
-		if n, err = client.Nodes().Get(nodeName, metav1.GetOptions{}); err != nil {
+	// 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) {
+		// First get the node object
+		n, err := client.CoreV1().Nodes().Get(masterName, metav1.GetOptions{})
+		if err != nil {
 			return false, nil
 		}
+
 		// The node may appear to have no labels at first,
 		// so we wait for it to get hostname label.
-		_, found := n.ObjectMeta.Labels[kubeletapis.LabelHostname]
-		return found, nil
-	})
+		if _, found := n.ObjectMeta.Labels[kubeletapis.LabelHostname]; !found {
+			return false, nil
+		}
 
 		oldData, err := json.Marshal(n)
 		if err != nil {
-		return err
+			return false, err
 		}
 
-	// The master node is tainted and labelled accordingly
-	n.ObjectMeta.Labels[kubeadmconstants.LabelNodeRoleMaster] = ""
-	addTaintIfNotExists(n, v1.Taint{Key: kubeadmconstants.LabelNodeRoleMaster, Value: "", Effect: "NoSchedule"})
+		// The master node should be tainted and labelled accordingly
+		markMasterNode(n)
 
 		newData, err := json.Marshal(n)
 		if err != nil {
-		return err
+			return false, err
 		}
 
 		patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, v1.Node{})
 		if err != nil {
-		return err
+			return false, err
 		}
 
-	if _, err := client.Nodes().Patch(n.Name, types.StrategicMergePatchType, patchBytes); err != nil {
+		if _, err := client.CoreV1().Nodes().Patch(n.Name, types.StrategicMergePatchType, patchBytes); err != nil {
 			if apierrs.IsConflict(err) {
-			fmt.Println("[apiclient] Temporarily unable to update master node metadata due to conflict (will retry)")
-			time.Sleep(apiCallRetryInterval)
-			attemptToUpdateMasterRoleLabelsAndTaints(client, nodeName)
-		} else {
-			return err
+				fmt.Println("[markmaster] Temporarily unable to update master node metadata due to conflict (will retry)")
+				return false, nil
 			}
+			return false, err
 		}
 
-	return nil
+		fmt.Printf("[markmaster] Master %s tainted and labelled with key/value: %s=%q\n", masterName, kubeadmconstants.LabelNodeRoleMaster, "")
+
+		return true, nil
+	})
+}
+
+func markMasterNode(n *v1.Node) {
+	n.ObjectMeta.Labels[kubeadmconstants.LabelNodeRoleMaster] = ""
+	addTaintIfNotExists(n, kubeadmconstants.MasterTaint)
 }
 
 func addTaintIfNotExists(n *v1.Node, t v1.Taint) {
@@ -92,13 +94,3 @@ func addTaintIfNotExists(n *v1.Node, t v1.Taint) {
 
 	n.Spec.Taints = append(n.Spec.Taints, t)
 }
-
-// UpdateMasterRoleLabelsAndTaints taints the master and sets the master label
-func UpdateMasterRoleLabelsAndTaints(client *clientset.Clientset, nodeName string) error {
-	// TODO: Use iterate instead of recursion
-	err := attemptToUpdateMasterRoleLabelsAndTaints(client, nodeName)
-	if err != nil {
-		return fmt.Errorf("failed to update master node - [%v]", err)
-	}
-	return nil
-}

cmd/kubeadm/app/phases/apiconfig/setupmaster_test.go → cmd/kubeadm/app/phases/markmaster/markmaster_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package apiconfig
+package markmaster
 
 import (
 	"bytes"
@@ -24,7 +24,7 @@ import (
 	"net/http/httptest"
 	"testing"
 
-	apiv1 "k8s.io/api/core/v1"
+	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientset "k8s.io/client-go/kubernetes"
 	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -34,11 +34,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/node"
 )
 
-const masterLabel = kubeadmconstants.LabelNodeRoleMaster
-
-var masterTaint = &apiv1.Taint{Key: kubeadmconstants.LabelNodeRoleMaster, Value: "", Effect: "NoSchedule"}
-
-func TestUpdateMasterRoleLabelsAndTaints(t *testing.T) {
+func TestMarkMaster(t *testing.T) {
 	// Note: this test takes advantage of the deterministic marshalling of
 	// JSON provided by strategicpatch so that "expectedPatch" can use a
 	// string equality test instead of a logical JSON equality test. That
@@ -47,7 +43,7 @@ func TestUpdateMasterRoleLabelsAndTaints(t *testing.T) {
 	tests := []struct {
 		name          string
 		existingLabel string
-		existingTaint *apiv1.Taint
+		existingTaint *v1.Taint
 		expectedPatch string
 	}{
 		{
@@ -59,26 +55,26 @@ func TestUpdateMasterRoleLabelsAndTaints(t *testing.T) {
 		{
 			"master label missing",
 			"",
-			masterTaint,
+			&kubeadmconstants.MasterTaint,
 			"{\"metadata\":{\"labels\":{\"node-role.kubernetes.io/master\":\"\"}}}",
 		},
 		{
 			"master taint missing",
-			masterLabel,
+			kubeadmconstants.LabelNodeRoleMaster,
 			nil,
 			"{\"spec\":{\"taints\":[{\"effect\":\"NoSchedule\",\"key\":\"node-role.kubernetes.io/master\",\"timeAdded\":null}]}}",
 		},
 		{
 			"nothing missing",
-			masterLabel,
-			masterTaint,
+			kubeadmconstants.LabelNodeRoleMaster,
+			&kubeadmconstants.MasterTaint,
 			"{}",
 		},
 	}
 
 	for _, tc := range tests {
 		hostname := node.GetHostname("")
-		masterNode := &apiv1.Node{
+		masterNode := &v1.Node{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: hostname,
 				Labels: map[string]string{
@@ -97,7 +93,7 @@ func TestUpdateMasterRoleLabelsAndTaints(t *testing.T) {
 
 		jsonNode, err := json.Marshal(masterNode)
 		if err != nil {
-			t.Fatalf("UpdateMasterRoleLabelsAndTaints(%s): unexpected encoding error: %v", tc.name, err)
+			t.Fatalf("MarkMaster(%s): unexpected encoding error: %v", tc.name, err)
 		}
 
 		var patchRequest string
@@ -105,7 +101,7 @@ func TestUpdateMasterRoleLabelsAndTaints(t *testing.T) {
 			w.Header().Set("Content-Type", "application/json")
 
 			if req.URL.Path != "/api/v1/nodes/"+hostname {
-				t.Errorf("UpdateMasterRoleLabelsAndTaints(%s): request for unexpected HTTP resource: %v", tc.name, req.URL.Path)
+				t.Errorf("MarkMaster(%s): request for unexpected HTTP resource: %v", tc.name, req.URL.Path)
 				w.WriteHeader(http.StatusNotFound)
 				return
 			}
@@ -115,7 +111,7 @@ func TestUpdateMasterRoleLabelsAndTaints(t *testing.T) {
 			case "PATCH":
 				patchRequest = toString(req.Body)
 			default:
-				t.Errorf("UpdateMasterRoleLabelsAndTaints(%s): request for unexpected HTTP verb: %v", tc.name, req.Method)
+				t.Errorf("MarkMaster(%s): request for unexpected HTTP verb: %v", tc.name, req.Method)
 				w.WriteHeader(http.StatusNotFound)
 				return
 			}
@@ -127,16 +123,16 @@ func TestUpdateMasterRoleLabelsAndTaints(t *testing.T) {
 
 		cs, err := clientsetFromTestServer(s)
 		if err != nil {
-			t.Fatalf("UpdateMasterRoleLabelsAndTaints(%s): unexpected error building clientset: %v", tc.name, err)
+			t.Fatalf("MarkMaster(%s): unexpected error building clientset: %v", tc.name, err)
 		}
 
-		err = UpdateMasterRoleLabelsAndTaints(cs, hostname)
+		err = MarkMaster(cs, hostname)
 		if err != nil {
-			t.Errorf("UpdateMasterRoleLabelsAndTaints(%s) returned unexpected error: %v", tc.name, err)
+			t.Errorf("MarkMaster(%s) returned unexpected error: %v", tc.name, err)
 		}
 
 		if tc.expectedPatch != patchRequest {
-			t.Errorf("UpdateMasterRoleLabelsAndTaints(%s) wanted patch %v, got %v", tc.name, tc.expectedPatch, patchRequest)
+			t.Errorf("MarkMaster(%s) wanted patch %v, got %v", tc.name, tc.expectedPatch, patchRequest)
 		}
 	}
 }