diff --git a/internal/memstore/ciMemStore.go b/internal/memstore/ciMemStore.go index c9b834440..7eb1a809e 100644 --- a/internal/memstore/ciMemStore.go +++ b/internal/memstore/ciMemStore.go @@ -84,8 +84,8 @@ func (m *MemStore) GetGroups() map[string]cistore.GroupData { } func (m *MemStore) AddGroupData(groupName string, newGroupData cistore.GroupData) error { - m.GroupsMutex.RLock() - defer m.GroupsMutex.RUnlock() + m.GroupsMutex.Lock() + defer m.GroupsMutex.Unlock() // get CI data and check if groups IDENTIFIER exists (creates if not) _, ok := m.Groups[groupName] if ok { @@ -115,8 +115,8 @@ func (m *MemStore) GetGroupData(groupName string) (cistore.GroupData, error) { // UpdateGroupData is similar to AddGroupData but only works if the group exists func (m *MemStore) UpdateGroupData(groupName string, groupData cistore.GroupData, create bool) error { - m.GroupsMutex.RLock() - defer m.GroupsMutex.RUnlock() + m.GroupsMutex.Lock() + defer m.GroupsMutex.Unlock() if create { m.Groups[groupName] = groupData return nil @@ -132,15 +132,15 @@ func (m *MemStore) UpdateGroupData(groupName string, groupData cistore.GroupData } func (m *MemStore) RemoveGroupData(name string) error { - m.GroupsMutex.RLock() - defer m.GroupsMutex.RUnlock() + m.GroupsMutex.Lock() + defer m.GroupsMutex.Unlock() delete(m.Groups, name) return nil } func (m *MemStore) GetInstanceInfo(nodeName string) (cistore.OpenCHAMIInstanceInfo, error) { - m.InstancesMutex.RLock() - defer m.InstancesMutex.RUnlock() + m.InstancesMutex.Lock() + defer m.InstancesMutex.Unlock() if _, ok := m.Instances[nodeName]; !ok { m.Instances[nodeName] = cistore.OpenCHAMIInstanceInfo{ InstanceID: generateInstanceId(), @@ -150,8 +150,8 @@ func (m *MemStore) GetInstanceInfo(nodeName string) (cistore.OpenCHAMIInstanceIn } func (m *MemStore) SetInstanceInfo(nodeName string, instanceInfo cistore.OpenCHAMIInstanceInfo) error { - m.InstancesMutex.RLock() - defer m.InstancesMutex.RUnlock() + m.InstancesMutex.Lock() + defer m.InstancesMutex.Unlock() if _, ok := m.Instances[nodeName]; !ok { // This is a creation operation if instanceInfo.InstanceID == "" { @@ -167,8 +167,8 @@ func (m *MemStore) SetInstanceInfo(nodeName string, instanceInfo cistore.OpenCHA } func (m *MemStore) DeleteInstanceInfo(nodeName string) error { - m.InstancesMutex.RLock() - defer m.InstancesMutex.RUnlock() + m.InstancesMutex.Lock() + defer m.InstancesMutex.Unlock() delete(m.Instances, nodeName) return nil } diff --git a/internal/memstore/ciMemStore_test.go b/internal/memstore/ciMemStore_test.go index d4ce6d513..207106ca2 100644 --- a/internal/memstore/ciMemStore_test.go +++ b/internal/memstore/ciMemStore_test.go @@ -4,8 +4,10 @@ import ( "fmt" "os" "path/filepath" + "sync" "testing" + "github.com/OpenCHAMI/cloud-init/pkg/cistore" "github.com/stretchr/testify/require" storetesting "github.com/OpenCHAMI/cloud-init/pkg/cistore/testing" @@ -96,3 +98,63 @@ base-url: http://test.example/ testInvalidFile = `this is not yaml` ) + +func TestConcurrentInstanceAccess(t *testing.T) { + store := NewMemStore() + + store.SetInstanceInfo("node1", cistore.OpenCHAMIInstanceInfo{InstanceID: "i-001"}) // nolint:errcheck + + t.Run("concurrent_reads", func(t *testing.T) { + var wg sync.WaitGroup + for i := 0; i < 100; i++ { + wg.Add(1) + go func(id int) { + defer wg.Done() + for j := 0; j < 10; j++ { + _, err := store.GetInstanceInfo("node1") + if err != nil { + t.Errorf("concurrent read failed: %v", err) + return + } + } + }(i) + } + wg.Wait() + }) + + t.Run("concurrent_writes", func(t *testing.T) { + var wg sync.WaitGroup + for i := 0; i < 50; i++ { + wg.Add(1) + go func(id int) { + defer wg.Done() + nodeID := fmt.Sprintf("node-%d", id) + err := store.SetInstanceInfo(nodeID, cistore.OpenCHAMIInstanceInfo{InstanceID: fmt.Sprintf("i-%d", id)}) + if err != nil { + t.Errorf("concurrent write failed for %s: %v", nodeID, err) + } + }(i) + } + wg.Wait() + }) + + t.Run("mixed_access", func(t *testing.T) { + var wg sync.WaitGroup + for i := 0; i < 25; i++ { + wg.Add(1) + go func(id int) { + defer wg.Done() + nodeID := fmt.Sprintf("mixed-node-%d", id) + _ = store.SetInstanceInfo(nodeID, cistore.OpenCHAMIInstanceInfo{InstanceID: fmt.Sprintf("i-mixed-%d", id)}) + }(i) + } + for i := 0; i < 25; i++ { + wg.Add(1) + go func(id int) { + defer wg.Done() + _, _ = store.GetInstanceInfo("node1") + }(i) + } + wg.Wait() + }) +}