Merge pull request #122887 from jpbetz/retry-generate-name-create
Implement KEP-4420: Retry Generate Name
This commit is contained in:
		| @@ -258,6 +258,12 @@ const ( | ||||
| 	// Lock to default and remove after v1.22 based on user feedback that should be reflected in KEP #1972 update | ||||
| 	ExecProbeTimeout featuregate.Feature = "ExecProbeTimeout" | ||||
|  | ||||
| 	// owner: @jpbetz | ||||
| 	// alpha: v1.30 | ||||
| 	// Resource create requests using generateName are retried automatically by the apiserver | ||||
| 	// if the generated name conflicts with an existing resource name, up to a maximum number of 7 retries. | ||||
| 	RetryGenerateName featuregate.Feature = "RetryGenerateName" | ||||
|  | ||||
| 	// owner: @bobbypage | ||||
| 	// alpha: v1.20 | ||||
| 	// beta:  v1.21 | ||||
| @@ -980,6 +986,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS | ||||
|  | ||||
| 	ExecProbeTimeout: {Default: true, PreRelease: featuregate.GA}, // lock to default and remove after v1.22 based on KEP #1972 update | ||||
|  | ||||
| 	RetryGenerateName: {Default: false, PreRelease: featuregate.Alpha}, | ||||
|  | ||||
| 	GracefulNodeShutdown: {Default: true, PreRelease: featuregate.Beta}, | ||||
|  | ||||
| 	GracefulNodeShutdownBasedOnPodPriority: {Default: true, PreRelease: featuregate.Beta}, | ||||
|   | ||||
| @@ -189,6 +189,12 @@ const ( | ||||
| 	// clients. | ||||
| 	UnauthenticatedHTTP2DOSMitigation featuregate.Feature = "UnauthenticatedHTTP2DOSMitigation" | ||||
|  | ||||
| 	// owner: @jpbetz | ||||
| 	// alpha: v1.30 | ||||
| 	// Resource create requests using generateName are retried automatically by the apiserver | ||||
| 	// if the generated name conflicts with an existing resource name, up to a maximum number of 7 retries. | ||||
| 	RetryGenerateName featuregate.Feature = "RetryGenerateName" | ||||
|  | ||||
| 	// owner: @caesarxuchao @roycaihw | ||||
| 	// alpha: v1.20 | ||||
| 	// | ||||
| @@ -294,6 +300,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS | ||||
|  | ||||
| 	RemainingItemCount: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32 | ||||
|  | ||||
| 	RetryGenerateName: {Default: false, PreRelease: featuregate.Alpha}, | ||||
|  | ||||
| 	ServerSideApply: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.29 | ||||
|  | ||||
| 	ServerSideFieldValidation: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.29 | ||||
|   | ||||
| @@ -23,6 +23,8 @@ import ( | ||||
| 	"sync" | ||||
| 	"time" | ||||
|  | ||||
| 	"sigs.k8s.io/structured-merge-diff/v4/fieldpath" | ||||
|  | ||||
| 	apierrors "k8s.io/apimachinery/pkg/api/errors" | ||||
| 	"k8s.io/apimachinery/pkg/api/meta" | ||||
| 	"k8s.io/apimachinery/pkg/api/validation" | ||||
| @@ -40,15 +42,16 @@ import ( | ||||
| 	"k8s.io/apimachinery/pkg/watch" | ||||
| 	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" | ||||
| 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request" | ||||
| 	"k8s.io/apiserver/pkg/features" | ||||
| 	"k8s.io/apiserver/pkg/registry/generic" | ||||
| 	"k8s.io/apiserver/pkg/registry/rest" | ||||
| 	"k8s.io/apiserver/pkg/storage" | ||||
| 	storeerr "k8s.io/apiserver/pkg/storage/errors" | ||||
| 	"k8s.io/apiserver/pkg/storage/etcd3/metrics" | ||||
| 	"k8s.io/apiserver/pkg/util/dryrun" | ||||
| 	utilfeature "k8s.io/apiserver/pkg/util/feature" | ||||
| 	flowcontrolrequest "k8s.io/apiserver/pkg/util/flowcontrol/request" | ||||
| 	"k8s.io/client-go/tools/cache" | ||||
| 	"sigs.k8s.io/structured-merge-diff/v4/fieldpath" | ||||
|  | ||||
| 	"k8s.io/klog/v2" | ||||
| ) | ||||
| @@ -392,11 +395,54 @@ func (e *Store) ListPredicate(ctx context.Context, p storage.SelectionPredicate, | ||||
| // finishNothing is a do-nothing FinishFunc. | ||||
| func finishNothing(context.Context, bool) {} | ||||
|  | ||||
| // maxNameGenerationCreateAttempts is the maximum number of | ||||
| // times create will be attempted when generateName is used | ||||
| // and create attempts fails due to name conflict errors. | ||||
| // Each attempt uses a newly randomly generated name. | ||||
| // 8 was selected as the max because it is sufficient to generate | ||||
| // 1 million names per generateName prefix, with only a 0.1% | ||||
| // probability of any generated name conflicting with existing names. | ||||
| // Without retry, a 0.1% probability occurs at ~500 | ||||
| // generated names and a 50% probability occurs at ~4500 | ||||
| // generated names. | ||||
| const maxNameGenerationCreateAttempts = 8 | ||||
|  | ||||
| // Create inserts a new item according to the unique key from the object. | ||||
| // Note that registries may mutate the input object (e.g. in the strategy | ||||
| // hooks).  Tests which call this might want to call DeepCopy if they expect to | ||||
| // be able to examine the input and output objects for differences. | ||||
| func (e *Store) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { | ||||
| 	if utilfeature.DefaultFeatureGate.Enabled(features.RetryGenerateName) && needsNameGeneration(obj) { | ||||
| 		return e.createWithGenerateNameRetry(ctx, obj, createValidation, options) | ||||
| 	} | ||||
|  | ||||
| 	return e.create(ctx, obj, createValidation, options) | ||||
| } | ||||
|  | ||||
| // needsNameGeneration returns true if the obj has a generateName but no name. | ||||
| func needsNameGeneration(obj runtime.Object) bool { | ||||
| 	if objectMeta, err := meta.Accessor(obj); err == nil { | ||||
| 		if len(objectMeta.GetGenerateName()) > 0 && len(objectMeta.GetName()) == 0 { | ||||
| 			return true | ||||
| 		} | ||||
| 	} | ||||
| 	return false | ||||
| } | ||||
|  | ||||
| // createWithGenerateNameRetry attempts to create obj up to maxNameGenerationCreateAttempts | ||||
| // when create fails due to a name conflict error. Each attempt randomly generates a new | ||||
| // name based on generateName. | ||||
| func (e *Store) createWithGenerateNameRetry(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (resultObj runtime.Object, err error) { | ||||
| 	for i := 0; i < maxNameGenerationCreateAttempts; i++ { | ||||
| 		resultObj, err = e.create(ctx, obj.DeepCopyObject(), createValidation, options) | ||||
| 		if err == nil || !apierrors.IsAlreadyExists(err) { | ||||
| 			return resultObj, err | ||||
| 		} | ||||
| 	} | ||||
| 	return resultObj, err | ||||
| } | ||||
|  | ||||
| func (e *Store) create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { | ||||
| 	var finishCreate FinishFunc = finishNothing | ||||
|  | ||||
| 	// Init metadata as early as possible. | ||||
|   | ||||
| @@ -30,6 +30,7 @@ import ( | ||||
| 	"time" | ||||
|  | ||||
| 	fuzz "github.com/google/gofuzz" | ||||
|  | ||||
| 	"k8s.io/apimachinery/pkg/api/apitesting" | ||||
| 	apiequality "k8s.io/apimachinery/pkg/api/equality" | ||||
| 	"k8s.io/apimachinery/pkg/api/errors" | ||||
| @@ -48,6 +49,7 @@ import ( | ||||
| 	"k8s.io/apiserver/pkg/apis/example" | ||||
| 	examplev1 "k8s.io/apiserver/pkg/apis/example/v1" | ||||
| 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request" | ||||
| 	"k8s.io/apiserver/pkg/features" | ||||
| 	"k8s.io/apiserver/pkg/registry/generic" | ||||
| 	"k8s.io/apiserver/pkg/registry/rest" | ||||
| 	"k8s.io/apiserver/pkg/storage" | ||||
| @@ -56,7 +58,9 @@ import ( | ||||
| 	"k8s.io/apiserver/pkg/storage/names" | ||||
| 	"k8s.io/apiserver/pkg/storage/storagebackend/factory" | ||||
| 	storagetesting "k8s.io/apiserver/pkg/storage/testing" | ||||
| 	utilfeature "k8s.io/apiserver/pkg/util/feature" | ||||
| 	"k8s.io/client-go/tools/cache" | ||||
| 	featuregatetesting "k8s.io/component-base/featuregate/testing" | ||||
| ) | ||||
|  | ||||
| var scheme = runtime.NewScheme() | ||||
| @@ -383,6 +387,95 @@ func TestStoreCreate(t *testing.T) { | ||||
| 	} | ||||
| } | ||||
|  | ||||
| // sequentialNameGenerator generates names by appending a monotonically-increasing integer to the base. | ||||
| type sequentialNameGenerator struct { | ||||
| 	seq int | ||||
| } | ||||
|  | ||||
| func (m *sequentialNameGenerator) GenerateName(base string) string { | ||||
| 	generated := fmt.Sprintf("%s%d", base, m.seq) | ||||
| 	m.seq++ | ||||
| 	return generated | ||||
| } | ||||
|  | ||||
| func TestStoreCreateWithRetryNameGenerate(t *testing.T) { | ||||
| 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RetryGenerateName, true)() | ||||
|  | ||||
| 	namedObj := func(id int) *example.Pod { | ||||
| 		return &example.Pod{ | ||||
| 			ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("prefix-%d", id), Namespace: "test"}, | ||||
| 			Spec:       example.PodSpec{NodeName: "machine"}, | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	generateNameObj := &example.Pod{ | ||||
| 		ObjectMeta: metav1.ObjectMeta{GenerateName: "prefix-", Namespace: "test"}, | ||||
| 		Spec:       example.PodSpec{NodeName: "machine"}, | ||||
| 	} | ||||
|  | ||||
| 	testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test") | ||||
| 	destroyFunc, registry := NewTestGenericStoreRegistry(t) | ||||
| 	defer destroyFunc() | ||||
|  | ||||
| 	seqNameGenerator := &sequentialNameGenerator{} | ||||
| 	registry.CreateStrategy = &testRESTStrategy{scheme, seqNameGenerator, true, false, true} | ||||
|  | ||||
| 	for i := 0; i < 7; i++ { | ||||
| 		_, err := registry.Create(testContext, namedObj(i), rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) | ||||
| 		if err != nil { | ||||
| 			t.Errorf("Unexpected error: %v", err) | ||||
| 		} | ||||
| 	} | ||||
| 	generated, err := registry.Create(testContext, generateNameObj, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) | ||||
| 	if err != nil { | ||||
| 		t.Errorf("Unexpected error: %v", err) | ||||
| 	} | ||||
| 	generatedMeta, err := meta.Accessor(generated) | ||||
| 	if err != nil { | ||||
| 		t.Fatal(err) | ||||
| 	} | ||||
| 	if generatedMeta.GetName() != "prefix-7" { | ||||
| 		t.Errorf("Expected prefix-7 but got %s", generatedMeta.GetName()) | ||||
| 	} | ||||
|  | ||||
| 	// Now that 8 generated names (0..7) are claimed, 8 name generation attempts will not be enough | ||||
| 	// and create should return an already exists error. | ||||
| 	seqNameGenerator.seq = 0 | ||||
| 	_, err = registry.Create(testContext, generateNameObj, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) | ||||
| 	if err == nil || !errors.IsAlreadyExists(err) { | ||||
| 		t.Error("Expected already exists error") | ||||
| 	} | ||||
| } | ||||
|  | ||||
| func TestStoreCreateWithRetryNameGenerateFeatureDisabled(t *testing.T) { | ||||
| 	namedObj := func(id int) *example.Pod { | ||||
| 		return &example.Pod{ | ||||
| 			ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("prefix-%d", id), Namespace: "test"}, | ||||
| 			Spec:       example.PodSpec{NodeName: "machine"}, | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	generateNameObj := &example.Pod{ | ||||
| 		ObjectMeta: metav1.ObjectMeta{GenerateName: "prefix-", Namespace: "test"}, | ||||
| 		Spec:       example.PodSpec{NodeName: "machine"}, | ||||
| 	} | ||||
|  | ||||
| 	testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test") | ||||
| 	destroyFunc, registry := NewTestGenericStoreRegistry(t) | ||||
| 	defer destroyFunc() | ||||
|  | ||||
| 	registry.CreateStrategy = &testRESTStrategy{scheme, &sequentialNameGenerator{}, true, false, true} | ||||
|  | ||||
| 	_, err := registry.Create(testContext, namedObj(0), rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) | ||||
| 	if err != nil { | ||||
| 		t.Errorf("Unexpected error: %v", err) | ||||
| 	} | ||||
| 	_, err = registry.Create(testContext, generateNameObj, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) | ||||
| 	if err == nil || !errors.IsAlreadyExists(err) { | ||||
| 		t.Error("Expected already exists error") | ||||
| 	} | ||||
| } | ||||
|  | ||||
| func TestNewCreateOptionsFromUpdateOptions(t *testing.T) { | ||||
| 	f := fuzz.New().NilChance(0.0).NumElements(1, 1) | ||||
|  | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Kubernetes Prow Robot
					Kubernetes Prow Robot