From 5e1c6cd0d4ffb03234b32208b5f1158b02c75fe2 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 5 Apr 2023 10:04:35 +0200 Subject: [PATCH] pkg/registry/flowcontrol: avoid race condition during Create 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() :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() :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() :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() :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 --- pkg/registry/flowcontrol/ensurer/flowschema.go | 5 ++++- .../flowcontrol/ensurer/prioritylevelconfiguration.go | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/registry/flowcontrol/ensurer/flowschema.go b/pkg/registry/flowcontrol/ensurer/flowschema.go index 4dbfb19c456..6cad99ffd91 100644 --- a/pkg/registry/flowcontrol/ensurer/flowschema.go +++ b/pkg/registry/flowcontrol/ensurer/flowschema.go @@ -118,7 +118,10 @@ type fsEnsurer struct { func (e *fsEnsurer) Ensure(flowSchemas []*flowcontrolv1beta3.FlowSchema) error { for _, flowSchema := range flowSchemas { - if err := ensureConfiguration(e.wrapper, e.strategy, flowSchema); err != nil { + // This code gets called by different goroutines. To avoid race conditions when + // https://github.com/kubernetes/kubernetes/blob/330b5a2b8dbd681811cb8235947557c99dd8e593/staging/src/k8s.io/apimachinery/pkg/runtime/helper.go#L221-L243 + // temporarily modifies the TypeMeta, we have to make a copy here. + if err := ensureConfiguration(e.wrapper, e.strategy, flowSchema.DeepCopy()); err != nil { return err } } diff --git a/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration.go b/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration.go index 4da5bc92c4c..a536d649d4c 100644 --- a/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration.go +++ b/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration.go @@ -119,7 +119,10 @@ type plEnsurer struct { func (e *plEnsurer) Ensure(priorityLevels []*flowcontrolv1beta3.PriorityLevelConfiguration) error { for _, priorityLevel := range priorityLevels { - if err := ensureConfiguration(e.wrapper, e.strategy, priorityLevel); err != nil { + // This code gets called by different goroutines. To avoid race conditions when + // https://github.com/kubernetes/kubernetes/blob/330b5a2b8dbd681811cb8235947557c99dd8e593/staging/src/k8s.io/apimachinery/pkg/runtime/helper.go#L221-L243 + // temporarily modifies the TypeMeta, we have to make a copy here. + if err := ensureConfiguration(e.wrapper, e.strategy, priorityLevel.DeepCopy()); err != nil { return err } }