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 03ec793f81f..40bca49665f 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 @@ -387,6 +387,9 @@ func (e *Store) Create(ctx context.Context, obj runtime.Object, createValidation return nil, err } else { rest.FillObjectMetaSystemFields(objectMeta) + if len(objectMeta.GetGenerateName()) > 0 && len(objectMeta.GetName()) == 0 { + objectMeta.SetName(e.CreateStrategy.GenerateName(objectMeta.GetGenerateName())) + } } if e.BeginCreate != nil { diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go index 1c0b2cc6a76..f7af4ec89fc 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go @@ -2845,3 +2845,72 @@ func TestValidateIndexers(t *testing.T) { } } } + +type predictableNameGenerator struct { + index int +} + +func (p *predictableNameGenerator) GenerateName(base string) string { + p.index++ + return fmt.Sprintf("%s%d", base, p.index) +} + +func TestStoreCreateGenerateNameConflict(t *testing.T) { + // podA will be stored with name foo12345 + podA := &example.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "foo1", Namespace: "test"}, + Spec: example.PodSpec{NodeName: "machine"}, + } + // podB will generate the same name as podA "foo1" in the first try + podB := &example.Pod{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "foo", Namespace: "test"}, + Spec: example.PodSpec{NodeName: "machine2"}, + } + + testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test") + destroyFunc, registry := NewTestGenericStoreRegistry(t) + defer destroyFunc() + // re-define delete strategy to have graceful delete capability + defaultCreateStrategy := &testRESTStrategy{scheme, &predictableNameGenerator{}, true, false, true} + registry.CreateStrategy = defaultCreateStrategy + + // create the object (DeepCopy because the registry mutates the objects) + objA, err := registry.Create(testContext, podA.DeepCopy(), rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // get the object + checkobjA, err := registry.Get(testContext, podA.Name, &metav1.GetOptions{}) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // verify objects are equal + if e, a := objA, checkobjA; !reflect.DeepEqual(e, a) { + t.Errorf("Expected %#v, got %#v", e, a) + } + + // now try to create the second pod (DeepCopy because the registry mutate the objects) + _, err = registry.Create(testContext, podB.DeepCopy(), rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) + if !errors.IsAlreadyExists(err) { + t.Errorf("Unexpected error: %+v", err) + } + + // check the 'alraedy exists' msg correspond to the generated name + msg := &err.(*errors.StatusError).ErrStatus.Message + if !strings.Contains(*msg, "already exists, the server was not able to generate a unique name for the object") { + t.Errorf("Unexpected error without the 'was not able to generate a unique name' in message: %v", err) + } + + // now try to create the second pod again + objB, err := registry.Create(testContext, podB.DeepCopy(), rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + if objB.(*example.Pod).Name != "foo2" && objB.(*example.Pod).GenerateName != "foo" { + t.Errorf("Unexpected object: %+v", objB) + } + +} 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 40bf8e20e96..a534467c335 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go @@ -92,7 +92,7 @@ type RESTCreateStrategy interface { } // BeforeCreate ensures that common operations for all resources are performed on creation. It only returns -// errors that can be converted to api.Status. It invokes PrepareForCreate, then GenerateName, then Validate. +// errors that can be converted to api.Status. It invokes PrepareForCreate, then Validate. // It returns nil if the object should be created. func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime.Object) error { objectMeta, kind, kerr := objectMetaAndKind(strategy, obj) @@ -105,6 +105,11 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime. return errors.NewInternalError(fmt.Errorf("system metadata was not initialized")) } + // ensure the name has been generated + if len(objectMeta.GetGenerateName()) > 0 && len(objectMeta.GetName()) == 0 { + return errors.NewInternalError(fmt.Errorf("metadata.name was not generated")) + } + // ensure namespace on the object is correct, or error if a conflicting namespace was set in the object requestNamespace, ok := genericapirequest.NamespaceFrom(ctx) if !ok { @@ -116,10 +121,6 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime. strategy.PrepareForCreate(ctx, obj) - if len(objectMeta.GetGenerateName()) > 0 && len(objectMeta.GetName()) == 0 { - objectMeta.SetName(strategy.GenerateName(objectMeta.GetGenerateName())) - } - // Ensure managedFields is not set unless the feature is enabled if !utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) { objectMeta.SetManagedFields(nil)