Bugfix/exclusive map access

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
 }

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()
+	})
+}