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.

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

tests: fix data race in KongConsumer data race tests #692

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Oct 3, 2024

What this PR does / why we need it:

An attempt at solving the data race in KongConsumer envtest suite.

Example: https://github.com/Kong/gateway-operator/actions/runs/11162590333/job/31027578476?pr=687#step:5:440

==================
WARNING: DATA RACE
Read at 0x00c000c88708 by goroutine 4747:
  github.com/kong/gateway-operator/test/envtest.TestKongConsumer.func1.5()
      /home/runner/work/gateway-operator/gateway-operator/test/envtest/konnect_entities_kongconsumer_test.go:116 +0x7c
  runtime.call128()
      /home/runner/go/pkg/mod/golang.org/[email protected]/src/runtime/asm_amd64.s:773 +0x57
  reflect.Value.Call()
      /home/runner/go/pkg/mod/golang.org/[email protected]/src/reflect/value.go:380 +0xb5
  github.com/stretchr/testify/mock.argumentMatcher.Matches()
      /home/runner/go/pkg/mod/github.com/stretchr/[email protected]/mock/mock.go:860 +0x2e4
  github.com/stretchr/testify/mock.Arguments.Diff.func1()
      /home/runner/work/gateway-operator/gateway-operator/controller/konnect/reconciler_generic.go:536 +0x5cf2
  github.com/kong/gateway-operator/controller/konnect.(*KonnectEntityReconciler[github.com/kong/kubernetes-configuration/api/configuration/v1.KongConsumer,*github.com/kong/kubernetes-configuration/api/configuration/v1.KongConsumer]).Reconcile()
      /home/runner/work/gateway-operator/gateway-operator/controller/konnect/reconciler_generic.go:114 +0xa4
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[go.shape.struct { k8s.io/apimachinery/pkg/types.NamespacedName }]).Reconcile()
      /home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:116 +0x1c1
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[go.shape.struct { k8s.io/apimachinery/pkg/types.NamespacedName }]).reconcileHandler()
      /home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:303 +0x53d
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[go.shape.struct { k8s.io/apimachinery/pkg/types.NamespacedName }]).processNextWorkItem()
      /home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:263 +0x3ac
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[go.shape.struct { k8s.io/apimachinery/pkg/types.NamespacedName }]).Start.func2.2()
      /home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:224 +0xe4

Goroutine 4747 (running) created at:
  github.com/stretchr/testify/assert.EventuallyWithT()
      /home/runner/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:1978 +0x42a
  github.com/kong/gateway-operator/test/envtest.TestKongConsumer.func1()
      /home/runner/work/gateway-operator/gateway-operator/test/envtest/konnect_entities_kongconsumer_test.go:126 +0x1a34
  testing.tRunner()
      /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1742 +0x44

Goroutine 4275 (running) created at:
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[go.shape.struct { k8s.io/apimachinery/pkg/types.NamespacedName }]).Start.func2()
      /home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:220 +0x74b
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[go.shape.struct { k8s.io/apimachinery/pkg/types.NamespacedName }]).Start()
      /home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:231 +0x3ab
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[sigs.k8s.io/controller-runtime/pkg/reconcile.Request]).Start()
      /home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:145 +0x4a
  sigs.k8s.io/controller-runtime/pkg/manager.(*runnableGroup).reconcile.func1()
      /home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/manager/runnable_group.go:226 +0x1c6
  sigs.k8s.io/controller-runtime/pkg/manager.(*runnableGroup).reconcile.gowrap1()
      /home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/manager/runnable_group.go:229 +0x41
==================

Possibly fixes #655

@pmalek pmalek added this to the KGO v1.4.x milestone Oct 3, 2024
@pmalek pmalek marked this pull request as ready for review October 3, 2024 13:25
@pmalek pmalek requested a review from a team as a code owner October 3, 2024 13:25
Copy link
Contributor

@czeslavo czeslavo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduced this in a minimal example, looks like a legit fix 🤞

func TestSomething(t *testing.T) {
	c := &configurationv1.KongConsumer{
		CustomID: "custom-id",
	}
	ctx, _ := context.WithTimeout(context.Background(), 10*time.Second)
	go func() {
		for {
			select {
			case <-ctx.Done():
				return
			default:
				c.CustomID = "changed-id"
			}
		}
	}()

	go func() {
		for {
			select {
			case <-ctx.Done():
				return
			default:
				ci := sdkkonnectcomp.ConsumerInput{
					CustomID: &c.CustomID,
				}
				_ = *ci.GetCustomID()
			}
		}
	}()
}

@pmalek pmalek force-pushed the fix-datarace-in-kongconsumer-envtest branch from 928d5d2 to 5ac8235 Compare October 3, 2024 14:05
@pmalek pmalek enabled auto-merge (squash) October 3, 2024 14:08
@pmalek pmalek merged commit 477b132 into main Oct 3, 2024
21 checks passed
@pmalek pmalek deleted the fix-datarace-in-kongconsumer-envtest branch October 3, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Konnect envtest suite sometimes catches a data race
2 participants