Merge pull request kubernetes#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.

kubernetes/kubeadm#278

Fixes: kubernetes/kubeadm#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.
-func CreateServiceAccounts(clientset *clientset.Clientset) error {
+// CreateServiceAccounts creates the necessary serviceaccounts that kubeadm uses/might use, if they don't already exist.
+func CreateServiceAccounts(clientset clientset.Interface) error {
  serviceAccounts := []v1.ServiceAccount{
  {
  ObjectMeta: metav1.ObjectMeta{
@@ -64,7 +65,9 @@ func CreateServiceAccounts(clientset *clientset.Clientset) error {
 
  for _, sa := range serviceAccounts {
  if _, err := clientset.CoreV1().ServiceAccounts(metav1.NamespaceSystem).Create(&sa); err != nil {
- return err
+ 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)
+ }
+ }
+
+ }
+}