kustomize
kustomize copied to clipboard
api: data race when calling `Kustomizer.Run()` from multiple goroutines
Describe the bug
Data race occurs when Kustomizer.Run() is called from multiple goroutines.
The culprit seems to be in openapi.SetSchema() which manipulates the global variable: kubernetesOpenAPIVersion
Files that can reproduce the issue
k := krusty.MakeKustomizer(krusty.MakeDefaultOptions())
m, err := k.Run(filesys.MakeFsOnDisk(), path)
Expected output
No data race
Actual output
Data race.
Stack trace:
2022-10-07T00:06:29.6668305Z ==================
2022-10-07T00:06:29.6831615Z WARNING: DATA RACE
2022-10-07T00:06:29.7090851Z Read at 0x0000048e6d00 by goroutine 2050:
2022-10-07T00:06:29.7091666Z sigs.k8s.io/kustomize/kyaml/openapi.SetSchema()
2022-10-07T00:06:29.7092542Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/openapi/openapi.go:555 +0x55
2022-10-07T00:06:29.7093591Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateDirectory()
2022-10-07T00:06:29.7094450Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:489 +0x419
2022-10-07T00:06:29.7095362Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateResources()
2022-10-07T00:06:29.7097449Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:429 +0x3d3
2022-10-07T00:06:29.7098652Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateTarget()
2022-10-07T00:06:29.7099520Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:188 +0xac
2022-10-07T00:06:29.7100663Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).AccumulateTarget()
2022-10-07T00:06:29.7104983Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:181 +0x22b
2022-10-07T00:06:29.7120296Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).makeCustomizedResMap()
2022-10-07T00:06:29.7121222Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:127 +0x117
2022-10-07T00:06:29.7121958Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).MakeCustomizedResMap()
2022-10-07T00:06:29.7125031Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:118 +0x456
2022-10-07T00:06:29.7125612Z sigs.k8s.io/kustomize/api/krusty.(*Kustomizer).Run()
2022-10-07T00:06:29.7126358Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/krusty/kustomizer.go:88 +0x449
2022-10-07T00:06:29.7127459Z github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl.runKustomize()
2022-10-07T00:06:29.7128575Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl/kustomize.go:53 +0x218
2022-10-07T00:06:29.7129466Z github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl.GetKustomizedManifest()
2022-10-07T00:06:29.7131247Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl/kustomize.go:43 +0x287
2022-10-07T00:06:29.7132361Z github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.getManifest()
2022-10-07T00:06:29.7133485Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:328 +0x3ea
2022-10-07T00:06:29.7134380Z github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.metallbDeployHack()
2022-10-07T00:06:29.7135506Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:363 +0x1f3
2022-10-07T00:06:29.7136465Z github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.deployMetallbForKindCluster()
2022-10-07T00:06:29.7137799Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:167 +0x1fa
2022-10-07T00:06:29.7138663Z github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.(*addon).Deploy()
2022-10-07T00:06:29.7139930Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:84 +0xed
2022-10-07T00:06:29.7140787Z github.com/kong/kubernetes-testing-framework/pkg/clusters/types/kind.(*Cluster).DeployAddon()
2022-10-07T00:06:29.7141847Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/types/kind/cluster.go:122 +0x1dc
2022-10-07T00:06:29.7142650Z github.com/kong/kubernetes-testing-framework/pkg/environments.(*Builder).Build.func1()
2022-10-07T00:06:29.7143664Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/environments/builder.go:154 +0x9b
2022-10-07T00:06:29.7144051Z
2022-10-07T00:06:29.7144252Z Previous write at 0x0000048e6d00 by goroutine 2027:
2022-10-07T00:06:29.7144698Z sigs.k8s.io/kustomize/kyaml/openapi.SetSchema()
2022-10-07T00:06:29.7145375Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/openapi/openapi.go:574 +0x245
2022-10-07T00:06:29.7145947Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateDirectory()
2022-10-07T00:06:29.7146722Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:489 +0x419
2022-10-07T00:06:29.7147538Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateResources()
2022-10-07T00:06:29.7148389Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:429 +0x3d3
2022-10-07T00:06:29.7148959Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateTarget()
2022-10-07T00:06:29.7149698Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:188 +0xac
2022-10-07T00:06:29.7150293Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).AccumulateTarget()
2022-10-07T00:06:29.7151025Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:181 +0x22b
2022-10-07T00:06:29.7151609Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateDirectory()
2022-10-07T00:06:29.7152348Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:509 +0xeb3
2022-10-07T00:06:29.7152949Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateResources()
2022-10-07T00:06:29.7153697Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:429 +0x3d3
2022-10-07T00:06:29.7154267Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateTarget()
2022-10-07T00:06:29.7154997Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:188 +0xac
2022-10-07T00:06:29.7155568Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).AccumulateTarget()
2022-10-07T00:06:29.7156315Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:181 +0x22b
2022-10-07T00:06:29.7156898Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).makeCustomizedResMap()
2022-10-07T00:06:29.7157641Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:127 +0x117
2022-10-07T00:06:29.7158215Z sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).MakeCustomizedResMap()
2022-10-07T00:06:29.7198895Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/internal/target/kusttarget.go:118 +0x456
2022-10-07T00:06:29.7199528Z sigs.k8s.io/kustomize/api/krusty.(*Kustomizer).Run()
2022-10-07T00:06:29.7200240Z /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/[email protected]/krusty/kustomizer.go:88 +0x449
2022-10-07T00:06:29.7201098Z github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl.runKustomize()
2022-10-07T00:06:29.7202426Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl/kustomize.go:53 +0x218
2022-10-07T00:06:29.7203378Z github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl.GetKustomizedManifest()
2022-10-07T00:06:29.7204536Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl/kustomize.go:43 +0x287
2022-10-07T00:06:29.7205404Z github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.getManifest()
2022-10-07T00:06:29.7206521Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:328 +0x3ea
2022-10-07T00:06:29.7207434Z github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.metallbDeployHack()
2022-10-07T00:06:29.7208549Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:363 +0x1f3
2022-10-07T00:06:29.7209613Z github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.deployMetallbForKindCluster()
2022-10-07T00:06:29.7210768Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:167 +0x1fa
2022-10-07T00:06:29.7211662Z github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.(*addon).Deploy()
2022-10-07T00:06:29.7212749Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:84 +0xed
2022-10-07T00:06:29.7213829Z github.com/kong/kubernetes-testing-framework/pkg/clusters/types/kind.(*Cluster).DeployAddon()
2022-10-07T00:06:29.7215033Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/types/kind/cluster.go:122 +0x1dc
2022-10-07T00:06:29.7215905Z github.com/kong/kubernetes-testing-framework/pkg/environments.(*Builder).Build.func1()
2022-10-07T00:06:29.7217602Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/environments/builder.go:154 +0x9b
2022-10-07T00:06:29.7218319Z
2022-10-07T00:06:29.7218658Z Goroutine 2050 (running) created at:
2022-10-07T00:06:29.7219484Z github.com/kong/kubernetes-testing-framework/pkg/environments.(*Builder).Build()
2022-10-07T00:06:29.7220555Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/environments/builder.go:153 +0xd98
2022-10-07T00:06:29.7221424Z github.com/kong/kubernetes-testing-framework/test/integration.TestKindClusterProxyOnly()
2022-10-07T00:06:29.7222550Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/test/integration/kind_cluster_test.go:102 +0xe99
2022-10-07T00:06:29.7223408Z testing.tRunner()
2022-10-07T00:06:29.7224002Z /opt/hostedtoolcache/go/1.19.1/x64/src/testing/testing.go:1446 +0x216
2022-10-07T00:06:29.7224447Z testing.(*T).Run.func1()
2022-10-07T00:06:29.7225108Z /opt/hostedtoolcache/go/1.19.1/x64/src/testing/testing.go:1493 +0x47
2022-10-07T00:06:29.7225366Z
2022-10-07T00:06:29.7225532Z Goroutine 2027 (running) created at:
2022-10-07T00:06:29.7226232Z github.com/kong/kubernetes-testing-framework/pkg/environments.(*Builder).Build()
2022-10-07T00:06:29.7227458Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/environments/builder.go:153 +0xd98
2022-10-07T00:06:29.7228352Z github.com/kong/kubernetes-testing-framework/test/integration.TestKongEnterprisePostgres()
2022-10-07T00:06:29.7229484Z /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/test/integration/enterprise_test.go:46 +0xfca
2022-10-07T00:06:29.7230018Z testing.tRunner()
2022-10-07T00:06:29.7230689Z /opt/hostedtoolcache/go/1.19.1/x64/src/testing/testing.go:1446 +0x216
2022-10-07T00:06:29.7231132Z testing.(*T).Run.func1()
2022-10-07T00:06:29.7231711Z /opt/hostedtoolcache/go/1.19.1/x64/src/testing/testing.go:1493 +0x47
Kustomize version
sigs.k8s.io/kustomize/api v0.12.1
Additional context
Related issue: https://github.com/Kong/kubernetes-testing-framework/issues/404
If that's the intended behaviour then at the very least the API should have a note saying that this cannot be called from multiple goroutines.
@pmalek: This issue is currently awaiting triage.
SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.
The triage/accepted label can be added by org members by writing /triage accepted in a comment.
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.
This package https://github.com/kubernetes-sigs/kustomize/blob/2ec4b971e91ec92cb5a087bd925b3f95ac196040/kyaml/openapi/openapi.go#L28 uses global variables. It sets and reads kubernetesOpenAPIVersion in many places.
I agree with your assessment of why this is happening, and that ideally we wouldn't use global variables (these days we have a linter in place to flag them to help prevent problems like this from being introduced). I suspect the fix may be quite involved though, as performance is a big issue with openapi; we'd likely need the Kustomize layer to start holding the state the global variable is being used for, and any PR would need to have performance benchmarks. I'll accept the issue, but for transparency I don't think this is likely to be taken on by the Kustomize team ourselves; our bandwidth is very limited and this doesn't affect the kustomize or kubectl kustomize CLIs themselves.
/triage accepted /help
@KnVerey: This request has been marked as needing help from a contributor.
Guidelines
Please ensure that the issue body includes answers to the following questions:
- Why are we solving this issue?
- To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
- Does this issue have zero to low barrier of entry?
- How can the assignee reach out to you for help?
For more details on the requirements of such an issue, please see here and ensure that they are met.
If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.
In response to this:
I agree with your assessment of why this is happening, and that ideally we wouldn't use global variables (these days we have a linter in place to flag them to help prevent problems like this from being introduced). I suspect the fix may be quite involved though, as performance is a big issue with openapi; we'd likely need the Kustomize layer to start holding the state the global variable is being used for, and any PR would need to have performance benchmarks. I'll accept the issue, but for transparency I don't think this is likely to be taken on by the Kustomize team ourselves; our bandwidth is very limited and this doesn't affect the
kustomizeorkubectl kustomizeCLIs themselves./triage accepted /help
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.
Understood. Thanks for your response. How would you feel about introducing a lock on that variable, and making it be accessed/set from a func that controls this lock? Wouldn't that help?
Yes, you're right, that should also work and I would be happy to accept a contribution to that effect. If you're interested, please /assign yourself to this issue. 😄
Submitted #4967 @KnVerey. PTAL