Flaky test: TestMultitenantAlertmanager_zoneAwareSharding (arm64) — ownership computed before peer instances appear in the ring view

[CharlieTLe]

Summary

TestMultitenantAlertmanager_zoneAwareSharding fails intermittently on the test (arm64) job with expected: 3, actual: 4 at pkg/alertmanager/multitenant_test.go:891 — the zoneA assertion on cortex_alertmanager_tenants_owned.

Observed on run 35636828860 (attempt 1, test (arm64)), on a backport PR that touches only pkg/querier/tenantfederation and pkg/ingester. Re-running the job passed, and the test passes locally 15/15 with -race on arm64, so it is timing-dependent rather than a regression.

Failure output

--- FAIL: TestMultitenantAlertmanager_zoneAwareSharding (0.34s)
    multitenant_test.go:891:
        	Error Trace:	/__w/cortex/cortex/pkg/alertmanager/multitenant_test.go:891
        	Error:      	Not equal:
        	            	expected: 3
        	            	actual  : 4
        	Test:       	TestMultitenantAlertmanager_zoneAwareSharding
FAIL
FAIL	github.com/cortexproject/cortex/pkg/alertmanager	48.579s

(The job log for attempt 1 is no longer retrievable — the successful re-run replaced it — hence the inline copy.)

Root cause

The test starts three alertmanagers sharing one in-memory Consul KV: instance-1 and instance-2 in zoneA, instance-3 in zoneB, with ReplicationFactor = 2 and zone awareness on. It then calls loadAndSyncConfigs on each and asserts that the cortex_alertmanager_tenants_owned gauges sum to 3 per zone.

Ownership is resolved per instance against that instance's own ring client view:

https://github.com/cortexproject/cortex/blob/master/pkg/alertmanager/multitenant.go#L734

alertmanagers, err := am.ring.Get(users.ShardByUser(userID), getSyncRingOp(...), nil, nil, nil)
...
return alertmanagers.Includes(am.ringLifecycler.GetInstanceAddr()), nil

services.StartAndAwaitRunning in createInstance only waits for that instance to reach ACTIVE in the ring — it establishes nothing about when the other instances' ring clients observe it. Each alertmanager's ring client picks up peers asynchronously through its KV watch, so there is no happens-before edge between instance-2 joining and instance-1's ring client seeing it.

With RF=2 across two zones, ring.Get returns one instance per zone, so zoneA's two instances should split the three tenants 2/1 and sum to 3. If instance-1's ring view still omits instance-2 when loadAndSyncConfigs runs, instance-1 believes it is the only instance in zoneA and claims all 3; instance-2 then evaluates with the converged view and claims its 1, summing to 4.

This also explains why only line 891 fails: zoneB has a single instance, which owns all 3 tenants under any ring view, so line 892 is insensitive to the race. The window is sub-millisecond on a fast machine, which is why it surfaces on the loaded arm64 runners.

The race is latent in the test as originally written in #4204 (2021) — no recent change introduced it.

Proposed fix

Wait for all three ring clients to converge before calling loadAndSyncConfigs. InstancesCount() is on the ReadRing interface, and test.Poll is already used elsewhere in this file:

// Wait until all instances see the full ring, otherwise ownership is
// computed against a partial view and tenants get double-counted.
for _, am := range []*MultitenantAlertmanager{am1ZoneA, am2ZoneA, am1ZoneB} {
	test.Poll(t, 5*time.Second, 3, func() interface{} {
		return am.ring.InstancesCount()
	})
}

An alternative — polling the metric itself until it settles — would mask a genuine ownership bug, so gating on ring convergence is preferable.