From 44308b2cc3d454c1dc632b4062d8e373e5791941 Mon Sep 17 00:00:00 2001
From: shuguansheng <shuguansheng@huawei.com>
Date: Sun, 12 Apr 2026 21:58:14 +0800
Subject: [PATCH] Refactor role validation logic in validator.go

Signed-off-by: shuguansheng <shuguansheng@huawei.com>
---
 .../webhook/validator.go                      | 56 +++++++++----------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/pkg/model-serving-controller/webhook/validator.go b/pkg/model-serving-controller/webhook/validator.go
index 07e1c3a59..587c07d02 100644
--- a/pkg/model-serving-controller/webhook/validator.go
+++ b/pkg/model-serving-controller/webhook/validator.go
@@ -209,15 +209,16 @@ func validateGangPolicy(ms *workloadv1alpha1.ModelServing) field.ErrorList {
 	minRoleReplicasPath := field.NewPath("spec").Child("template").Child("gangPolicy").Child("minRoleReplicas")
 
 	// Create a map of role names for quick lookup
-	roleNames := make(map[string]bool)
-	for _, role := range ms.Spec.Template.Roles {
-		roleNames[role.Name] = true
+	roleNames := make(map[string]workloadv1alpha1.Role)
+	for _, roleElement := range ms.Spec.Template.Roles {
+		roleNames[roleElement.Name] = roleElement
 	}
 
 	// Validate each minRoleReplicas entry
 	for roleName, minReplicas := range minRoleReplicas {
 		// Check if the role exists
-		if !roleNames[roleName] {
+		role, ok := roleMap[roleName]
+		if !ok {
 			allErrs = append(allErrs, field.Invalid(
 				minRoleReplicasPath.Key(roleName),
 				roleName,
@@ -227,34 +228,29 @@ func validateGangPolicy(ms *workloadv1alpha1.ModelServing) field.ErrorList {
 		}
 
 		// Find the role to check its actual replicas
-		for _, role := range ms.Spec.Template.Roles {
-			if role.Name == roleName {
-				/// Calculate total replicas for this role
-				// minRoleReplicas is compared against the number of Role replicas
-				replicas := int32(1)
-				if role.Replicas != nil {
-					replicas = *role.Replicas
-				}
+		// Calculate total replicas for this role
+		// minRoleReplicas is compared against the number of Role replicas
+		replicas := int32(1)
+		if role.Replicas != nil {
+			replicas = *role.Replicas
+		}
 
-				// Validate minReplicas doesn't exceed total replicas
-				if minReplicas > replicas {
-					allErrs = append(allErrs, field.Invalid(
-						minRoleReplicasPath.Key(roleName),
-						minReplicas,
-						fmt.Sprintf("minRoleReplicas (%d) for role %s cannot exceed replicas (%d)", minReplicas, roleName, replicas),
-					))
-				}
+		// Validate minReplicas doesn't exceed total replicas
+		if minReplicas > replicas {
+			allErrs = append(allErrs, field.Invalid(
+				minRoleReplicasPath.Key(roleName),
+				minReplicas,
+				fmt.Sprintf("minRoleReplicas (%d) for role %s cannot exceed replicas (%d)", minReplicas, roleName, replicas),
+			))
+		}
 
-				// Validate minReplicas is non-negative
-				if minReplicas < 0 {
-					allErrs = append(allErrs, field.Invalid(
-						minRoleReplicasPath.Key(roleName),
-						minReplicas,
-						fmt.Sprintf("minRoleReplicas for role %s must be non-negative", roleName),
-					))
-				}
-				break
-			}
+		// Validate minReplicas is non-negative
+		if minReplicas < 0 {
+			allErrs = append(allErrs, field.Invalid(
+				minRoleReplicasPath.Key(roleName),
+				minReplicas,
+				fmt.Sprintf("minRoleReplicas for role %s must be non-negative", roleName),
+			))
 		}
 	}