fix: Selenium Grid in case multiple scaler triggers are activate (ked…

…acore#6437) * fix: Selenium Grid scaler avoids overlapping when multiple browserVersion triggers are active Signed-off-by: Viet Nguyen Duc <[email protected]> * Update CHANGELOG Signed-off-by: Viet Nguyen Duc <[email protected]> * Fix e2e template test Signed-off-by: Viet Nguyen Duc <[email protected]> * Change imagePullPolicy to Always to take latest change Signed-off-by: Viet Nguyen Duc <[email protected]> * Update platformName default value as empty Signed-off-by: Viet Nguyen Duc <[email protected]> --------- Signed-off-by: Viet Nguyen Duc <[email protected]> Signed-off-by: Jorge Turrado <[email protected]>
JorTurFer · Dec 24, 2024 · 4f07727 · 4f07727
1 parent a39721d
commit 4f07727
Show file tree

Hide file tree

Showing 4 changed files with 1,372 additions and 237 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -62,6 +62,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio
 - **General**: Paused ScaledObject count is reported correctly after operator restart ([#6321](https://github.com/kedacore/keda/issues/6321))
 - **General**: ScaledJobs ready status set to true when recoverred problem ([#6329](https://github.com/kedacore/keda/pull/6329))
 - **Selenium Grid Scaler**: Exposes sum of pending and ongoing sessions to KDEA ([#6368](https://github.com/kedacore/keda/pull/6368))
+- **Selenium Grid Scaler**: Selenium Grid in case multiple scaler triggers are activate ([#6437](https://github.com/kedacore/keda/pull/6437))
 
 ### Other
 

diff --git a/pkg/scalers/selenium_grid_scaler.go b/pkg/scalers/selenium_grid_scaler.go
@@ -33,12 +33,12 @@ type seleniumGridScalerMetadata struct {
 	Username            string `keda:"name=username,                 order=authParams;resolvedEnv, optional"`
 	Password            string `keda:"name=password,                 order=authParams;resolvedEnv, optional"`
 	AccessToken         string `keda:"name=accessToken,              order=authParams;resolvedEnv, optional"`
-	BrowserName         string `keda:"name=browserName,              order=triggerMetadata"`
+	BrowserName         string `keda:"name=browserName,              order=triggerMetadata, optional"`
 	SessionBrowserName  string `keda:"name=sessionBrowserName,       order=triggerMetadata, optional"`
+	BrowserVersion      string `keda:"name=browserVersion,           order=triggerMetadata, optional"`
+	PlatformName        string `keda:"name=platformName,             order=triggerMetadata, optional"`
 	ActivationThreshold int64  `keda:"name=activationThreshold,      order=triggerMetadata, optional"`
-	BrowserVersion      string `keda:"name=browserVersion,           order=triggerMetadata, default=latest"`
 	UnsafeSsl           bool   `keda:"name=unsafeSsl,                order=triggerMetadata, default=false"`
-	PlatformName        string `keda:"name=platformName,             order=triggerMetadata, default=linux"`
 	NodeMaxSessions     int64  `keda:"name=nodeMaxSessions,          order=triggerMetadata, default=1"`
 
 	TargetValue int64
@@ -96,20 +96,16 @@ type Slot struct {
 }
 
 type Capability struct {
-	BrowserName    string `json:"browserName"`
-	BrowserVersion string `json:"browserVersion"`
-	PlatformName   string `json:"platformName"`
+	BrowserName    string `json:"browserName,omitempty"`
+	BrowserVersion string `json:"browserVersion,omitempty"`
+	PlatformName   string `json:"platformName,omitempty"`
 }
 
 type Stereotypes []struct {
 	Slots      int64      `json:"slots"`
 	Stereotype Capability `json:"stereotype"`
 }
 
-const (
-	DefaultBrowserVersion string = "latest"
-)
-
 func NewSeleniumGridScaler(config *scalersconfig.ScalerConfig) (Scaler, error) {
 	metricType, err := GetMetricTargetType(config)
 	if err != nil {
@@ -227,22 +223,22 @@ func (s *seleniumGridScaler) getSessionsQueueLength(ctx context.Context, logger
 	return newRequestNodes, onGoingSession, nil
 }
 
-func countMatchingSlotsStereotypes(stereotypes Stereotypes, request Capability, browserName string, browserVersion string, sessionBrowserName string, platformName string) int64 {
+func countMatchingSlotsStereotypes(stereotypes Stereotypes, browserName string, browserVersion string, sessionBrowserName string, platformName string) int64 {
 	var matchingSlots int64
 	for _, stereotype := range stereotypes {
-		if checkCapabilitiesMatch(stereotype.Stereotype, request, browserName, browserVersion, sessionBrowserName, platformName) {
+		if checkStereotypeCapabilitiesMatch(stereotype.Stereotype, browserName, browserVersion, sessionBrowserName, platformName) {
 			matchingSlots += stereotype.Slots
 		}
 	}
 	return matchingSlots
 }
 
-func countMatchingSessions(sessions Sessions, request Capability, browserName string, browserVersion string, sessionBrowserName string, platformName string, logger logr.Logger) int64 {
+func countMatchingSessions(sessions Sessions, browserName string, browserVersion string, sessionBrowserName string, platformName string, logger logr.Logger) int64 {
 	var matchingSessions int64
 	for _, session := range sessions {
 		var capability = Capability{}
-		if err := json.Unmarshal([]byte(session.Capabilities), &capability); err == nil {
-			if checkCapabilitiesMatch(capability, request, browserName, browserVersion, sessionBrowserName, platformName) {
+		if err := json.Unmarshal([]byte(session.Slot.Stereotype), &capability); err == nil {
+			if checkStereotypeCapabilitiesMatch(capability, browserName, browserVersion, sessionBrowserName, platformName) {
 				matchingSessions++
 			}
 		} else {
@@ -252,27 +248,40 @@ func countMatchingSessions(sessions Sessions, request Capability, browserName st
 	return matchingSessions
 }
 
-func checkCapabilitiesMatch(capability Capability, requestCapability Capability, browserName string, browserVersion string, sessionBrowserName string, platformName string) bool {
-	// Ensure the logic should be aligned with DefaultSlotMatcher in Selenium Grid - SeleniumHQ/selenium/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java
-	// A browserName matches when one of the following conditions is met:
-	// 1. `browserName` in capability matches with `browserName` or `sessionBrowserName` in scaler metadata
-	// 2. `browserName` in request capability is empty or not provided
-	var browserNameMatches = strings.EqualFold(capability.BrowserName, browserName) || strings.EqualFold(capability.BrowserName, sessionBrowserName) ||
-		requestCapability.BrowserName == ""
-	// A browserVersion matches when one of the following conditions is met:
-	// 1. `browserVersion` in request capability is empty or not provided or `stable`
-	// 2. `browserVersion` in capability matches with prefix of the scaler metadata `browserVersion`
-	// 3. `browserVersion` in scaler metadata is `latest`
-	var browserVersionMatches = requestCapability.BrowserVersion == "" || requestCapability.BrowserVersion == "stable" ||
-		strings.HasPrefix(capability.BrowserVersion, browserVersion) || browserVersion == DefaultBrowserVersion
-	// A platformName matches when one of the following conditions is met:
-	// 1. `platformName` in request capability is empty or not provided
-	// 2. `platformName` in capability is empty or not provided
-	// 3. `platformName` in capability matches with the scaler metadata `platformName`
-	// 4. `platformName` in scaler metadata is empty or not provided
-	var platformNameMatches = requestCapability.PlatformName == "" || capability.PlatformName == "" ||
-		strings.EqualFold(capability.PlatformName, platformName) || platformName == ""
-	return browserNameMatches && browserVersionMatches && platformNameMatches
+// This function checks if the request capabilities match the scaler metadata
+func checkRequestCapabilitiesMatch(request Capability, browserName string, browserVersion string, _ string, platformName string) bool {
+	// Check if browserName matches
+	browserNameMatch := request.BrowserName == "" && browserName == "" ||
+		strings.EqualFold(browserName, request.BrowserName)
+
+	// Check if browserVersion matches
+	browserVersionMatch := (request.BrowserVersion == "" && browserVersion == "") ||
+		(request.BrowserVersion == "stable" && browserVersion == "") ||
+		(strings.HasPrefix(browserVersion, request.BrowserVersion) && request.BrowserVersion != "" && browserVersion != "")
+
+	// Check if platformName matches
+	platformNameMatch := request.PlatformName == "" && platformName == "" ||
+		strings.EqualFold(platformName, request.PlatformName)
+
+	return browserNameMatch && browserVersionMatch && platformNameMatch
+}
+
+// This function checks if Node stereotypes or ongoing sessions match the scaler metadata
+func checkStereotypeCapabilitiesMatch(capability Capability, browserName string, browserVersion string, sessionBrowserName string, platformName string) bool {
+	// Check if browserName matches
+	browserNameMatch := capability.BrowserName == "" && browserName == "" ||
+		strings.EqualFold(browserName, capability.BrowserName) ||
+		strings.EqualFold(sessionBrowserName, capability.BrowserName)
+
+	// Check if browserVersion matches
+	browserVersionMatch := capability.BrowserVersion == "" && browserVersion == "" ||
+		(strings.HasPrefix(browserVersion, capability.BrowserVersion) && capability.BrowserVersion != "" && browserVersion != "")
+
+	// Check if platformName matches
+	platformNameMatch := capability.PlatformName == "" && platformName == "" ||
+		strings.EqualFold(platformName, capability.PlatformName)
+
+	return browserNameMatch && browserVersionMatch && platformNameMatch
 }
 
 func checkNodeReservedSlots(reservedNodes []ReservedNodes, nodeID string, availableSlots int64) int64 {
@@ -318,36 +327,32 @@ func getCountFromSeleniumResponse(b []byte, browserName string, browserVersion s
 		var isRequestMatched bool
 		var requestCapability = Capability{}
 		if err := json.Unmarshal([]byte(sessionQueueRequest), &requestCapability); err == nil {
-			if checkCapabilitiesMatch(requestCapability, requestCapability, browserName, browserVersion, sessionBrowserName, platformName) {
+			if checkRequestCapabilitiesMatch(requestCapability, browserName, browserVersion, sessionBrowserName, platformName) {
 				queueSlots++
 				isRequestMatched = true
 			}
 		} else {
 			logger.Error(err, fmt.Sprintf("Error when unmarshaling sessionQueueRequest capability: %s", err))
 		}
 
-		// Skip the request if the capability does not match the scaler parameters
-		if !isRequestMatched {
-			continue
-		}
-
 		var isRequestReserved bool
-		var sumOfCurrentSessionsMatch int64
 		// Check if the matched request can be assigned to available slots of existing Nodes in the Grid
 		for _, node := range nodes {
-			// Count ongoing sessions that match the request capability and scaler metadata
-			var currentSessionsMatch = countMatchingSessions(node.Sessions, requestCapability, browserName, browserVersion, sessionBrowserName, platformName, logger)
-			sumOfCurrentSessionsMatch += currentSessionsMatch
 			// Check if node is UP and has available slots (maxSession > sessionCount)
-			if strings.EqualFold(node.Status, "UP") && checkNodeReservedSlots(reservedNodes, node.ID, node.MaxSession-node.SessionCount) > 0 {
+			if isRequestMatched && strings.EqualFold(node.Status, "UP") && checkNodeReservedSlots(reservedNodes, node.ID, node.MaxSession-node.SessionCount) > 0 {
 				var stereotypes = Stereotypes{}
 				var availableSlotsMatch int64
 				if err := json.Unmarshal([]byte(node.Stereotypes), &stereotypes); err == nil {
 					// Count available slots that match the request capability and scaler metadata
-					availableSlotsMatch += countMatchingSlotsStereotypes(stereotypes, requestCapability, browserName, browserVersion, sessionBrowserName, platformName)
+					availableSlotsMatch += countMatchingSlotsStereotypes(stereotypes, browserName, browserVersion, sessionBrowserName, platformName)
 				} else {
 					logger.Error(err, fmt.Sprintf("Error when unmarshaling node stereotypes: %s", err))
 				}
+				if availableSlotsMatch == 0 {
+					continue
+				}
+				// Count ongoing sessions that match the request capability and scaler metadata
+				var currentSessionsMatch = countMatchingSessions(node.Sessions, browserName, browserVersion, sessionBrowserName, platformName, logger)
 				// Count remaining available slots can be reserved for this request
 				var availableSlotsCanBeReserved = checkNodeReservedSlots(reservedNodes, node.ID, node.MaxSession-node.SessionCount)
 				// Reserve one available slot for the request if available slots match is greater than current sessions match
@@ -359,11 +364,8 @@ func getCountFromSeleniumResponse(b []byte, browserName string, browserVersion s
 				}
 			}
 		}
-		if sumOfCurrentSessionsMatch > onGoingSessions {
-			onGoingSessions = sumOfCurrentSessionsMatch
-		}
 		// Check if the matched request can be assigned to available slots of new Nodes will be scaled up, since the scaler parameter `nodeMaxSessions` can be greater than 1
-		if !isRequestReserved {
+		if isRequestMatched && !isRequestReserved {
 			for _, newRequestNode := range newRequestNodes {
 				if newRequestNode.SlotCount > 0 {
 					newRequestNodes = updateOrAddReservedNode(newRequestNodes, newRequestNode.ID, newRequestNode.SlotCount-1, nodeMaxSessions)
@@ -373,10 +375,15 @@ func getCountFromSeleniumResponse(b []byte, browserName string, browserVersion s
 			}
 		}
 		// Check if a new Node should be scaled up to reserve for the matched request
-		if !isRequestReserved {
+		if isRequestMatched && !isRequestReserved {
 			newRequestNodes = updateOrAddReservedNode(newRequestNodes, string(rune(requestIndex)), nodeMaxSessions-1, nodeMaxSessions)
 		}
 	}
 
+	// Count ongoing sessions across all nodes that match the scaler metadata
+	for _, node := range nodes {
+		onGoingSessions += countMatchingSessions(node.Sessions, browserName, browserVersion, sessionBrowserName, platformName, logger)
+	}
+
 	return int64(len(newRequestNodes)), onGoingSessions, nil
 }