Allow ServiceAccountsController to manage multiple named service accounts

cmd/kube-controller-manager/app/controllermanager.go

@@ -263,7 +263,7 @@ func (s *CMServer) Run(_ []string) error {
 
 	serviceaccount.NewServiceAccountsController(
 		kubeClient,
-		serviceaccount.DefaultServiceAccountControllerOptions(),
+		serviceaccount.DefaultServiceAccountsControllerOptions(),
 	).Run()
 
 	select {}

pkg/serviceaccount/serviceaccounts_controller.go

@@ -28,6 +28,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
 	"github.com/golang/glog"
 )
@@ -41,24 +42,38 @@ func nameIndexFunc(obj interface{}) (string, error) {
 	return meta.Name(), nil
 }
 
-type ServiceAccountControllerOptions struct {
-	Name                 string
+// ServiceAccountsControllerOptions contains options for running a ServiceAccountsController
+type ServiceAccountsControllerOptions struct {
+	// Names is the set of service account names to ensure exist in every namespace
+	Names util.StringSet
+
+	// ServiceAccountResync is the interval between full resyncs of ServiceAccounts.
+	// If non-zero, all service accounts will be re-listed this often.
+	// Otherwise, re-list will be delayed as long as possible (until the watch is closed or times out).
 	ServiceAccountResync time.Duration
-	NamespaceResync      time.Duration
+
+	// NamespaceResync is the interval between full resyncs of Namespaces.
+	// If non-zero, all namespaces will be re-listed this often.
+	// Otherwise, re-list will be delayed as long as possible (until the watch is closed or times out).
+	NamespaceResync time.Duration
 }
 
-func DefaultServiceAccountControllerOptions() ServiceAccountControllerOptions {
-	return ServiceAccountControllerOptions{Name: "default"}
+func DefaultServiceAccountsControllerOptions() ServiceAccountsControllerOptions {
+	return ServiceAccountsControllerOptions{Names: util.NewStringSet("default")}
 }
 
 // NewServiceAccountsController returns a new *ServiceAccountsController.
-func NewServiceAccountsController(cl *client.Client, options ServiceAccountControllerOptions) *ServiceAccountsController {
+func NewServiceAccountsController(cl client.Interface, options ServiceAccountsControllerOptions) *ServiceAccountsController {
 	e := &ServiceAccountsController{
 		client: cl,
-		name:   options.Name,
+		names:  options.Names,
 	}
 
-	accountSelector := fields.SelectorFromSet(map[string]string{client.ObjectNameField: options.Name})
+	accountSelector := fields.Everything()
+	if len(options.Names) == 1 {
+		// If we're maintaining a single account, we can scope the accounts we watch to just that name
+		accountSelector = fields.SelectorFromSet(map[string]string{client.ObjectNameField: options.Names.List()[0]})
+	}
 	e.serviceAccounts, e.serviceAccountController = framework.NewIndexerInformer(
 		&cache.ListWatch{
 			ListFunc: func() (runtime.Object, error) {
@@ -101,8 +116,8 @@ func NewServiceAccountsController(cl *client.Client, options ServiceAccountContr
 type ServiceAccountsController struct {
 	stopChan chan struct{}
 
-	client *client.Client
-	name   string
+	client client.Interface
+	names  util.StringSet
 
 	serviceAccounts cache.Indexer
 	namespaces      cache.Indexer
@@ -137,27 +152,34 @@ func (e *ServiceAccountsController) serviceAccountDeleted(obj interface{}) {
 		// corresponding secrets will be cleaned up during the Secret re-list
 		return
 	}
-	e.createDefaultServiceAccountIfNeeded(serviceAccount.Namespace)
+	// If the deleted service account is one we're maintaining, recreate it
+	if e.names.Has(serviceAccount.Name) {
+		e.createServiceAccountIfNeeded(serviceAccount.Name, serviceAccount.Namespace)
+	}
 }
 
 // namespaceAdded reacts to a Namespace creation by creating a default ServiceAccount object
 func (e *ServiceAccountsController) namespaceAdded(obj interface{}) {
 	namespace := obj.(*api.Namespace)
-	e.createDefaultServiceAccountIfNeeded(namespace.Name)
+	for _, name := range e.names.List() {
+		e.createServiceAccountIfNeeded(name, namespace.Name)
+	}
 }
 
 // namespaceUpdated reacts to a Namespace update (or re-list) by creating a default ServiceAccount in the namespace if needed
 func (e *ServiceAccountsController) namespaceUpdated(oldObj interface{}, newObj interface{}) {
 	newNamespace := newObj.(*api.Namespace)
-	e.createDefaultServiceAccountIfNeeded(newNamespace.Name)
+	for _, name := range e.names.List() {
+		e.createServiceAccountIfNeeded(name, newNamespace.Name)
+	}
 }
 
-// createDefaultServiceAccountIfNeeded creates a default ServiceAccount in the given namespace if:
-// * it default ServiceAccount does not already exist
+// createServiceAccountIfNeeded creates a ServiceAccount with the given name in the given namespace if:
+// * the named ServiceAccount does not already exist
 // * the specified namespace exists
 // * the specified namespace is in the ACTIVE phase
-func (e *ServiceAccountsController) createDefaultServiceAccountIfNeeded(namespace string) {
-	serviceAccount, err := e.getDefaultServiceAccount(namespace)
+func (e *ServiceAccountsController) createServiceAccountIfNeeded(name, namespace string) {
+	serviceAccount, err := e.getServiceAccount(name, namespace)
 	if err != nil {
 		glog.Error(err)
 		return
@@ -181,22 +203,21 @@ func (e *ServiceAccountsController) createDefaultServiceAccountIfNeeded(namespac
 		return
 	}
 
-	e.createDefaultServiceAccount(namespace)
+	e.createServiceAccount(name, namespace)
 }
 
 // createDefaultServiceAccount creates a default ServiceAccount in the specified namespace
-func (e *ServiceAccountsController) createDefaultServiceAccount(namespace string) {
+func (e *ServiceAccountsController) createServiceAccount(name, namespace string) {
 	serviceAccount := &api.ServiceAccount{}
-	serviceAccount.Name = e.name
+	serviceAccount.Name = name
 	serviceAccount.Namespace = namespace
 	if _, err := e.client.ServiceAccounts(namespace).Create(serviceAccount); err != nil {
 		glog.Error(err)
 	}
 }
 
-// getDefaultServiceAccount returns the default ServiceAccount for the given namespace
-func (e *ServiceAccountsController) getDefaultServiceAccount(namespace string) (*api.ServiceAccount, error) {
-
+// getDefaultServiceAccount returns the ServiceAccount with the given name for the given namespace
+func (e *ServiceAccountsController) getServiceAccount(name, namespace string) (*api.ServiceAccount, error) {
 	key := &api.ServiceAccount{ObjectMeta: api.ObjectMeta{Namespace: namespace}}
 	accounts, err := e.serviceAccounts.Index("namespace", key)
 	if err != nil {
@@ -205,7 +226,7 @@ func (e *ServiceAccountsController) getDefaultServiceAccount(namespace string) (
 
 	for _, obj := range accounts {
 		serviceAccount := obj.(*api.ServiceAccount)
-		if e.name == serviceAccount.Name {
+		if name == serviceAccount.Name {
 			return serviceAccount, nil
 		}
 	}

pkg/serviceaccount/serviceaccounts_controller_test.go

@@ -23,7 +23,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/testapi"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/testclient"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
@@ -51,6 +51,9 @@ func makeTestServer(t *testing.T, namespace string, serviceAccountResponse serve
 func TestServiceAccountCreation(t *testing.T) {
 	ns := api.NamespaceDefault
 
+	defaultName := "default"
+	managedName := "managed"
+
 	activeNS := &api.Namespace{
 		ObjectMeta: api.ObjectMeta{Name: ns},
 		Status: api.NamespaceStatus{
@@ -63,79 +66,118 @@ func TestServiceAccountCreation(t *testing.T) {
 			Phase: api.NamespaceTerminating,
 		},
 	}
-	serviceAccount := &api.ServiceAccount{
+	defaultServiceAccount := &api.ServiceAccount{
+		ObjectMeta: api.ObjectMeta{
+			Name:            defaultName,
+			Namespace:       ns,
+			ResourceVersion: "1",
+		},
+	}
+	managedServiceAccount := &api.ServiceAccount{
 		ObjectMeta: api.ObjectMeta{
-			Name:            "default",
+			Name:            managedName,
+			Namespace:       ns,
+			ResourceVersion: "1",
+		},
+	}
+	unmanagedServiceAccount := &api.ServiceAccount{
+		ObjectMeta: api.ObjectMeta{
+			Name:            "other-unmanaged",
 			Namespace:       ns,
 			ResourceVersion: "1",
 		},
 	}
 
 	testcases := map[string]struct {
-		ExistingNamespace      *api.Namespace
-		ExistingServiceAccount *api.ServiceAccount
+		ExistingNamespace       *api.Namespace
+		ExistingServiceAccounts []*api.ServiceAccount
 
 		AddedNamespace        *api.Namespace
 		UpdatedNamespace      *api.Namespace
 		DeletedServiceAccount *api.ServiceAccount
 
-		ExpectCreatedServiceAccount bool
+		ExpectCreatedServiceAccounts []string
 	}{
+		"new active namespace missing serviceaccounts": {
+			ExistingServiceAccounts:      []*api.ServiceAccount{},
+			AddedNamespace:               activeNS,
+			ExpectCreatedServiceAccounts: util.NewStringSet(defaultName, managedName).List(),
+		},
 		"new active namespace missing serviceaccount": {
-			AddedNamespace:              activeNS,
-			ExpectCreatedServiceAccount: true,
+			ExistingServiceAccounts:      []*api.ServiceAccount{managedServiceAccount},
+			AddedNamespace:               activeNS,
+			ExpectCreatedServiceAccounts: []string{defaultName},
 		},
-		"new active namespace with serviceaccount": {
-			ExistingServiceAccount:      serviceAccount,
-			AddedNamespace:              activeNS,
-			ExpectCreatedServiceAccount: false,
+		"new active namespace with serviceaccounts": {
+			ExistingServiceAccounts:      []*api.ServiceAccount{defaultServiceAccount, managedServiceAccount},
+			AddedNamespace:               activeNS,
+			ExpectCreatedServiceAccounts: []string{},
 		},
+
 		"new terminating namespace": {
-			AddedNamespace:              terminatingNS,
-			ExpectCreatedServiceAccount: false,
+			ExistingServiceAccounts:      []*api.ServiceAccount{},
+			AddedNamespace:               terminatingNS,
+			ExpectCreatedServiceAccounts: []string{},
 		},
 
+		"updated active namespace missing serviceaccounts": {
+			ExistingServiceAccounts:      []*api.ServiceAccount{},
+			UpdatedNamespace:             activeNS,
+			ExpectCreatedServiceAccounts: util.NewStringSet(defaultName, managedName).List(),
+		},
 		"updated active namespace missing serviceaccount": {
-			UpdatedNamespace:            activeNS,
-			ExpectCreatedServiceAccount: true,
+			ExistingServiceAccounts:      []*api.ServiceAccount{defaultServiceAccount},
+			UpdatedNamespace:             activeNS,
+			ExpectCreatedServiceAccounts: []string{managedName},
 		},
-		"updated active namespace with serviceaccount": {
-			ExistingServiceAccount:      serviceAccount,
-			UpdatedNamespace:            activeNS,
-			ExpectCreatedServiceAccount: false,
+		"updated active namespace with serviceaccounts": {
+			ExistingServiceAccounts:      []*api.ServiceAccount{defaultServiceAccount, managedServiceAccount},
+			UpdatedNamespace:             activeNS,
+			ExpectCreatedServiceAccounts: []string{},
 		},
 		"updated terminating namespace": {
-			UpdatedNamespace:            terminatingNS,
-			ExpectCreatedServiceAccount: false,
+			ExistingServiceAccounts:      []*api.ServiceAccount{},
+			UpdatedNamespace:             terminatingNS,
+			ExpectCreatedServiceAccounts: []string{},
 		},
 
 		"deleted serviceaccount without namespace": {
-			DeletedServiceAccount:       serviceAccount,
-			ExpectCreatedServiceAccount: false,
+			DeletedServiceAccount:        defaultServiceAccount,
+			ExpectCreatedServiceAccounts: []string{},
 		},
 		"deleted serviceaccount with active namespace": {
-			ExistingNamespace:           activeNS,
-			DeletedServiceAccount:       serviceAccount,
-			ExpectCreatedServiceAccount: true,
+			ExistingNamespace:            activeNS,
+			DeletedServiceAccount:        defaultServiceAccount,
+			ExpectCreatedServiceAccounts: []string{defaultName},
 		},
 		"deleted serviceaccount with terminating namespace": {
-			ExistingNamespace:           terminatingNS,
-			DeletedServiceAccount:       serviceAccount,
-			ExpectCreatedServiceAccount: false,
+			ExistingNamespace:            terminatingNS,
+			DeletedServiceAccount:        defaultServiceAccount,
+			ExpectCreatedServiceAccounts: []string{},
+		},
+		"deleted unmanaged serviceaccount with active namespace": {
+			ExistingNamespace:            activeNS,
+			DeletedServiceAccount:        unmanagedServiceAccount,
+			ExpectCreatedServiceAccounts: []string{},
+		},
+		"deleted unmanaged serviceaccount with terminating namespace": {
+			ExistingNamespace:            terminatingNS,
+			DeletedServiceAccount:        unmanagedServiceAccount,
+			ExpectCreatedServiceAccounts: []string{},
 		},
 	}
 
 	for k, tc := range testcases {
-
-		testServer, handler := makeTestServer(t, ns, serverResponse{http.StatusOK, serviceAccount})
-		client := client.NewOrDie(&client.Config{Host: testServer.URL, Version: testapi.Version()})
-		controller := NewServiceAccountsController(client, DefaultServiceAccountControllerOptions())
+		client := testclient.NewSimpleFake(defaultServiceAccount, managedServiceAccount)
+		options := DefaultServiceAccountsControllerOptions()
+		options.Names = util.NewStringSet(defaultName, managedName)
+		controller := NewServiceAccountsController(client, options)
 
 		if tc.ExistingNamespace != nil {
 			controller.namespaces.Add(tc.ExistingNamespace)
 		}
-		if tc.ExistingServiceAccount != nil {
-			controller.serviceAccounts.Add(tc.ExistingServiceAccount)
+		for _, s := range tc.ExistingServiceAccounts {
+			controller.serviceAccounts.Add(s)
 		}
 
 		if tc.AddedNamespace != nil {
@@ -150,16 +192,20 @@ func TestServiceAccountCreation(t *testing.T) {
 			controller.serviceAccountDeleted(tc.DeletedServiceAccount)
 		}
 
-		if tc.ExpectCreatedServiceAccount {
-			if !handler.ValidateRequestCount(t, 1) {
-				t.Errorf("%s: Expected a single creation call", k)
+		if len(tc.ExpectCreatedServiceAccounts) != len(client.Actions) {
+			t.Errorf("%s: Expected to create accounts %#v. Actual actions were: %#v", k, tc.ExpectCreatedServiceAccounts, client.Actions)
+			continue
+		}
+		for i, expectedName := range tc.ExpectCreatedServiceAccounts {
+			action := client.Actions[i]
+			if action.Action != "create-serviceaccount" {
+				t.Errorf("%s: Unexpected action %s", k, action.Action)
+				break
 			}
-		} else {
-			if !handler.ValidateRequestCount(t, 0) {
-				t.Errorf("%s: Expected no creation calls", k)
+			createdAccount := action.Value.(*api.ServiceAccount)
+			if createdAccount.Name != expectedName {
+				t.Errorf("%s: Expected %s to be created, got %s", k, expectedName, createdAccount.Name)
 			}
 		}
-
-		testServer.Close()
 	}
 }

test/integration/service_account_test.go

@@ -423,7 +423,7 @@ func startServiceAccountTestServer(t *testing.T) (*client.Client, client.Config,
 	// Start the service account and service account token controllers
 	tokenController := serviceaccount.NewTokensController(rootClient, serviceaccount.DefaultTokenControllerOptions(serviceaccount.JWTTokenGenerator(serviceAccountKey)))
 	tokenController.Run()
-	serviceAccountController := serviceaccount.NewServiceAccountsController(rootClient, serviceaccount.DefaultServiceAccountControllerOptions())
+	serviceAccountController := serviceaccount.NewServiceAccountsController(rootClient, serviceaccount.DefaultServiceAccountsControllerOptions())
 	serviceAccountController.Run()
 	// Start the admission plugin reflectors
 	serviceAccountAdmission.Run()