Skip to content

Commit

Permalink
clustermesh: drop node observer global variables from tests
Browse files Browse the repository at this point in the history
Move them as fields of the testObserver struct, to prevent possible
flakes caused by interference and leftover state due to the usage
of global variables accessed by multiple tests.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
  • Loading branch information
giorio94 authored and julianwiedmann committed May 13, 2024
1 parent 3f5e735 commit c297c62
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 24 deletions.
47 changes: 25 additions & 22 deletions pkg/clustermesh/clustermesh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@ import (
testidentity "github.com/cilium/cilium/pkg/testutils/identity"
)

var (
nodes = map[string]*testNode{}
nodesMutex lock.RWMutex
)

type testNode struct {
// Name is the name of the node. This is typically the hostname of the node.
Name string
Expand Down Expand Up @@ -66,20 +61,27 @@ var testNodeCreator = func() store.Key {
return &n
}

type testObserver struct{}
type testObserver struct {
nodes map[string]*testNode
nodesMutex lock.RWMutex
}

func newNodesObserver() *testObserver {
return &testObserver{nodes: make(map[string]*testNode)}
}

func (o *testObserver) OnUpdate(k store.Key) {
n := k.(*testNode)
nodesMutex.Lock()
nodes[n.GetKeyName()] = n
nodesMutex.Unlock()
o.nodesMutex.Lock()
o.nodes[n.GetKeyName()] = n
o.nodesMutex.Unlock()
}

func (o *testObserver) OnDelete(k store.NamedKey) {
n := k.(*testNode)
nodesMutex.Lock()
delete(nodes, n.GetKeyName())
nodesMutex.Unlock()
o.nodesMutex.Lock()
delete(o.nodes, n.GetKeyName())
o.nodesMutex.Unlock()
}

func TestClusterMesh(t *testing.T) {
Expand Down Expand Up @@ -139,11 +141,12 @@ func TestClusterMesh(t *testing.T) {

usedIDs := NewClusterMeshUsedIDs()
storeFactory := store.NewFactory(store.MetricsProvider())
nodesObserver := newNodesObserver()
cm := NewClusterMesh(hivetest.Lifecycle(t), Configuration{
Config: common.Config{ClusterMeshConfig: dir},
ClusterInfo: types.ClusterInfo{ID: 255, Name: "test2", MaxConnectedClusters: 255},
NodeKeyCreator: testNodeCreator,
NodeObserver: &testObserver{},
NodeObserver: nodesObserver,
RemoteIdentityWatcher: mgr,
IPCache: ipc,
ClusterIDsManager: usedIDs,
Expand Down Expand Up @@ -212,9 +215,9 @@ func TestClusterMesh(t *testing.T) {

// wait for all cm nodes in both clusters to appear in the node list
require.EventuallyWithT(t, func(c *assert.CollectT) {
nodesMutex.RLock()
defer nodesMutex.RUnlock()
assert.Len(c, nodes, 3*len(nodeNames))
nodesObserver.nodesMutex.RLock()
defer nodesObserver.nodesMutex.RUnlock()
assert.Len(c, nodesObserver.nodes, 3*len(nodeNames))
}, timeout, tick, "Nodes not watched correctly")

require.NoError(t, os.Remove(config2), "Failed to remove config file for cluster2")
Expand All @@ -234,9 +237,9 @@ func TestClusterMesh(t *testing.T) {

// wait for the nodes of the removed cluster to disappear
require.EventuallyWithT(t, func(c *assert.CollectT) {
nodesMutex.RLock()
defer nodesMutex.RUnlock()
assert.Len(c, nodes, 2*len(nodeNames))
nodesObserver.nodesMutex.RLock()
defer nodesObserver.nodesMutex.RUnlock()
assert.Len(c, nodesObserver.nodes, 2*len(nodeNames))
}, timeout, tick, "Nodes were not drained correctly")

require.NoError(t, os.Remove(config1), "Failed to remove config file for cluster1")
Expand All @@ -249,9 +252,9 @@ func TestClusterMesh(t *testing.T) {

// wait for the nodes of the removed cluster to disappear
require.EventuallyWithT(t, func(c *assert.CollectT) {
nodesMutex.RLock()
defer nodesMutex.RUnlock()
assert.Len(c, nodes, 0)
nodesObserver.nodesMutex.RLock()
defer nodesObserver.nodesMutex.RUnlock()
assert.Len(c, nodesObserver.nodes, 0)
}, timeout, tick, "Nodes were not drained correctly")

// Make sure that IDs are freed
Expand Down
2 changes: 1 addition & 1 deletion pkg/clustermesh/remote_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func TestRemoteClusterRun(t *testing.T) {
cm := ClusterMesh{
conf: Configuration{
NodeKeyCreator: testNodeCreator,
NodeObserver: &testObserver{},
NodeObserver: newNodesObserver(),
IPCache: &ipc,
RemoteIdentityWatcher: allocator,
ClusterIDsManager: NewClusterMeshUsedIDs(),
Expand Down
2 changes: 1 addition & 1 deletion pkg/clustermesh/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func setup(tb testing.TB) *ClusterMeshServicesTestSuite {
Config: common.Config{ClusterMeshConfig: dir},
ClusterInfo: cmtypes.ClusterInfo{ID: 255, Name: "test2", MaxConnectedClusters: 255},
NodeKeyCreator: testNodeCreator,
NodeObserver: &testObserver{},
NodeObserver: newNodesObserver(),
ServiceMerger: s.svcCache,
RemoteIdentityWatcher: mgr,
IPCache: ipc,
Expand Down

0 comments on commit c297c62

Please sign in to comment.