Merge pull request #9009 from liggitt/token_controller_fixes

pkg/controller/serviceaccount/tokens_controller.go

@@ -32,9 +32,12 @@ import (
 	"k8s.io/kubernetes/pkg/registry/secret"
 	"k8s.io/kubernetes/pkg/runtime"
 	"k8s.io/kubernetes/pkg/util/sets"
+	"k8s.io/kubernetes/pkg/util/wait"
 	"k8s.io/kubernetes/pkg/watch"
 )
 
+const NumServiceAccountRemoveReferenceRetries = 10
+
 // TokensControllerOptions contains options for the TokensController
 type TokensControllerOptions struct {
 	// TokenGenerator is the generator to use to create new tokens
@@ -239,8 +242,16 @@ func (e *TokensController) secretDeleted(obj interface{}) {
 		return
 	}
 
-	if _, err := e.removeSecretReferenceIfNeeded(serviceAccount, secret.Name); err != nil {
-		glog.Error(err)
+	for i := 1; i <= NumServiceAccountRemoveReferenceRetries; i++ {
+		if _, err := e.removeSecretReferenceIfNeeded(serviceAccount, secret.Name); err != nil {
+			if apierrors.IsConflict(err) && i < NumServiceAccountRemoveReferenceRetries {
+				time.Sleep(wait.Jitter(100*time.Millisecond, 0.0))
+				continue
+			}
+			glog.Error(err)
+			break
+		}
+		break
 	}
 }
 
@@ -274,6 +285,21 @@ func (e *TokensController) createSecretIfNeeded(serviceAccount *api.ServiceAccou
 
 // createSecret creates a secret of type ServiceAccountToken for the given ServiceAccount
 func (e *TokensController) createSecret(serviceAccount *api.ServiceAccount) error {
+	// We don't want to update the cache's copy of the service account
+	// so add the secret to a freshly retrieved copy of the service account
+	serviceAccounts := e.client.ServiceAccounts(serviceAccount.Namespace)
+	liveServiceAccount, err := serviceAccounts.Get(serviceAccount.Name)
+	if err != nil {
+		return err
+	}
+	if liveServiceAccount.ResourceVersion != serviceAccount.ResourceVersion {
+		// our view of the service account is not up to date
+		// we'll get notified of an update event later and get to try again
+		// this only prevent interactions between successive runs of this controller's event handlers, but that is useful
+		glog.V(2).Infof("View of ServiceAccount %s/%s is not up to date, skipping token creation", serviceAccount.Namespace, serviceAccount.Name)
+		return nil
+	}
+
 	// Build the secret
 	secret := &api.Secret{
 		ObjectMeta: api.ObjectMeta{
@@ -303,16 +329,9 @@ func (e *TokensController) createSecret(serviceAccount *api.ServiceAccount) erro
 		return err
 	}
 
-	// We don't want to update the cache's copy of the service account
-	// so add the secret to a freshly retrieved copy of the service account
-	serviceAccounts := e.client.ServiceAccounts(serviceAccount.Namespace)
-	serviceAccount, err = serviceAccounts.Get(serviceAccount.Name)
-	if err != nil {
-		return err
-	}
-	serviceAccount.Secrets = append(serviceAccount.Secrets, api.ObjectReference{Name: secret.Name})
+	liveServiceAccount.Secrets = append(liveServiceAccount.Secrets, api.ObjectReference{Name: secret.Name})
 
-	_, err = serviceAccounts.Update(serviceAccount)
+	_, err = serviceAccounts.Update(liveServiceAccount)
 	if err != nil {
 		// we weren't able to use the token, try to clean it up.
 		glog.V(2).Infof("Deleting secret %s/%s because reference couldn't be added (%v)", secret.Namespace, secret.Name, err)

pkg/controller/serviceaccount/tokens_controller_test.go

@@ -77,6 +77,13 @@ func serviceAccount(secretRefs []api.ObjectReference) *api.ServiceAccount {
 	}
 }
 
+// updatedServiceAccount returns a service account with the resource version modified
+func updatedServiceAccount(secretRefs []api.ObjectReference) *api.ServiceAccount {
+	sa := serviceAccount(secretRefs)
+	sa.ResourceVersion = "2"
+	return sa
+}
+
 // opaqueSecret returns a persisted non-ServiceAccountToken secret named "regular-secret-1"
 func opaqueSecret() *api.Secret {
 	return &api.Secret{
@@ -179,8 +186,8 @@ func TestTokenCreation(t *testing.T) {
 
 			AddedServiceAccount: serviceAccount(emptySecretReferences()),
 			ExpectedActions: []testclient.Action{
-				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewGetAction("serviceaccounts", api.NamespaceDefault, "default"),
+				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewUpdateAction("serviceaccounts", api.NamespaceDefault, serviceAccount(addTokenSecretReference(emptySecretReferences()))),
 			},
 		},
@@ -191,8 +198,8 @@ func TestTokenCreation(t *testing.T) {
 
 			AddedServiceAccount: serviceAccount(emptySecretReferences()),
 			ExpectedActions: []testclient.Action{
-				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewGetAction("serviceaccounts", api.NamespaceDefault, "default"),
+				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewUpdateAction("serviceaccounts", api.NamespaceDefault, serviceAccount(addTokenSecretReference(emptySecretReferences()))),
 			},
 		},
@@ -201,8 +208,8 @@ func TestTokenCreation(t *testing.T) {
 
 			AddedServiceAccount: serviceAccount(missingSecretReferences()),
 			ExpectedActions: []testclient.Action{
-				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewGetAction("serviceaccounts", api.NamespaceDefault, "default"),
+				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewUpdateAction("serviceaccounts", api.NamespaceDefault, serviceAccount(addTokenSecretReference(missingSecretReferences()))),
 			},
 		},
@@ -219,8 +226,8 @@ func TestTokenCreation(t *testing.T) {
 
 			AddedServiceAccount: serviceAccount(regularSecretReferences()),
 			ExpectedActions: []testclient.Action{
-				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewGetAction("serviceaccounts", api.NamespaceDefault, "default"),
+				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewUpdateAction("serviceaccounts", api.NamespaceDefault, serviceAccount(addTokenSecretReference(regularSecretReferences()))),
 			},
 		},
@@ -231,14 +238,22 @@ func TestTokenCreation(t *testing.T) {
 			AddedServiceAccount: serviceAccount(tokenSecretReferences()),
 			ExpectedActions:     []testclient.Action{},
 		},
+		"new serviceaccount with no secrets with resource conflict": {
+			ClientObjects: []runtime.Object{updatedServiceAccount(emptySecretReferences()), createdTokenSecret()},
+
+			AddedServiceAccount: serviceAccount(emptySecretReferences()),
+			ExpectedActions: []testclient.Action{
+				testclient.NewGetAction("serviceaccounts", api.NamespaceDefault, "default"),
+			},
+		},
 
 		"updated serviceaccount with no secrets": {
 			ClientObjects: []runtime.Object{serviceAccount(emptySecretReferences()), createdTokenSecret()},
 
 			UpdatedServiceAccount: serviceAccount(emptySecretReferences()),
 			ExpectedActions: []testclient.Action{
-				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewGetAction("serviceaccounts", api.NamespaceDefault, "default"),
+				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewUpdateAction("serviceaccounts", api.NamespaceDefault, serviceAccount(addTokenSecretReference(emptySecretReferences()))),
 			},
 		},
@@ -249,8 +264,8 @@ func TestTokenCreation(t *testing.T) {
 
 			UpdatedServiceAccount: serviceAccount(emptySecretReferences()),
 			ExpectedActions: []testclient.Action{
-				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewGetAction("serviceaccounts", api.NamespaceDefault, "default"),
+				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewUpdateAction("serviceaccounts", api.NamespaceDefault, serviceAccount(addTokenSecretReference(emptySecretReferences()))),
 			},
 		},
@@ -259,8 +274,8 @@ func TestTokenCreation(t *testing.T) {
 
 			UpdatedServiceAccount: serviceAccount(missingSecretReferences()),
 			ExpectedActions: []testclient.Action{
-				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewGetAction("serviceaccounts", api.NamespaceDefault, "default"),
+				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewUpdateAction("serviceaccounts", api.NamespaceDefault, serviceAccount(addTokenSecretReference(missingSecretReferences()))),
 			},
 		},
@@ -277,8 +292,8 @@ func TestTokenCreation(t *testing.T) {
 
 			UpdatedServiceAccount: serviceAccount(regularSecretReferences()),
 			ExpectedActions: []testclient.Action{
-				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewGetAction("serviceaccounts", api.NamespaceDefault, "default"),
+				testclient.NewCreateAction("secrets", api.NamespaceDefault, createdTokenSecret()),
 				testclient.NewUpdateAction("serviceaccounts", api.NamespaceDefault, serviceAccount(addTokenSecretReference(regularSecretReferences()))),
 			},
 		},
@@ -288,6 +303,14 @@ func TestTokenCreation(t *testing.T) {
 			UpdatedServiceAccount: serviceAccount(tokenSecretReferences()),
 			ExpectedActions:       []testclient.Action{},
 		},
+		"updated serviceaccount with no secrets with resource conflict": {
+			ClientObjects: []runtime.Object{updatedServiceAccount(emptySecretReferences()), createdTokenSecret()},
+
+			UpdatedServiceAccount: serviceAccount(emptySecretReferences()),
+			ExpectedActions: []testclient.Action{
+				testclient.NewGetAction("serviceaccounts", api.NamespaceDefault, "default"),
+			},
+		},
 
 		"deleted serviceaccount with no secrets": {
 			DeletedServiceAccount: serviceAccount(emptySecretReferences()),