-
Notifications
You must be signed in to change notification settings - Fork 41.9k
pkg/registry/flowcontrol: avoid race condition during Create #117107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pkg/registry/flowcontrol: avoid race condition during Create #117107
Conversation
k8s.io/kubernetes/test/integration/controlplane.TestReconcilerAPIServerLeaseMultiCombined suffered from race conditions. The underlying reason is that https://github.com/kubernetes/kubernetes/blob/330b5a2b8dbd681811cb8235947557c99dd8e593/staging/src/k8s.io/apimachinery/pkg/runtime/helper.go#L221-L243 temporarily modifies the object that it is meant to encode. Callers of client-go Create calls must be aware of that and pass in unique object if they might get called concurrently. It's not clear where these goroutines came from, but the data race seems genuine: WARNING: DATA RACE Read at 0x00c0001d66f0 by goroutine 70907: k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/apis/meta/v1.(*TypeMeta).GroupVersionKind() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/apis/meta/v1/meta.go:126 +0x64 k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime.WithVersionEncoder.Encode() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime/helper.go:231 +0x176 k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime.(*WithVersionEncoder).Encode() <autogenerated>:1 +0xfb k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime.Encode() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime/codec.go:50 +0xb3 k8s.io/kubernetes/vendor/k8s.io/client-go/rest.(*Request).Body() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/client-go/rest/request.go:469 +0x884 k8s.io/kubernetes/vendor/k8s.io/client-go/kubernetes/typed/flowcontrol/v1beta3.(*flowSchemas).Create() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/client-go/kubernetes/typed/flowcontrol/v1beta3/flowschema.go:118 +0x23c k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer.(*flowSchemaWrapper).Create() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer/flowschema.go:156 +0x12b k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer.ensureConfiguration() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer/strategy.go:235 +0x147 k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer.(*fsEnsurer).Ensure() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer/flowschema.go:121 +0xd2 k8s.io/kubernetes/pkg/registry/flowcontrol/rest.ensureSuggestedConfiguration() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/rest/storage_flowcontrol.go:211 +0x417 k8s.io/kubernetes/pkg/registry/flowcontrol/rest.ensure() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/rest/storage_flowcontrol.go:186 +0x99 k8s.io/kubernetes/pkg/registry/flowcontrol/rest.(*bootstrapConfigurationEnsurer).ensureAPFBootstrapConfiguration.func1() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/rest/storage_flowcontrol.go:157 +0xb4 k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait.runConditionWithCrashProtectionWithContext() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:154 +0x7b k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait.poll() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait/poll.go:245 +0x57 k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait.PollImmediateUntilWithContext() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait/poll.go:200 +0x59 k8s.io/kubernetes/pkg/registry/flowcontrol/rest.(*bootstrapConfigurationEnsurer).ensureAPFBootstrapConfiguration() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/rest/storage_flowcontrol.go:153 +0x237 k8s.io/kubernetes/pkg/registry/flowcontrol/rest.(*bootstrapConfigurationEnsurer).ensureAPFBootstrapConfiguration-fm() <autogenerated>:1 +0x58 k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.runPostStartHook.func1() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/hooks.go:199 +0xa1 k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.runPostStartHook() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/hooks.go:200 +0xda k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.(*GenericAPIServer).RunPostStartHooks.func2() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/hooks.go:166 +0xb4 Previous write at 0x00c0001d66f0 by goroutine 69811: k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/apis/meta/v1.(*TypeMeta).SetGroupVersionKind() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/apis/meta/v1/meta.go:121 +0x193 k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime.WithVersionEncoder.Encode() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime/helper.go:241 +0x3d9 k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime.(*WithVersionEncoder).Encode() <autogenerated>:1 +0xfb k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime.Encode() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime/codec.go:50 +0xb3 k8s.io/kubernetes/vendor/k8s.io/client-go/rest.(*Request).Body() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/client-go/rest/request.go:469 +0x884 k8s.io/kubernetes/vendor/k8s.io/client-go/kubernetes/typed/flowcontrol/v1beta3.(*flowSchemas).Create() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/client-go/kubernetes/typed/flowcontrol/v1beta3/flowschema.go:118 +0x23c k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer.(*flowSchemaWrapper).Create() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer/flowschema.go:156 +0x12b k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer.ensureConfiguration() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer/strategy.go:235 +0x147 k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer.(*fsEnsurer).Ensure() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/ensurer/flowschema.go:121 +0xd2 k8s.io/kubernetes/pkg/registry/flowcontrol/rest.ensureSuggestedConfiguration() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/rest/storage_flowcontrol.go:211 +0x417 k8s.io/kubernetes/pkg/registry/flowcontrol/rest.ensure() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/rest/storage_flowcontrol.go:186 +0x99 k8s.io/kubernetes/pkg/registry/flowcontrol/rest.(*bootstrapConfigurationEnsurer).ensureAPFBootstrapConfiguration.func1() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/rest/storage_flowcontrol.go:157 +0xb4 k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait.runConditionWithCrashProtectionWithContext() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:154 +0x7b k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait.poll() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait/poll.go:245 +0x57 k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait.PollImmediateUntilWithContext() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/wait/poll.go:200 +0x59 k8s.io/kubernetes/pkg/registry/flowcontrol/rest.(*bootstrapConfigurationEnsurer).ensureAPFBootstrapConfiguration() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/registry/flowcontrol/rest/storage_flowcontrol.go:153 +0x237 k8s.io/kubernetes/pkg/registry/flowcontrol/rest.(*bootstrapConfigurationEnsurer).ensureAPFBootstrapConfiguration-fm() <autogenerated>:1 +0x58 k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.runPostStartHook.func1() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/hooks.go:199 +0xa1 k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.runPostStartHook() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/hooks.go:200 +0xda k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.(*GenericAPIServer).RunPostStartHooks.func2() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/hooks.go:166 +0xb4 Goroutine 70907 (running) created at: k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.(*GenericAPIServer).RunPostStartHooks() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/hooks.go:166 +0x167 k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.preparedGenericAPIServer.NonBlockingRun() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/genericapiserver.go:729 +0x21a k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.preparedGenericAPIServer.Run() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/genericapiserver.go:578 +0x907 k8s.io/kubernetes/vendor/k8s.io/kube-aggregator/pkg/apiserver.preparedAPIAggregator.Run() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go:447 +0xf8 k8s.io/kubernetes/cmd/kube-apiserver/app/testing.StartTestServer.func3() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/testing/testserver.go:260 +0x109 k8s.io/kubernetes/cmd/kube-apiserver/app/testing.StartTestServer.func9() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/testing/testserver.go:263 +0x47 Goroutine 69811 (running) created at: k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.(*GenericAPIServer).RunPostStartHooks() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/hooks.go:166 +0x167 k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.preparedGenericAPIServer.NonBlockingRun() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/genericapiserver.go:729 +0x21a k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.preparedGenericAPIServer.Run() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/genericapiserver.go:578 +0x907 k8s.io/kubernetes/vendor/k8s.io/kube-aggregator/pkg/apiserver.preparedAPIAggregator.Run() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go:447 +0xf8 k8s.io/kubernetes/cmd/kube-apiserver/app/testing.StartTestServer.func3() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/testing/testserver.go:260 +0x109 k8s.io/kubernetes/cmd/kube-apiserver/app/testing.StartTestServer.func9() /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/testing/testserver.go:263 +0x47
|
Please note that we're already in Test Freeze for the Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Wed Apr 5 10:36:42 UTC 2023. |
|
/sig api-machinery |
|
/assign @cici37 |
|
/lgtm |
|
LGTM label has been added. Git tree hash: bb30ae0de5291c04522ce5925a01e9b10048fd69
|
|
/cc @MikeSpreitzer Since you're touching this code as we speak in #111422 /approve /hold |
|
Are we saying that one process is running two servers, each with its own post-start-hooks? If that is so then yes, the side-effects in that Encoder will be a problem and this PR will fix it. |
|
/unhold |
MikeSpreitzer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I understand when there would be two servers running in the same process, but it is legitimate to protect against the possibility of that.
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MikeSpreitzer, pohly, wojtek-t The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
@pohly: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest "[It] [sig-network] Services should fail health check node port if there are only terminating endpoints" |
What type of PR is this?
/kind bug
/kind failing-test
What this PR does / why we need it:
k8s.io/kubernetes/test/integration/controlplane.TestReconcilerAPIServerLeaseMultiCombined suffered from race conditions. The underlying reason is that
kubernetes/staging/src/k8s.io/apimachinery/pkg/runtime/helper.go
Lines 221 to 243 in 330b5a2
Which issue(s) this PR fixes:
Related-to ##82497
It's not clear where these goroutines came from, but the data race seems genuine:
Special notes for your reviewer:
Not sure whether this also could occur in production.
Does this PR introduce a user-facing change?