Merge pull request #46875 from pipejakob/idem-rbac

Automatic merge from submit-queue (batch tested with PRs 46897, 46899, 46864, 46854, 46875)

kubeadm: Idempotent service account creation.

**What this PR does / why we need it**:
During `kubeadm init`, ignore errors for duplicates when creating service accounts.

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

Fixes: https://github.com/kubernetes/kubeadm/issues/288

**Release note**:

```release-note
NONE
```

cmd/kubeadm/app/phases/apiconfig/BUILD

@@ -46,17 +46,25 @@ filegroup(
 
 go_test(
     name = "go_default_test",
-    srcs = ["setupmaster_test.go"],
+    srcs = [
+        "clusterroles_test.go",
+        "setupmaster_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/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/pkg/api/v1:go_default_library",
         "//vendor/k8s.io/client-go/rest:go_default_library",
+        "//vendor/k8s.io/client-go/testing:go_default_library",
     ],
 )

cmd/kubeadm/app/phases/apiconfig/clusterroles.go

@@ -19,6 +19,7 @@ package apiconfig
 import (
 	"fmt"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/pkg/api/v1"
@@ -45,8 +46,8 @@ const (
 
 // TODO: Are there any unit tests that could be made for this file other than duplicating all values and logic in a separate file?
 
-// CreateServiceAccounts creates the necessary serviceaccounts that kubeadm uses/might use.
+// CreateServiceAccounts creates the necessary serviceaccounts that kubeadm uses/might use, if they don't already exist.
-func CreateServiceAccounts(clientset *clientset.Clientset) error {
+func CreateServiceAccounts(clientset clientset.Interface) error {
 	serviceAccounts := []v1.ServiceAccount{
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -64,9 +65,11 @@ func CreateServiceAccounts(clientset *clientset.Clientset) error {
 
 	for _, sa := range serviceAccounts {
 		if _, err := clientset.CoreV1().ServiceAccounts(metav1.NamespaceSystem).Create(&sa); err != nil {
+			if !apierrors.IsAlreadyExists(err) {
 				return err
 			}
 		}
+	}
 	return nil
 }
 

cmd/kubeadm/app/phases/apiconfig/clusterroles_test.go

@@ -0,0 +1,83 @@
+/*
+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 (
+	"testing"
+
+	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"
+	"k8s.io/kubernetes/pkg/api"
+)
+
+func TestCreateServiceAccounts(t *testing.T) {
+	tests := []struct {
+		name      string
+		createErr error
+		expectErr bool
+	}{
+		{
+			"error-free case",
+			nil,
+			false,
+		},
+		{
+			"duplication errors should be ignored",
+			apierrors.NewAlreadyExists(api.Resource(""), ""),
+			false,
+		},
+		{
+			"unexpected errors should be returned",
+			apierrors.NewUnauthorized(""),
+			true,
+		},
+	}
+
+	for _, tc := range tests {
+		client := clientsetfake.NewSimpleClientset()
+		if tc.createErr != nil {
+			client.PrependReactor("create", "serviceaccounts", func(action core.Action) (bool, runtime.Object, error) {
+				return true, nil, tc.createErr
+			})
+		}
+
+		err := CreateServiceAccounts(client)
+		if tc.expectErr {
+			if err == nil {
+				t.Errorf("CreateServiceAccounts(%s) wanted err, got nil", tc.name)
+			}
+			continue
+		} else if !tc.expectErr && err != nil {
+			t.Errorf("CreateServiceAccounts(%s) returned unexpected err: %v", tc.name, err)
+		}
+
+		wantResourcesCreated := 2
+		if len(client.Actions()) != wantResourcesCreated {
+			t.Errorf("CreateServiceAccounts(%s) should have made %d actions, but made %d", tc.name, wantResourcesCreated, len(client.Actions()))
+		}
+
+		for _, action := range client.Actions() {
+			if action.GetVerb() != "create" || action.GetResource().Resource != "serviceaccounts" {
+				t.Errorf("CreateServiceAccounts(%s) called [%v %v], but wanted [create serviceaccounts]",
+					tc.name, action.GetVerb(), action.GetResource().Resource)
+			}
+		}
+
+	}
+}