diff --git a/pkg/controller/serviceaccount/tokens_controller.go b/pkg/controller/serviceaccount/tokens_controller.go index 667c32b431b..e4b6afaf495 100644 --- a/pkg/controller/serviceaccount/tokens_controller.go +++ b/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) diff --git a/pkg/controller/serviceaccount/tokens_controller_test.go b/pkg/controller/serviceaccount/tokens_controller_test.go index 144c0b952f8..2cfbc39373a 100644 --- a/pkg/controller/serviceaccount/tokens_controller_test.go +++ b/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()),