Skip to content

Commit 374b38f

Browse files
committed
impelment a new test
1 parent 7a47c33 commit 374b38f

2 files changed

Lines changed: 98 additions & 4 deletions

File tree

rest/cluster_compat.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -452,9 +452,10 @@ func (m *clusterCompatManager) Refresh(ctx context.Context) {
452452

453453
// refreshNodeRegistrations iterates the tracked buckets, registers this node, and returns
454454
// the per-node version map and the aggregate freeze record across those registries. Returns
455-
// an error if every tracked bucket failed so callers can leave the previously-cached state
456-
// in place — stale is preferable to flipping the cluster compat version to nil on a transient
457-
// bucket outage.
455+
// an error only if every tracked bucket failed for a reason other than vanishing — stale cache
456+
// is preferable to flipping the cluster compat version to nil on a transient bucket outage. If
457+
// every tracked bucket vanished instead, that's reported as success with empty results, so the
458+
// caller updates the cache to reflect that no buckets remain tracked.
458459
func (m *clusterCompatManager) refreshNodeRegistrations(ctx context.Context) (map[string]base.ClusterCompatVersion, *base.RegistryFreeze, map[string]*base.RegistryPreCCVAwareNode, *base.ClusterCompatVersion, error) {
459460
buckets := m.trackedBucketList()
460461
if len(buckets) == 0 {
@@ -475,6 +476,7 @@ func (m *clusterCompatManager) refreshNodeRegistrations(ctx context.Context) (ma
475476
var aggregateFreeze *base.RegistryFreeze
476477
var aggregateHWM *base.ClusterCompatVersion
477478
succeeded := 0
479+
vanished := 0
478480
for _, bucket := range buckets {
479481
_, ratchet := eligibleBuckets[bucket]
480482
preCCVAwarePeers := m.sc.observePreCCVAwarePeersForBucket(ctx, bucket)
@@ -492,6 +494,7 @@ func (m *clusterCompatManager) refreshNodeRegistrations(ctx context.Context) (ma
492494
if errors.Is(err, errBucketDoesNotExist) {
493495
base.InfofCtx(ctx, base.KeyConfig, "Bucket %s no longer exists in cluster; removing from cluster compatibility manager: %v", base.MD(bucket), err)
494496
m.releaseBucket(bucket)
497+
vanished++
495498
continue
496499
}
497500
base.WarnfCtx(ctx, "Failed to register node version in bucket %s: %v", base.MD(bucket), err)
@@ -530,9 +533,11 @@ func (m *clusterCompatManager) refreshNodeRegistrations(ctx context.Context) (ma
530533
preCCVAwareMap[uuid] = &cp
531534
}
532535
}
533-
if succeeded == 0 {
536+
if succeeded == 0 && vanished < len(buckets) {
534537
return nil, nil, nil, nil, fmt.Errorf("no tracked bucket registries could be updated (%d tracked)", len(buckets))
535538
}
539+
// If every bucket vanished, the aggregates below are still empty/nil — correctly
540+
// representing "no buckets tracked" for the cache.
536541
return nodeMap, aggregateFreeze, preCCVAwareMap, aggregateHWM, nil
537542
}
538543

rest/cluster_compat_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
package rest
1010

1111
import (
12+
"context"
13+
"errors"
1214
"fmt"
1315
"maps"
1416
"net/http"
@@ -1935,3 +1937,90 @@ func TestClusterCompatRefreshAndStopUntrackVanishedBucket(t *testing.T) {
19351937
require.NotContains(t, clusterCompat.trackedBucketList(), fakeBucket, "vanished bucket should be untracked")
19361938
}
19371939
}
1940+
1941+
// fakeVanishingBootstrapConnection simulates a bucket that no longer exists in the cluster:
1942+
// GetMetadataDocument always fails, and any bucket absent from knownBuckets is then classified
1943+
// as errBucketDoesNotExist by getGatewayRegistry. Only the methods refreshNodeRegistrations
1944+
// actually calls are implemented; anything else panics via the nil embedded interface.
1945+
type fakeVanishingBootstrapConnection struct {
1946+
base.BootstrapConnection
1947+
knownBuckets []string
1948+
}
1949+
1950+
func (f *fakeVanishingBootstrapConnection) GetConfigBuckets(_ context.Context) ([]string, error) {
1951+
return f.knownBuckets, nil
1952+
}
1953+
1954+
func (f *fakeVanishingBootstrapConnection) GetMetadataDocument(_ context.Context, _, _ string, _ any) (uint64, error) {
1955+
return 0, errors.New("simulated: unable to read metadata document")
1956+
}
1957+
1958+
// newTestClusterCompatManager builds a clusterCompatManager backed by a fake bootstrap
1959+
// connection, skipping the full RestTester/ServerContext setup — refreshNodeRegistrations only
1960+
// touches fields that are safe at their zero value here (NodeUID, Config, _databases(Lock)).
1961+
func newTestClusterCompatManager(knownBuckets []string, trackedBuckets ...string) *clusterCompatManager {
1962+
sc := &ServerContext{
1963+
Config: &StartupConfig{},
1964+
NodeUID: "test-node",
1965+
BootstrapContext: &bootstrapContext{
1966+
Connection: &fakeVanishingBootstrapConnection{knownBuckets: knownBuckets},
1967+
},
1968+
}
1969+
tracked := make(map[string]struct{}, len(trackedBuckets))
1970+
for _, b := range trackedBuckets {
1971+
tracked[b] = struct{}{}
1972+
}
1973+
return &clusterCompatManager{sc: sc, trackedBuckets: tracked}
1974+
}
1975+
1976+
// TestClusterCompatRefreshNodeRegistrationsAllBucketsVanished verifies that when every tracked
1977+
// bucket vanishes in the same pass, refreshNodeRegistrations reports success with empty
1978+
// results rather than an error — otherwise Refresh would never clear the cache, since it stops
1979+
// calling refreshNodeRegistrations once trackedBuckets is empty.
1980+
func TestClusterCompatRefreshNodeRegistrationsAllBucketsVanished(t *testing.T) {
1981+
ctx := base.TestCtx(t)
1982+
m := newTestClusterCompatManager(nil, "vanished-bucket-1", "vanished-bucket-2")
1983+
1984+
nodes, freeze, preCCVAwareNodes, hwm, err := m.refreshNodeRegistrations(ctx)
1985+
require.NoError(t, err, "all-vanished tracked buckets must not be reported as a failure")
1986+
assert.Empty(t, nodes)
1987+
assert.Nil(t, freeze)
1988+
assert.Empty(t, preCCVAwareNodes)
1989+
assert.Nil(t, hwm)
1990+
assert.Empty(t, m.trackedBucketList(), "vanished buckets must be untracked")
1991+
}
1992+
1993+
// TestClusterCompatRefreshAllBucketsVanishedClearsCache verifies that Refresh actually clears a
1994+
// previously-cached, now-stale cluster compat version once every tracked bucket has vanished.
1995+
func TestClusterCompatRefreshAllBucketsVanishedClearsCache(t *testing.T) {
1996+
ctx := base.TestCtx(t)
1997+
m := newTestClusterCompatManager(nil, "vanished-bucket")
1998+
1999+
staleVersion := base.NewClusterCompatVersion(3, 3)
2000+
m.cachedVersion = &staleVersion
2001+
m.cachedNodes = map[string]base.ClusterCompatVersion{"some-other-node": staleVersion}
2002+
2003+
m.Refresh(ctx)
2004+
2005+
assert.Nil(t, m.getCachedVersion(), "cache must be cleared once no buckets remain tracked")
2006+
assert.Empty(t, m.NodeVersions())
2007+
assert.Empty(t, m.trackedBucketList())
2008+
}
2009+
2010+
// TestClusterCompatRefreshNodeRegistrationsPartialVanishKeepsError verifies the fix is scoped
2011+
// to the all-vanished case: if a vanished bucket is mixed with a bucket that fails for another
2012+
// reason, the result is still reported as an error so Refresh keeps the old cached state
2013+
// instead of caching an incomplete view.
2014+
func TestClusterCompatRefreshNodeRegistrationsPartialVanishKeepsError(t *testing.T) {
2015+
ctx := base.TestCtx(t)
2016+
// flaky-bucket exists (per GetConfigBuckets) but every read against it fails, so it's a
2017+
// real, non-vanished failure. vanished-bucket doesn't exist, so it's classified as vanished.
2018+
m := newTestClusterCompatManager([]string{"flaky-bucket"}, "vanished-bucket", "flaky-bucket")
2019+
2020+
_, _, _, _, err := m.refreshNodeRegistrations(ctx)
2021+
require.Error(t, err, "a mix of vanished and non-vanished failures must still be reported as an error")
2022+
2023+
tracked := m.trackedBucketList()
2024+
assert.NotContains(t, tracked, "vanished-bucket", "vanished bucket must still be untracked")
2025+
assert.Contains(t, tracked, "flaky-bucket", "non-vanished failures must not untrack the bucket")
2026+
}

0 commit comments

Comments
 (0)