Skip to content

Data race: gkeClient and gkeClientCtx stored as shared Handler fields across 3 concurrent workers #1625

Description

@kunaldevxxx

Summary

Handler.gkeClient and Handler.gkeClientCtx are struct fields that are overwritten at the start of every reconcile call. The operator starts with 3 concurrent worker goroutines (start.All(ctx, 3, ...) in main.go:66), all sharing the same Handler instance. This creates an unguarded data race.

Root Cause

In controller/gke-cluster-config-handler.go, the Handler struct holds client state as fields:

// line 71-72

Every reconcile call overwrites these fields before using them:

// lines 101-116 in OnGkeConfigChanged
h.gkeClientCtx = ctx
...
h.gkeClient = gkeClient

With 3 workers running concurrently:

  • Worker A sets h.gkeClient for cluster X
  • Worker B overwrites h.gkeClient for cluster Y
  • Worker A now calls GKE APIs using cluster Y's client — operating on the wrong cluster

The same pattern exists in OnGkeConfigRemoved (lines 197–209).

Impact

  • GKE API calls may execute against the wrong cluster
  • In the worst case: node pool updates, deletions, or upgrades applied to an unintended cluster
  • The race is detectable with go test -race but won't surface in normal unit tests

Steps to Reproduce

Run the operator with two or more GKEClusterConfig objects reconciling simultaneously.

Run with the -race flag:

go test -race ./controller/...

Expected Behavior

Each reconcile should use a locally scoped gkeClient and ctx, not mutate shared struct fields.

Suggested Fix

Pass ctx and gkeClient as parameters into each sub-handler rather than storing them on the struct:

func (h *Handler) OnGkeConfigChanged(_ string, config *gkev1.GKEClusterConfig) (*gkev1.GKEClusterConfig, error) {
    ctx, cancel := context.WithCancel(context.Background())
    defer cancel()

    cred, err := GetSecret(ctx, h.secrets, &config.Spec)
    gkeClient, err := gke.GetGKEClusterClient(ctx, cred)

    // pass ctx and gkeClient as arguments, not via h.*
    return h.checkAndUpdate(ctx, gkeClient, config)
}

Remove gkeClient and gkeClientCtx from the Handler struct entirely.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions