From 20c45d08fbac9587641cf3d3f0e2664307365c66 Mon Sep 17 00:00:00 2001 From: Jacob Beacham Date: Wed, 31 May 2017 17:53:42 -0700 Subject: [PATCH] kubeadm: don't duplicate master taint if it already exists. This helps enable a graceful upgrade/downgrade process between 1.6.x and 1.7.x kubeadm clusters (although no guarantees outside of that range) by doing: $ kubeadm init --kubernetes-version --skip-preflight-checks Without this change, the command fails with an error that the node taint is duplicated. This is part of https://github.com/kubernetes/kubeadm/issues/278 --- cmd/kubeadm/app/phases/apiconfig/BUILD | 18 ++ .../app/phases/apiconfig/setupmaster.go | 12 +- .../app/phases/apiconfig/setupmaster_test.go | 157 ++++++++++++++++++ 3 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 cmd/kubeadm/app/phases/apiconfig/setupmaster_test.go diff --git a/cmd/kubeadm/app/phases/apiconfig/BUILD b/cmd/kubeadm/app/phases/apiconfig/BUILD index 215f3f838dd6f..459d2823ae2fe 100644 --- a/cmd/kubeadm/app/phases/apiconfig/BUILD +++ b/cmd/kubeadm/app/phases/apiconfig/BUILD @@ -5,6 +5,7 @@ licenses(["notice"]) load( "@io_bazel_rules_go//go:def.bzl", "go_library", + "go_test", ) go_library( @@ -42,3 +43,20 @@ filegroup( srcs = [":package-srcs"], tags = ["automanaged"], ) + +go_test( + name = "go_default_test", + srcs = ["setupmaster_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/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/pkg/api/v1:go_default_library", + "//vendor/k8s.io/client-go/rest:go_default_library", + ], +) diff --git a/cmd/kubeadm/app/phases/apiconfig/setupmaster.go b/cmd/kubeadm/app/phases/apiconfig/setupmaster.go index b38ef4a06386c..0b0bd0b94ea2d 100644 --- a/cmd/kubeadm/app/phases/apiconfig/setupmaster.go +++ b/cmd/kubeadm/app/phases/apiconfig/setupmaster.go @@ -59,7 +59,7 @@ func attemptToUpdateMasterRoleLabelsAndTaints(client *clientset.Clientset) error // The master node is tainted and labelled accordingly n.ObjectMeta.Labels[kubeadmconstants.LabelNodeRoleMaster] = "" - n.Spec.Taints = append(n.Spec.Taints, v1.Taint{Key: kubeadmconstants.LabelNodeRoleMaster, Value: "", Effect: "NoSchedule"}) + addTaintIfNotExists(n, v1.Taint{Key: kubeadmconstants.LabelNodeRoleMaster, Value: "", Effect: "NoSchedule"}) newData, err := json.Marshal(n) if err != nil { @@ -84,6 +84,16 @@ func attemptToUpdateMasterRoleLabelsAndTaints(client *clientset.Clientset) error return nil } +func addTaintIfNotExists(n *v1.Node, t v1.Taint) { + for _, taint := range n.Spec.Taints { + if taint == t { + return + } + } + + n.Spec.Taints = append(n.Spec.Taints, t) +} + // UpdateMasterRoleLabelsAndTaints taints the master and sets the master label func UpdateMasterRoleLabelsAndTaints(client *clientset.Clientset) error { // TODO: Use iterate instead of recursion diff --git a/cmd/kubeadm/app/phases/apiconfig/setupmaster_test.go b/cmd/kubeadm/app/phases/apiconfig/setupmaster_test.go new file mode 100644 index 0000000000000..5e25a71698600 --- /dev/null +++ b/cmd/kubeadm/app/phases/apiconfig/setupmaster_test.go @@ -0,0 +1,157 @@ +/* +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 apiconfig + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clientset "k8s.io/client-go/kubernetes" + corev1 "k8s.io/client-go/kubernetes/typed/core/v1" + apiv1 "k8s.io/client-go/pkg/api/v1" + restclient "k8s.io/client-go/rest" + kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" + kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" + "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) { + // 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 + // will need to change if strategicpatch's behavior changes in the + // future. + tests := []struct { + name string + existingLabel string + existingTaint *apiv1.Taint + expectedPatch string + }{ + { + "master label and taint missing", + "", + nil, + "{\"metadata\":{\"labels\":{\"node-role.kubernetes.io/master\":\"\"}},\"spec\":{\"taints\":[{\"effect\":\"NoSchedule\",\"key\":\"node-role.kubernetes.io/master\",\"timeAdded\":null}]}}", + }, + { + "master label missing", + "", + masterTaint, + "{\"metadata\":{\"labels\":{\"node-role.kubernetes.io/master\":\"\"}}}", + }, + { + "master taint missing", + masterLabel, + nil, + "{\"spec\":{\"taints\":[{\"effect\":\"NoSchedule\",\"key\":\"node-role.kubernetes.io/master\",\"timeAdded\":null}]}}", + }, + { + "nothing missing", + masterLabel, + masterTaint, + "{}", + }, + } + + for _, tc := range tests { + hostname := node.GetHostname("") + masterNode := &apiv1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: hostname, + Labels: map[string]string{ + kubeletapis.LabelHostname: hostname, + }, + }, + } + + if tc.existingLabel != "" { + masterNode.ObjectMeta.Labels[tc.existingLabel] = "" + } + + if tc.existingTaint != nil { + masterNode.Spec.Taints = append(masterNode.Spec.Taints, *tc.existingTaint) + } + + jsonNode, err := json.Marshal(masterNode) + if err != nil { + t.Fatalf("UpdateMasterRoleLabelsAndTaints(%s): unexpected encoding error: %v", tc.name, err) + } + + var patchRequest string + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + 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) + w.WriteHeader(http.StatusNotFound) + return + } + + switch req.Method { + case "GET": + case "PATCH": + patchRequest = toString(req.Body) + default: + t.Errorf("UpdateMasterRoleLabelsAndTaints(%s): request for unexpected HTTP verb: %v", tc.name, req.Method) + w.WriteHeader(http.StatusNotFound) + return + } + + w.WriteHeader(http.StatusOK) + w.Write(jsonNode) + })) + defer s.Close() + + cs, err := clientsetFromTestServer(s) + if err != nil { + t.Fatalf("UpdateMasterRoleLabelsAndTaints(%s): unexpected error building clientset: %v", tc.name, err) + } + + err = UpdateMasterRoleLabelsAndTaints(cs) + if err != nil { + t.Errorf("UpdateMasterRoleLabelsAndTaints(%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) + } + } +} + +func clientsetFromTestServer(s *httptest.Server) (*clientset.Clientset, error) { + rc := &restclient.Config{Host: s.URL} + c, err := corev1.NewForConfig(rc) + if err != nil { + return nil, err + } + return &clientset.Clientset{CoreV1Client: c}, nil +} + +func toString(r io.Reader) string { + buf := new(bytes.Buffer) + buf.ReadFrom(r) + return buf.String() +}