github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-31 15:25:57 +00:00
Code Issues Projects Releases Wiki Activity

apiserver: fix data race in etcd metrics

Browse Source 
7a63997c8a added a global variable which gets set multiple times by
different goroutines in integration tests, leading to a data race:

WARNING: DATA RACE
Write at 0x00000a626928 by goroutine 87080:
  k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/storage/etcd3/metrics.SetStorageMonitorGetter()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/storage/etcd3/metrics/metrics.go:231 +0x104
  k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/options.(*EtcdOptions).ApplyWithStorageFactoryTo()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/options/etcd.go:242 +0xbd
  k8s.io/kubernetes/pkg/controlplane/apiserver.BuildGenericConfig()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/controlplane/apiserver/config.go:124 +0x1c3d
  k8s.io/kubernetes/cmd/kube-apiserver/app.CreateKubeAPIServerConfig()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/server.go:218 +0xeb
  k8s.io/kubernetes/cmd/kube-apiserver/app.NewConfig()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/config.go:74 +0xd5
  k8s.io/kubernetes/cmd/kube-apiserver/app/testing.StartTestServer()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/testing/testserver.go:299 +0x2e97
  k8s.io/kubernetes/cmd/kube-apiserver/app/testing.StartTestServerOrDie()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/testing/testserver.go:423 +0xb2
  k8s.io/kubernetes/test/integration/controlplane.testReconcilersAPIServerLease.func3()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/integration/controlplane/kube_apiserver_test.go:486 +0x1dd
  k8s.io/kubernetes/test/integration/controlplane.testReconcilersAPIServerLease.func7()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/integration/controlplane/kube_apiserver_test.go:488 +0x47

Previous write at 0x00000a626928 by goroutine 87079:
  k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/storage/etcd3/metrics.SetStorageMonitorGetter()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/storage/etcd3/metrics/metrics.go:231 +0x104
  k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/options.(*EtcdOptions).ApplyWithStorageFactoryTo()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/options/etcd.go:242 +0xbd
  k8s.io/kubernetes/pkg/controlplane/apiserver.BuildGenericConfig()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/controlplane/apiserver/config.go:124 +0x1c3d
  k8s.io/kubernetes/cmd/kube-apiserver/app.CreateKubeAPIServerConfig()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/server.go:218 +0xeb
  k8s.io/kubernetes/cmd/kube-apiserver/app.NewConfig()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/config.go:74 +0xd5
  k8s.io/kubernetes/cmd/kube-apiserver/app/testing.StartTestServer()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/testing/testserver.go:299 +0x2e97
  k8s.io/kubernetes/cmd/kube-apiserver/app/testing.StartTestServerOrDie()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kube-apiserver/app/testing/testserver.go:423 +0xb2
  k8s.io/kubernetes/test/integration/controlplane.testReconcilersAPIServerLease.func3()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/integration/controlplane/kube_apiserver_test.go:486 +0x1dd
  k8s.io/kubernetes/test/integration/controlplane.testReconcilersAPIServerLease.func7()
      /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/integration/controlplane/kube_apiserver_test.go:488 +0x47

Mutex locking avoids the data race. Whether this variable really can be used
safely by those concurrent (?) tests is a different question...
This commit is contained in:
Patrick Ohly 2023-08-25 10:42:17 +02:00
parent 10c622e99a
commit 13a8ad12b8
1 changed files with 15 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
staging/src/k8s.io/apiserver/pkg/storage/etcd3/metrics/metrics.go
View File

@ -228,7 +228,7 @@ func UpdateEtcdDbSize(ep string, size int64) {
// SetStorageMonitorGetter sets monitor getter to allow monitoring etcd stats.
func SetStorageMonitorGetter(getter func() ([]Monitor, error)) {
storageMonitor.monitorGetter = getter
storageMonitor.setGetter(getter)
}
// UpdateLeaseObjectCount sets the etcd_lease_object_counts metric.
@ -258,9 +258,22 @@ type StorageMetrics struct {
type monitorCollector struct {
compbasemetrics.BaseStableCollector
mutex sync.Mutex
monitorGetter func() ([]Monitor, error)
}
func (m *monitorCollector) setGetter(monitorGetter func() ([]Monitor, error)) {
m.mutex.Lock()
defer m.mutex.Unlock()
m.monitorGetter = monitorGetter
}
func (m *monitorCollector) getGetter() func() ([]Monitor, error) {
m.mutex.Lock()
defer m.mutex.Unlock()
return m.monitorGetter
}
// DescribeWithStability implements compbasemetrics.StableColletor
func (c *monitorCollector) DescribeWithStability(ch chan<- *compbasemetrics.Desc) {
ch <- storageSizeDescription
@ -268,7 +281,7 @@ func (c *monitorCollector) DescribeWithStability(ch chan<- *compbasemetrics.Desc
// CollectWithStability implements compbasemetrics.StableColletor
func (c *monitorCollector) CollectWithStability(ch chan<- compbasemetrics.Metric) {
monitors, err := c.monitorGetter()
monitors, err := c.getGetter()()
if err != nil {
return
}