diff --git a/pkg/registry/core/service/storage/rest.go b/pkg/registry/core/service/storage/rest.go index 5167c7bc8bc6..dc4e6366b6e8 100644 --- a/pkg/registry/core/service/storage/rest.go +++ b/pkg/registry/core/service/storage/rest.go @@ -247,7 +247,7 @@ func (rs *REST) Create(ctx context.Context, obj runtime.Object, createValidation out, err := rs.services.Create(ctx, service, createValidation, options) if err != nil { - err = rest.CheckGeneratedNameError(rs.strategy, err, service) + err = rest.CheckGeneratedNameError(ctx, rs.strategy, err, service) } if err == nil { diff --git a/pkg/registry/core/service/strategy_test.go b/pkg/registry/core/service/strategy_test.go index b1bcefbbc8c7..bc5759788d9b 100644 --- a/pkg/registry/core/service/strategy_test.go +++ b/pkg/registry/core/service/strategy_test.go @@ -48,19 +48,23 @@ func newStrategy(cidr string, hasSecondary bool) (testStrategy Strategy, testSta } func TestCheckGeneratedNameError(t *testing.T) { + ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{ + Resource: "foos", + }) + testStrategy, _ := newStrategy("10.0.0.0/16", false) expect := errors.NewNotFound(api.Resource("foos"), "bar") - if err := rest.CheckGeneratedNameError(testStrategy, expect, &api.Service{}); err != expect { + if err := rest.CheckGeneratedNameError(ctx, testStrategy, expect, &api.Service{}); err != expect { t.Errorf("NotFoundError should be ignored: %v", err) } expect = errors.NewAlreadyExists(api.Resource("foos"), "bar") - if err := rest.CheckGeneratedNameError(testStrategy, expect, &api.Service{}); err != expect { + if err := rest.CheckGeneratedNameError(ctx, testStrategy, expect, &api.Service{}); err != expect { t.Errorf("AlreadyExists should be returned when no GenerateName field: %v", err) } expect = errors.NewAlreadyExists(api.Resource("foos"), "bar") - if err := rest.CheckGeneratedNameError(testStrategy, expect, &api.Service{ObjectMeta: metav1.ObjectMeta{GenerateName: "foo"}}); err == nil || !errors.IsServerTimeout(err) { + if err := rest.CheckGeneratedNameError(ctx, testStrategy, expect, &api.Service{ObjectMeta: metav1.ObjectMeta{GenerateName: "foo"}}); err == nil || !errors.IsAlreadyExists(err) { t.Errorf("expected try again later error: %v", err) } } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go index 959885cfbbe2..253fda22816a 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go @@ -170,6 +170,25 @@ func NewAlreadyExists(qualifiedResource schema.GroupResource, name string) *Stat }} } +// NewGenerateNameConflict returns an error indicating the server +// was not able to generate a valid name for a resource. +func NewGenerateNameConflict(qualifiedResource schema.GroupResource, name string, retryAfterSeconds int) *StatusError { + return &StatusError{metav1.Status{ + Status: metav1.StatusFailure, + Code: http.StatusConflict, + Reason: metav1.StatusReasonAlreadyExists, + Details: &metav1.StatusDetails{ + Group: qualifiedResource.Group, + Kind: qualifiedResource.Resource, + Name: name, + RetryAfterSeconds: int32(retryAfterSeconds), + }, + Message: fmt.Sprintf( + "%s %q already exists, the server was not able to generate a unique name for the object", + qualifiedResource.String(), name), + }} +} + // NewUnauthorized returns an error indicating the client is not authorized to perform the requested // action. func NewUnauthorized(reason string) *StatusError { diff --git a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go index ffba3f3aa543..057f548dbb0e 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go @@ -64,7 +64,7 @@ func TestErrorNew(t *testing.T) { } if !IsConflict(NewConflict(resource("tests"), "2", errors.New("message"))) { - t.Errorf("expected to be conflict") + t.Errorf("expected to be %s", metav1.StatusReasonAlreadyExists) } if !IsNotFound(NewNotFound(resource("tests"), "3")) { t.Errorf("expected to be %s", metav1.StatusReasonNotFound) @@ -88,6 +88,13 @@ func TestErrorNew(t *testing.T) { t.Errorf("expected to be %s", metav1.StatusReasonMethodNotAllowed) } + if !IsAlreadyExists(NewGenerateNameConflict(resource("tests"), "3", 1)) { + t.Errorf("expected to be %s", metav1.StatusReasonAlreadyExists) + } + if time, ok := SuggestsClientDelay(NewGenerateNameConflict(resource("tests"), "3", 1)); time != 1 || !ok { + t.Errorf("unexpected %d", time) + } + if time, ok := SuggestsClientDelay(NewServerTimeout(resource("tests"), "doing something", 10)); time != 10 || !ok { t.Errorf("unexpected %d", time) } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go index d94627834da5..faee83855a01 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go @@ -404,7 +404,7 @@ func (e *Store) Create(ctx context.Context, obj runtime.Object, createValidation out := e.NewFunc() if err := e.Storage.Create(ctx, key, obj, out, ttl, dryrun.IsDryRun(options.DryRun)); err != nil { err = storeerr.InterpretCreateError(err, qualifiedResource, name) - err = rest.CheckGeneratedNameError(e.CreateStrategy, err, obj) + err = rest.CheckGeneratedNameError(ctx, e.CreateStrategy, err, obj) if !apierrors.IsAlreadyExists(err) { return nil, err } @@ -659,7 +659,7 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj } if creating { err = storeerr.InterpretCreateError(err, qualifiedResource, name) - err = rest.CheckGeneratedNameError(e.CreateStrategy, err, creatingObj) + err = rest.CheckGeneratedNameError(ctx, e.CreateStrategy, err, creatingObj) } else { err = storeerr.InterpretUpdateError(err, qualifiedResource, name) } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go index 950260e457c0..942ee4774c0a 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/admission" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/storage/names" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -109,6 +110,7 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime. objectMeta.SetDeletionGracePeriodSeconds(nil) strategy.PrepareForCreate(ctx, obj) FillObjectMetaSystemFields(objectMeta) + if len(objectMeta.GetGenerateName()) > 0 && len(objectMeta.GetName()) == 0 { objectMeta.SetName(strategy.GenerateName(objectMeta.GetGenerateName())) } @@ -145,21 +147,31 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime. // CheckGeneratedNameError checks whether an error that occurred creating a resource is due // to generation being unable to pick a valid name. -func CheckGeneratedNameError(strategy RESTCreateStrategy, err error, obj runtime.Object) error { +func CheckGeneratedNameError(ctx context.Context, strategy RESTCreateStrategy, err error, obj runtime.Object) error { if !errors.IsAlreadyExists(err) { return err } - objectMeta, kind, kerr := objectMetaAndKind(strategy, obj) + objectMeta, gvk, kerr := objectMetaAndKind(strategy, obj) if kerr != nil { return kerr } if len(objectMeta.GetGenerateName()) == 0 { + // If we don't have a generated name, return the original error (AlreadyExists). + // When we're here, the user picked a name that is causing a conflict. return err } - return errors.NewServerTimeoutForKind(kind.GroupKind(), "POST", 0) + // Get the group resource information from the context, if populated. + gr := schema.GroupResource{} + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + gr = schema.GroupResource{Group: gvk.Group, Resource: requestInfo.Resource} + } + + // If we have a name and generated name, the server picked a name + // that already exists. + return errors.NewGenerateNameConflict(gr, objectMeta.GetName(), 1) } // objectMetaAndKind retrieves kind and ObjectMeta from a runtime object, or returns an error.