Merge pull request #46819 from pipejakob/idem-config

Automatic merge from submit-queue (batch tested with PRs 46787, 46876, 46621, 46907, 46819)

kubeadm: Only create bootstrap configmap if not exists.

**What this PR does / why we need it**:
The fact that this method was not idempotent was breaking kubeadm upgrades.

https://github.com/kubernetes/kubeadm/issues/278

**Release note**:

```release-note
NONE
```

cmd/kubeadm/app/cmd/init.go

@@ -247,7 +247,7 @@ func (i *Init) Run(out io.Writer) error {
 		return err
 	}
 
-	if err := tokenphase.CreateBootstrapConfigMap(adminKubeConfigPath); err != nil {
+	if err := tokenphase.CreateBootstrapConfigMapIfNotExists(client, adminKubeConfigPath); err != nil {
 		return err
 	}
 

cmd/kubeadm/app/phases/token/BUILD

@@ -13,7 +13,14 @@ go_test(
     srcs = ["bootstrap_test.go"],
     library = ":go_default_library",
     tags = ["automanaged"],
-    deps = ["//cmd/kubeadm/app/apis/kubeadm:go_default_library"],
+    deps = [
+        "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
+        "//pkg/api:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//vendor/k8s.io/client-go/kubernetes/fake:go_default_library",
+        "//vendor/k8s.io/client-go/testing:go_default_library",
+    ],
 )
 
 go_library(
@@ -21,7 +28,6 @@ go_library(
     srcs = ["bootstrap.go"],
     tags = ["automanaged"],
     deps = [
-        "//cmd/kubeadm/app/util/kubeconfig:go_default_library",
         "//cmd/kubeadm/app/util/token:go_default_library",
         "//pkg/bootstrap/api:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",

cmd/kubeadm/app/phases/token/bootstrap.go

@@ -26,7 +26,6 @@ import (
 	"k8s.io/client-go/pkg/api/v1"
 	"k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
-	kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
 	tokenutil "k8s.io/kubernetes/cmd/kubeadm/app/util/token"
 	bootstrapapi "k8s.io/kubernetes/pkg/bootstrap/api"
 )
@@ -87,16 +86,12 @@ func UpdateOrCreateToken(client *clientset.Clientset, token string, failIfExists
 	)
 }
 
-// CreateBootstrapConfigMap creates the public cluster-info ConfigMap
+// CreateBootstrapConfigMapIfNotExists creates the public cluster-info ConfigMap (if it doesn't already exist)
-func CreateBootstrapConfigMap(file string) error {
+func CreateBootstrapConfigMapIfNotExists(client clientset.Interface, file string) error {
 	adminConfig, err := clientcmd.LoadFromFile(file)
 	if err != nil {
 		return fmt.Errorf("failed to load admin kubeconfig [%v]", err)
 	}
-	client, err := kubeconfigutil.KubeConfigToClientSet(adminConfig)
-	if err != nil {
-		return err
-	}
 
 	adminCluster := adminConfig.Contexts[adminConfig.CurrentContext].Cluster
 	// Copy the cluster from admin.conf to the bootstrap kubeconfig, contains the CA cert and the server URL
@@ -118,6 +113,9 @@ func CreateBootstrapConfigMap(file string) error {
 	}
 
 	if _, err := client.CoreV1().ConfigMaps(metav1.NamespacePublic).Create(&bootstrapConfigMap); err != nil {
+		if apierrors.IsAlreadyExists(err) {
+			return nil
+		}
 		return err
 	}
 	return nil

cmd/kubeadm/app/phases/token/bootstrap_test.go

@@ -18,12 +18,35 @@ package token
 
 import (
 	"bytes"
+	"io/ioutil"
+	"os"
 	"testing"
 	"time"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	clientsetfake "k8s.io/client-go/kubernetes/fake"
+	core "k8s.io/client-go/testing"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	"k8s.io/kubernetes/pkg/api"
 )
 
+const testConfig = `apiVersion: v1
+clusters:
+- cluster:
+    server: https://10.128.0.6:6443
+  name: kubernetes
+contexts:
+- context:
+    cluster: kubernetes
+    user: kubernetes-admin
+  name: kubernetes-admin@kubernetes
+current-context: kubernetes-admin@kubernetes
+kind: Config
+preferences: {}
+users:
+- name: kubernetes-admin`
+
 func TestEncodeTokenSecretData(t *testing.T) {
 	var tests = []struct {
 		token *kubeadmapi.TokenDiscovery
@@ -57,3 +80,51 @@ func TestEncodeTokenSecretData(t *testing.T) {
 		}
 	}
 }
+
+func TestCreateBootstrapConfigMapIfNotExists(t *testing.T) {
+	tests := []struct {
+		name      string
+		createErr error
+		expectErr bool
+	}{
+		{
+			"successful case should have no error",
+			nil,
+			false,
+		},
+		{
+			"duplicate creation should have no error",
+			apierrors.NewAlreadyExists(api.Resource("configmaps"), "test"),
+			false,
+		},
+		{
+			"unexpected error should be returned",
+			apierrors.NewUnauthorized("go away!"),
+			true,
+		},
+	}
+
+	file, err := ioutil.TempFile("", "")
+	if err != nil {
+		t.Fatalf("could not create tempfile: %v", err)
+	}
+	defer os.Remove(file.Name())
+
+	file.Write([]byte(testConfig))
+
+	for _, tc := range tests {
+		client := clientsetfake.NewSimpleClientset()
+		if tc.createErr != nil {
+			client.PrependReactor("create", "configmaps", func(action core.Action) (bool, runtime.Object, error) {
+				return true, nil, tc.createErr
+			})
+		}
+
+		err = CreateBootstrapConfigMapIfNotExists(client, file.Name())
+		if tc.expectErr && err == nil {
+			t.Errorf("CreateBootstrapConfigMapIfNotExists(%s) wanted error, got nil", tc.name)
+		} else if !tc.expectErr && err != nil {
+			t.Errorf("CreateBootstrapConfigMapIfNotExists(%s) returned unexpected error: %v", tc.name, err)
+		}
+	}
+}