From c5cc58d0de6663627f4826ad808d82ea38b28635 Mon Sep 17 00:00:00 2001 From: googs1025 Date: Sun, 1 Jun 2025 13:44:23 +0800 Subject: [PATCH] fix: data race for patchResource func Signed-off-by: googs1025 Kubernetes-commit: 2fd93c0898856ba764adef8efcaace9ff0c49f23 --- pkg/endpoints/handlers/patch.go | 8 +++++ pkg/endpoints/handlers/rest_test.go | 48 +++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/pkg/endpoints/handlers/patch.go b/pkg/endpoints/handlers/patch.go index 0dc9ffb33..060c7cc30 100644 --- a/pkg/endpoints/handlers/patch.go +++ b/pkg/endpoints/handlers/patch.go @@ -726,6 +726,14 @@ func (p *patcher) patchResource(ctx context.Context, scope *RequestScope) (runti } return result, err }) + + // In case of a timeout error, the goroutine handling the request is still running. + // https://github.com/kubernetes/kubernetes/blob/d2c12afa4593e50a187075157d38748292b02733/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/finisher/finisher.go#L127-L146 + // We cannot reliably read the variable (data race!) and have to assume that + // the object was not created. + if errors.IsTimeout(err) { + return result, false, err + } return result, wasCreated, err } diff --git a/pkg/endpoints/handlers/rest_test.go b/pkg/endpoints/handlers/rest_test.go index 7f4b1f3e6..1429977fc 100644 --- a/pkg/endpoints/handlers/rest_test.go +++ b/pkg/endpoints/handlers/rest_test.go @@ -323,6 +323,25 @@ func TestPatchCustomResource(t *testing.T) { } } +type testTimeoutPatcher struct { + t *testing.T +} + +func (p *testTimeoutPatcher) New() runtime.Object { + return &example.Pod{} +} + +func (p *testTimeoutPatcher) Update(ctx context.Context, _ string, _ rest.UpdatedObjectInfo, _ rest.ValidateObjectFunc, _ rest.ValidateObjectUpdateFunc, _ bool, _ *metav1.UpdateOptions) (runtime.Object, bool, error) { + // Block until the context is canceled to simulate a timeout scenario. + <-ctx.Done() + return nil, false, ctx.Err() +} + +func (p *testTimeoutPatcher) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { + p.t.Fatal("unexpected call to testPatcher.Get") + return nil, errors.New("unexpected call to testPatcher.Get") +} + type testPatcher struct { t *testing.T @@ -432,6 +451,9 @@ type patchTestCase struct { expectedError string // if set, indicates the number of times patching was expected to be attempted expectedTries int + + // isTimeout for this test case + isTimeout bool } func (tc *patchTestCase) Run(t *testing.T) { @@ -564,8 +586,16 @@ func (tc *patchTestCase) Run(t *testing.T) { FieldManager: "test-manager", }, } - - ctx, cancel := context.WithTimeout(ctx, time.Second) + var timeout time.Duration + if tc.isTimeout { + // Simulate timeout by using a zero-duration timeout and the timeout patcher + // replace testPatcher with the timeout simulator + timeout = 0 + p.restPatcher = &testTimeoutPatcher{t: t} + } else { + timeout = time.Second + } + ctx, cancel := context.WithTimeout(ctx, timeout) resultObj, _, err := p.patchResource(ctx, &RequestScope{ FieldManager: fieldmanager, }) @@ -679,6 +709,20 @@ func TestPatchResourceNumberConversion(t *testing.T) { tc.Run(t) } +func TestPatchResourceTimeout(t *testing.T) { + tc := &patchTestCase{ + name: "TestPatchResourceTimeout", + startingPod: &example.Pod{}, + changedPod: &example.Pod{}, + updatePod: &example.Pod{}, + expectedPod: nil, + isTimeout: true, + expectedError: "Timeout: request did not complete within requested timeout - context deadline exceeded", + } + + tc.Run(t) +} + func TestPatchResourceWithVersionConflict(t *testing.T) { namespace := "bar" name := "foo"