Skip to content

test(controller): the DataVolume watch tests race on FakeInformers.Scheme under -race #5296

Open

Description

The DataVolume watch tests in the workload monitor controller fail under the race detector on main. With go1.27.2, go test -race ./internal/controller/ on 238b3ee reports three data races and fails TestTryStartDataVolumeWatch_UnsyncedSourceLeavesNoReaderAndRetries and TestTryStartDataVolumeWatch_ReconcileBeforeTheReaderIsRetried. The result is the same with controller-runtime v0.25.1 and v0.25.2. Without -race the package passes, and CI does not run the race detector, so nothing reports it today.

All three reports are about the same field, FakeInformers.Scheme:

Read at 0x00c0010d4ff8 by goroutine 246:
  sigs.k8s.io/controller-runtime/pkg/cache/informertest.(*FakeInformers).GetInformer()
      .../informertest/fake_cache.go:65
  sigs.k8s.io/controller-runtime/pkg/internal/source.(*Kind[...]).Start.func1.1()
      .../internal/source/kind.go:88

Previous write at 0x00c0010d4ff8 by goroutine 244:
  sigs.k8s.io/controller-runtime/pkg/cache/informertest.(*FakeInformers).GetInformer()
      .../informertest/fake_cache.go:66
  github.com/cozystack/cozystack/internal/controller.(*WorkloadMonitorReconciler).tryStartDataVolumeWatch()
      internal/controller/workloadmonitor_controller.go:257

Goroutine 246 (running) created at:
  sigs.k8s.io/controller-runtime/pkg/internal/source.(*Kind[...]).Start()
      .../internal/source/kind.go:78
  github.com/cozystack/cozystack/internal/controller.(*startingWatcher).Watch()
      internal/controller/workloadmonitor_controller_test.go:2021

The fake cache sets its scheme lazily, without a lock. FakeInformers.GetInformer assigns c.Scheme = scheme.Scheme when the field is nil. startingWatcher.Watch starts the source.Kind, and its goroutine calls GetInformer. At the same time tryStartDataVolumeWatch calls GetInformer on the test goroutine at line 257. dataVolumeInformers() fills only InformersByGVK, so Scheme is nil and both goroutines hit the write.

There are two ways to read this, and the trace points to the first one:

  1. The race is in the test harness. The fake cache is not safe for concurrent use on its first call, the manager's real cache does not have this lazy write, and the only racing address in the reports is FakeInformers.Scheme.
  2. The race is in the controller. tryStartDataVolumeWatch sets dataVolumeWatchStarted without the lock. None of the reports touches a reconciler field, though, and watchDataVolumes calls it from a single loop, so this needs two concurrent callers that I don't see.

Setting Scheme: scheme.Scheme in dataVolumeInformers() should fix it, because then the fake never writes the field during a test. To check, run go test -race -count=1 ./internal/controller/ -run TestTryStartDataVolumeWatch a few times, since the race depends on timing. Adding -race to go-unit-tests or test-controllers would catch the next one, but it costs CI time and is a separate decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    triage/needs-triageIndicates an issue needs triage by a maintainer

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions