github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add retry around create

Browse Source
This commit is contained in:
Joe Betz
2024-01-19 16:10:30 -05:00
parent 48b68582b8
commit a05db0dd22
4 changed files with 156 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
pkg/features/kube_features.go
View File

@@ -257,6 +257,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
@@ -983,6 +989,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},

8
staging/src/k8s.io/apiserver/pkg/features/kube_features.go
View File

@@ -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
//
@@ -293,6 +299,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

48
staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go
View File

@@ -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.

93
staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go
View File

@@ -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)