nebius/SWE-rebench-V2-PRs huggingface dataset:
Repository: Azure/aad-pod-identity
Pull request #564 (2020-04-15 16:13:22)

process del id list before add
<!-- Thank you for helping AAD Pod Identity with a pull request! -->

**Reason for Change**:
<!-- What does this PR improve or fix in AAD Pod Identity? Why is it needed? -->
Add is done after setting del ids in the map to ensure an identity if in both add and del list is not deleted

**Issue Fixed**:
<!-- If this PR fixes GitHub issue 1234, add "Fixes #1234" to the next line. -->

**Notes for Reviewers**:



Patch:
diff --git a/pkg/cloudprovider/cloudprovider.go b/pkg/cloudprovider/cloudprovider.go
index 070ddd23..fce716a9 100644
--- a/pkg/cloudprovider/cloudprovider.go
+++ b/pkg/cloudprovider/cloudprovider.go
@@ -178,14 +178,16 @@ func (c *Client) UpdateUserMSI(addUserAssignedMSIIDs, removeUserAssignedMSIIDs [
 	}
 
 	ids := make(map[string]bool)
-	// add new ids to the list
-	for _, userAssignedMSIID := range addUserAssignedMSIIDs {
-		ids[userAssignedMSIID] = true
-	}
 	// remove msi ids from the list
 	for _, userAssignedMSIID := range removeUserAssignedMSIIDs {
 		ids[userAssignedMSIID] = false
 	}
+	// add new ids to the list
+	// add is done after setting del ids in the map to ensure an identity if in
+	// both add and del list is not deleted
+	for _, userAssignedMSIID := range addUserAssignedMSIIDs {
+		ids[userAssignedMSIID] = true
+	}
 	requiresUpdate := info.SetUserIdentities(ids)
 
 	if requiresUpdate {


Test patch:
diff --git a/pkg/cloudprovider/cloudprovider_test.go b/pkg/cloudprovider/cloudprovider_test.go
index bde0cef6..a30bbe5c 100644
--- a/pkg/cloudprovider/cloudprovider_test.go
+++ b/pkg/cloudprovider/cloudprovider_test.go
@@ -59,9 +59,9 @@ func TestSimple(t *testing.T) {
 	vmssProvider := "azure:///subscriptions/fakeSub/resourceGroups/fakeGroup/providers/Microsoft.Compute/virtualMachineScaleSets/node4/virtualMachines/0"
 
 	for _, cfg := range []config.AzureConfig{
-		config.AzureConfig{},
-		config.AzureConfig{VMType: "vmss"},
-		config.AzureConfig{VMType: "vm"},
+		{},
+		{VMType: "vmss"},
+		{VMType: "vm"},
 	} {
 		desc := cfg.VMType
 		if desc == "" {
@@ -135,6 +135,13 @@ func TestSimple(t *testing.T) {
 				cloudClient.PrintMSI(t)
 				t.Error("MSI mismatch")
 			}
+
+			cloudClient.UpdateUserMSI([]string{"ID3"}, []string{"ID3"}, node4.Name, true)
+			testMSI = []string{"ID4", "ID3"}
+			if !cloudClient.CompareMSI(node4.Name, true, testMSI) {
+				cloudClient.PrintMSI(t)
+				t.Error("MSI mismatch")
+			}
 		})
 	}
 }
Repository: Azure/aad-pod-identity
Pull request #344 (2019-08-16 22:28:15)

Stats locks and tests
<!-- Thank you for helping AAD Pod Identity with a pull request! -->

**Reason for Change**:
<!-- What does this PR improve or fix in AAD Pod Identity? Why is it needed? -->
With parallel node updates there are code paths which updates these stats concurrently.
These changes add lock around stats and UT corresponding to that.

**Issue Fixed**:
<!-- If this PR fixes GitHub issue 1234, add "Fixes #1234" to the next line. -->

**Notes for Reviewers**:


Patch:
diff --git a/pkg/stats/stats.go b/pkg/stats/stats.go
index e7eb6d4e..f8c5d327 100644
--- a/pkg/stats/stats.go
+++ b/pkg/stats/stats.go
@@ -1,12 +1,14 @@
 package stats
 
 import (
+	"sync"
 	"time"
 
 	"github.com/golang/glog"
 )
 
 var GlobalStats map[StatsType]time.Duration
+var Mutex *sync.RWMutex
 
 type StatsType string
 
@@ -36,16 +38,21 @@ const (
 
 func Init() {
 	GlobalStats = make(map[StatsType]time.Duration)
+	Mutex = &sync.RWMutex{}
 }
 
 func Put(key StatsType, val time.Duration) {
 	if GlobalStats != nil {
+		Mutex.Lock()
+		defer Mutex.Unlock()
 		GlobalStats[key] = val
 	}
 }
 
 func Get(key StatsType) time.Duration {
 	if GlobalStats != nil {
+		Mutex.RLock()
+		defer Mutex.RUnlock()
 		return GlobalStats[key]
 	}
 	return 0
@@ -53,11 +60,15 @@ func Get(key StatsType) time.Duration {
 
 func Update(key StatsType, val time.Duration) {
 	if GlobalStats != nil {
+		Mutex.Lock()
+		defer Mutex.Unlock()
 		GlobalStats[key] = GlobalStats[key] + val
 	}
 }
 
 func Print(key StatsType) {
+	Mutex.RLock()
+	defer Mutex.RUnlock()
 	glog.Infof("%s: %s", key, GlobalStats[key])
 }
 
@@ -88,6 +99,8 @@ func PrintSync() {
 }
 
 func GetAll() map[StatsType]time.Duration {
+	Mutex.RLock()
+	defer Mutex.RUnlock()
 	return GlobalStats
 }
 


Test patch:
diff --git a/pkg/stats/stats_test.go b/pkg/stats/stats_test.go
index 853257c9..365d6f51 100644
--- a/pkg/stats/stats_test.go
+++ b/pkg/stats/stats_test.go
@@ -1,7 +1,9 @@
 package stats_test
 
 import (
+	"fmt"
 	"reflect"
+	"sync"
 	"testing"
 	"time"
 
@@ -31,3 +33,39 @@ func TestBasics(t *testing.T) {
 		panic("Stats added did not come back")
 	}
 }
+
+func TestConcurrency(t *testing.T) {
+	glog.Infof("Concurrency test starting")
+	stats.Init()
+	var wg sync.WaitGroup
+	var startWg sync.WaitGroup
+
+	// Make sure all of them start at the same time so that there are more chances of sync issues.
+	startWg.Add(1)
+	count := 100
+	duration := time.Second * 10
+	for i := 0; i < count; i++ {
+		wg.Add(1)
+		go func(c int) {
+			defer wg.Done()
+			startWg.Wait()
+			fmt.Printf("Updating %d \n", c)
+			stats.Update(stats.AssignedIDList, duration)
+		}(i)
+		wg.Add(1)
+		go func(c int) {
+			defer wg.Done()
+			startWg.Wait()
+			fmt.Printf("Getting %d\n", c)
+			stats.Get(stats.AssignedIDList)
+		}(i)
+	}
+
+	// All of them should start together.
+	startWg.Done()
+	wg.Wait()
+
+	if stats.Get(stats.AssignedIDList) != duration*time.Duration(count) {
+		panic("Stats did not get incremented.")
+	}
+}