MESH-6122: Add tests for GetServiceWithSuffixMatch and update functio…

…n calls with rollout selector Signed-off-by: Punakshi <[email protected]>
istio-ecosystem · Feb 11, 2025 · e942eca · e942eca
1 parent daa15a9
commit e942eca
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 4 deletions.
diff --git a/admiral/pkg/clusters/handler.go b/admiral/pkg/clusters/handler.go
@@ -161,7 +161,7 @@ func getServiceForRollout(ctx context.Context, rc *RemoteController, rollout *ro
 		blueGreenPreviewService = rolloutStrategy.BlueGreen.PreviewService
 		if len(blueGreenActiveService) == 0 {
 			//pick a service that ends in RolloutActiveServiceSuffix if one is available
-			blueGreenActiveService = GetServiceWithSuffixMatch(common.RolloutActiveServiceSuffix, cachedServices)
+			blueGreenActiveService = GetServiceWithSuffixMatch(common.RolloutActiveServiceSuffix, cachedServices, rollout.Spec.Selector)
 		}
 	} else if rolloutStrategy.Canary != nil {
 		//If istio canary perform below operations
@@ -259,11 +259,10 @@ func getServiceForRollout(ctx context.Context, rc *RemoteController, rollout *ro
 				since istio does not know the split info as there is no virtual service
 			*/
 
-			sName := GetServiceWithSuffixMatch(common.RolloutRootServiceSuffix, cachedServices)
+			sName := GetServiceWithSuffixMatch(common.RolloutRootServiceSuffix, cachedServices, rollout.Spec.Selector)
 			if len(sName) <= 0 {
 				//Fallback if root service not found
-				log.Infof("root service not found, falling back to stable for rollout=%s in namespace=%s and cluster=%s", rollout.Name, rollout.Namespace, rc.ClusterID)
-				sName = GetServiceWithSuffixMatch(common.RolloutStableServiceSuffix, cachedServices)
+				sName = GetServiceWithSuffixMatch(common.RolloutStableServiceSuffix, cachedServices, rollout.Spec.Selector)
 			}
 
 			// If root and stable not found, exit canary logic and use generic logic to choose random service

diff --git a/admiral/pkg/clusters/handler_test.go b/admiral/pkg/clusters/handler_test.go
@@ -979,3 +979,99 @@ func TestGetServiceForRolloutBlueGreen(t *testing.T) {
 		})
 	}
 }
+
+func TestGetServiceWithSuffixMatch(t *testing.T) {
+	selectorMap := map[string]string{
+		"app": "test",
+	}
+	labelSelector := &metaV1.LabelSelector{
+		MatchLabels: selectorMap,
+	}
+
+	services := []*coreV1.Service{
+		{
+			ObjectMeta: metaV1.ObjectMeta{Name: "service1-root-service"},
+			Spec: coreV1.ServiceSpec{
+				Selector: selectorMap,
+			},
+		},
+		{
+			ObjectMeta: metaV1.ObjectMeta{Name: "service2"},
+			Spec: coreV1.ServiceSpec{
+				Selector: selectorMap,
+			},
+		},
+		{
+			ObjectMeta: metaV1.ObjectMeta{Name: "service3-root-service"},
+			Spec: coreV1.ServiceSpec{
+				Selector: map[string]string{
+					"app": "different",
+				},
+			},
+		},
+	}
+
+	testCases := []struct {
+		name            string
+		suffix          string
+		services        []*coreV1.Service
+		rolloutSelector *metaV1.LabelSelector
+		expectedResult  string
+	}{
+		{
+			name:            "Match found, given suffux, services and selector. Should return service with suffix",
+			suffix:          "root-service",
+			services:        services,
+			rolloutSelector: labelSelector,
+			expectedResult:  "service1-root-service",
+		},
+		{
+			name:            "No match found, given incorrect suffux, services and selector. Should return empty",
+			suffix:          "nomatch",
+			services:        services,
+			rolloutSelector: labelSelector,
+			expectedResult:  "",
+		},
+		{
+			name:     "No match found, given suffux, services and not matching selector. Should return empty",
+			suffix:   "root-service",
+			services: services,
+			rolloutSelector: &metaV1.LabelSelector{
+				MatchLabels: map[string]string{
+					"app": "nomatch",
+				},
+			},
+			expectedResult: "",
+		},
+		{
+			name:            "Match found, given empty suffux, services and selector. Should return first service",
+			suffix:          "",
+			services:        services,
+			rolloutSelector: labelSelector,
+			expectedResult:  "service1-root-service",
+		},
+		{
+			name:            "No match found, given suffux, services and nil selector. Should return empty",
+			suffix:          "suffix",
+			services:        services,
+			rolloutSelector: nil,
+			expectedResult:  "",
+		},
+		{
+			name:            "No match found, given suffux, no services and selector. Should return empty",
+			suffix:          "suffix",
+			services:        []*coreV1.Service{},
+			rolloutSelector: labelSelector,
+			expectedResult:  "",
+		},
+	}
+
+	for _, c := range testCases {
+		t.Run(c.name, func(t *testing.T) {
+			result := GetServiceWithSuffixMatch(c.suffix, c.services, c.rolloutSelector)
+			if result != c.expectedResult {
+				t.Errorf("Failed. Got %v, expected %v", result, c.expectedResult)
+			}
+		})
+	}
+}