Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Race condition in istio NewClientsetFromConfig #34

Open
fernandosanchezs opened this issue Jul 7, 2023 · 2 comments
Open

Race condition in istio NewClientsetFromConfig #34

fernandosanchezs opened this issue Jul 7, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@fernandosanchezs
Copy link

fernandosanchezs commented Jul 7, 2023 •
edited
Loading

NewClientsetFromConfig, located in pkg/api/istio/networking.istio.io/v1beta1/clients.go experiences a race condition due to the use of global variables, such as scheme.Scheme on line 64.

More background and occurrences on the PR here: https://github.com/solo-io/gloo-mesh-enterprise/pull/10267

Example data race

WARNING: DATA RACE
Write at 0x00c00051c150 by goroutine 877:
  runtime.mapassign()
      /home/samu/.go/src/runtime/map.go:578 +0x0
  k8s.io/apimachinery/pkg/runtime.(*Scheme).AddUnversionedTypes()
      /home/samu/go/pkg/mod/k8s.io/[email protected]/pkg/runtime/scheme.go:128 +0x48c
  k8s.io/apimachinery/pkg/apis/meta/v1.AddToGroupVersion()
      /home/samu/go/pkg/mod/k8s.io/[email protected]/pkg/apis/meta/v1/register.go:75 +0xa84
  istio.io/client-go/pkg/apis/networking/v1beta1.addKnownTypes()
      /home/samu/go/pkg/mod/istio.io/[email protected]/pkg/apis/networking/v1beta1/register.gen.go:61 +0x1084
  k8s.io/apimachinery/pkg/runtime.(*SchemeBuilder).AddToScheme()
      /home/samu/go/pkg/mod/k8s.io/[email protected]/pkg/runtime/scheme_builder.go:29 +0xe8
  github.com/solo-io/external-apis/pkg/api/istio/networking.istio.io/v1beta1.NewClientsetFromConfig()
      /home/samu/go/pkg/mod/github.com/solo-io/[email protected]/pkg/api/istio/networking.istio.io/v1beta1/clients.go:65 +0x88
  github.com/solo-io/gloo-mesh-enterprise/test.NewIstioClient()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/env.go:789 +0x234
  github.com/solo-io/gloo-mesh-enterprise/test.(*KubeContext).GetVirtualServices()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/env.go:1177 +0x84
  github.com/solo-io/gloo-mesh-enterprise/test/perf/profiles.VirtualServiceMetric.func1()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/perf/profiles/metrics.go:167 +0xce
  github.com/solo-io/gloo-mesh-enterprise/test/perf/profiles.(*MetricsRecord).To.func1()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/perf/profiles/metrics.go:253 +0x60b
  github.com/solo-io/gloo-mesh-enterprise/test/perf/profiles.(*MetricsRecord).To.func2()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/perf/profiles/metrics.go:291 +0xf6
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /home/samu/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:75 +0x133

Previous read at 0x00c00051c150 by goroutine 873:
  runtime.mapaccess2()
      /home/samu/.go/src/runtime/map.go:456 +0x0
  k8s.io/apimachinery/pkg/runtime.(*Scheme).ObjectKinds()
      /home/samu/go/pkg/mod/k8s.io/[email protected]/pkg/runtime/scheme.go:267 +0x847
  k8s.io/apimachinery/pkg/runtime.(*parameterCodec).EncodeParameters()
      /home/samu/go/pkg/mod/k8s.io/[email protected]/pkg/runtime/codec.go:191 +0xfa
  k8s.io/client-go/rest.(*Request).SpecificallyVersionedParams()
      /home/samu/go/pkg/mod/k8s.io/[email protected]/rest/request.go:372 +0x199
  k8s.io/client-go/rest.(*Request).VersionedParams()
      /home/samu/go/pkg/mod/k8s.io/[email protected]/rest/request.go:365 +0x147
  k8s.io/client-go/kubernetes/typed/core/v1.(*namespaces).List()
      /home/samu/go/pkg/mod/k8s.io/[email protected]/kubernetes/typed/core/v1/namespace.go:90 +0x2f4
  github.com/solo-io/gloo-mesh-enterprise/test.(*KubeContext).GetNamespaces()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/env.go:1125 +0x19d
  github.com/solo-io/gloo-mesh-enterprise/test/perf/profiles.NamespaceMetric.func1()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/perf/profiles/metrics.go:117 +0xab
  github.com/solo-io/gloo-mesh-enterprise/test/perf/profiles.(*MetricsRecord).To.func1()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/perf/profiles/metrics.go:253 +0x60b
  github.com/solo-io/gloo-mesh-enterprise/test/perf/profiles.(*MetricsRecord).To.func2()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/perf/profiles/metrics.go:291 +0xf6
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /home/samu/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:75 +0x133

Goroutine 877 (running) created at:
  golang.org/x/sync/errgroup.(*Group).Go()
      /home/samu/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:72 +0x1ab
  github.com/solo-io/gloo-mesh-enterprise/test/perf/profiles.(*MetricsRecord).To()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/perf/profiles/metrics.go:290 +0x649
  github.com/solo-io/gloo-mesh-enterprise/test/perf_test.glob..func4.3.2()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/perf/perf_test.go:47 +0xcd
  github.com/onsi/ginkgo/v2/internal.extractBodyFunction.func3()
      /home/samu/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/node.go:463 +0x35
  github.com/onsi/ginkgo/v2/internal.(*Suite).runNode.func3()
      /home/samu/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:854 +0x2d7

Goroutine 873 (running) created at:
  golang.org/x/sync/errgroup.(*Group).Go()
      /home/samu/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:72 +0x1ab
  github.com/solo-io/gloo-mesh-enterprise/test/perf/profiles.(*MetricsRecord).To()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/perf/profiles/metrics.go:290 +0x649
  github.com/solo-io/gloo-mesh-enterprise/test/perf_test.glob..func4.3.2()
      /home/samu/github/solo.io/gloo-mesh-enterprise/test/perf/perf_test.go:47 +0xcd
  github.com/onsi/ginkgo/v2/internal.extractBodyFunction.func3()
      /home/samu/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/node.go:463 +0x35
  github.com/onsi/ginkgo/v2/internal.(*Suite).runNode.func3()
      /home/samu/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:854 +0x2d7
@fernandosanchezs fernandosanchezs added the bug Something isn't working label Jul 7, 2023
@marcogschmidt
Copy link

marcogschmidt commented Jul 7, 2023 •
edited
Loading

This is not the first time that we ran into this issue. The bug is actually in skv2, namely in the template that is used the generate the client code, see solo-io/skv2#375.

All the logic to initialize k8s Schemes should reside in an init function in the package for the given group/version, so it is executed exactly once when the package is first used.

@fernandosanchezs
Copy link
Author

@samuelvl has found a workaround using istio clients directly instead of external-apis. Does this solution have any downsides? Do we have a strong preference for our external-apis for some reason?

@fernandosanchezs fernandosanchezs mentioned this issue Jul 10, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@marcogschmidt @fernandosanchezs