Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/main' into fix-5570
Browse files Browse the repository at this point in the history
Signed-off-by: Yury Akudovich <[email protected]>
  • Loading branch information
yorik committed Jan 17, 2025
2 parents 538d7ba + fca23b9 commit 8a996cc
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 33 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ Here is an overview of all new **experimental** features:

- **General**: Fix event text when deactivation fails ([#6469](https://github.com/kedacore/keda/issues/6469))
- **AWS Scalers**: Add AWS region to the AWS Config Cache key ([#6128](https://github.com/kedacore/keda/issues/6128))
- **Selenium Grid**: Scaler logic on platformName is set empty or `any` ([#6477](https://github.com/kedacore/keda/issues/6477))

### Deprecations

Expand All @@ -88,8 +89,9 @@ New deprecation(s):

### Breaking Changes

- Change `InitialCooldownPeriod` from `int32` to `*int32` ([#6423](https://github.com/kedacore/keda/issues/6423))
- **General**: Change `InitialCooldownPeriod` from `int32` to `*int32` ([#6423](https://github.com/kedacore/keda/issues/6423))
- **General**: Remove Prometheus metric deprecations ([#6339](https://github.com/kedacore/keda/pull/6339))
- **External Scaler**: Remove deprecated tlsCertFile from External scaler ([#4549](https://github.com/kedacore/keda/issues/4549))

### Other

Expand Down
26 changes: 4 additions & 22 deletions pkg/scalers/external_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ type externalPushScaler struct {

type externalScalerMetadata struct {
scalerAddress string
tlsCertFile string
originalMetadata map[string]string
triggerIndex int
caCert string
Expand Down Expand Up @@ -116,10 +115,6 @@ func parseExternalScalerMetadata(config *scalersconfig.ScalerConfig) (externalSc
return meta, fmt.Errorf("scaler Address is a required field")
}

if val, ok := config.TriggerMetadata["tlsCertFile"]; ok && val != "" {
meta.tlsCertFile = val
}

meta.originalMetadata = make(map[string]string)
if val, ok := config.AuthParams["caCert"]; ok {
meta.caCert = val
Expand Down Expand Up @@ -164,7 +159,7 @@ func (s *externalScaler) Close(context.Context) error {
func (s *externalScaler) GetMetricSpecForScaling(ctx context.Context) []v2.MetricSpec {
var result []v2.MetricSpec

grpcClient, err := getClientForConnectionPool(s.metadata, s.logger)
grpcClient, err := getClientForConnectionPool(s.metadata)
if err != nil {
s.logger.Error(err, "error building grpc connection")
return result
Expand Down Expand Up @@ -199,7 +194,7 @@ func (s *externalScaler) GetMetricSpecForScaling(ctx context.Context) []v2.Metri
// GetMetricsAndActivity returns value for a supported metric and an error if there is a problem getting the metric
func (s *externalScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) {
var metrics []external_metrics.ExternalMetricValue
grpcClient, err := getClientForConnectionPool(s.metadata, s.logger)
grpcClient, err := getClientForConnectionPool(s.metadata)
if err != nil {
return []external_metrics.ExternalMetricValue{}, false, err
}
Expand Down Expand Up @@ -240,7 +235,7 @@ func (s *externalPushScaler) Run(ctx context.Context, active chan<- bool) {
defer close(active)
// It's possible for the connection to get terminated anytime, we need to run this in a retry loop
runWithLog := func() {
grpcClient, err := getClientForConnectionPool(s.metadata, s.logger)
grpcClient, err := getClientForConnectionPool(s.metadata)
if err != nil {
s.logger.Error(err, "error running internalRun")
return
Expand Down Expand Up @@ -301,24 +296,11 @@ var connectionPoolMutex sync.Mutex

// getClientForConnectionPool returns a grpcClient and a done() Func. The done() function must be called once the client is no longer
// in use to clean up the shared grpc.ClientConn
func getClientForConnectionPool(metadata externalScalerMetadata, logger logr.Logger) (pb.ExternalScalerClient, error) {
func getClientForConnectionPool(metadata externalScalerMetadata) (pb.ExternalScalerClient, error) {
connectionPoolMutex.Lock()
defer connectionPoolMutex.Unlock()

buildGRPCConnection := func(metadata externalScalerMetadata) (*grpc.ClientConn, error) {
// FIXME: DEPRECATED to be removed in v2.13 https://github.com/kedacore/keda/issues/4549
if metadata.tlsCertFile != "" {
logger.V(1).Info("tlsCertFile in ScaleObject metadata will be deprecated in v2.12. Please use" +
"tlsClientCert, tlsClientKey and caCert in TriggerAuthentication instead.")
creds, err := credentials.NewClientTLSFromFile(metadata.tlsCertFile, "")
if err != nil {
return nil, err
}
return grpc.NewClient(metadata.scalerAddress,
grpc.WithDefaultServiceConfig(grpcConfig),
grpc.WithTransportCredentials(creds))
}

tlsConfig, err := util.NewTLSConfig(metadata.tlsClientCert, metadata.tlsClientKey, metadata.caCert, metadata.unsafeSsl)
if err != nil {
return nil, err
Expand Down
19 changes: 9 additions & 10 deletions pkg/scalers/selenium_grid_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,35 +251,34 @@ func countMatchingSessions(sessions Sessions, browserName string, browserVersion
// 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 == "" ||
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 != "")
(request.BrowserVersion != "" && strings.HasPrefix(browserVersion, request.BrowserVersion))

// Check if platformName matches
platformNameMatch := request.PlatformName == "" && platformName == "" ||
strings.EqualFold(platformName, request.PlatformName)
platformNameMatch := (request.PlatformName == "" || strings.EqualFold("any", request.PlatformName) || strings.EqualFold(platformName, request.PlatformName)) &&
(platformName == "" || platformName == "any" || 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 == "" ||
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 != "")
browserVersionMatch := (capability.BrowserVersion == "" && browserVersion == "") ||
(capability.BrowserVersion != "" && strings.HasPrefix(browserVersion, capability.BrowserVersion))

// Check if platformName matches
platformNameMatch := capability.PlatformName == "" && platformName == "" ||
strings.EqualFold(platformName, capability.PlatformName)
platformNameMatch := (capability.PlatformName == "" || strings.EqualFold("any", capability.PlatformName) || strings.EqualFold(platformName, capability.PlatformName)) &&
(platformName == "" || platformName == "any" || strings.EqualFold(platformName, capability.PlatformName))

return browserNameMatch && browserVersionMatch && platformNameMatch
}
Expand Down
68 changes: 68 additions & 0 deletions pkg/scalers/selenium_grid_scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,74 @@ func Test_getCountFromSeleniumResponse(t *testing.T) {
wantOnGoingSessions: 2,
wantErr: false,
},
{
name: "1 queue request without platformName and scaler metadata without platfromName should return 1 new node and 1 ongoing session",
args: args{
b: []byte(`{
"data": {
"grid": {
"sessionCount": 2,
"maxSession": 2,
"totalSlots": 2
},
"nodesInfo": {
"nodes": [
{
"id": "node-1",
"status": "UP",
"sessionCount": 1,
"maxSession": 1,
"slotCount": 1,
"stereotypes": "[{\"slots\": 1, \"stereotype\": {\"browserName\": \"chrome\", \"platformName\": \"any\"}}]",
"sessions": [
{
"id": "session-1",
"capabilities": "{\"browserName\": \"chrome\", \"platformName\": \"any\"}",
"slot": {
"id": "9ce1edba-72fb-465e-b311-ee473d8d7b64",
"stereotype": "{\"browserName\": \"chrome\", \"platformName\": \"any\"}"
}
}
]
},
{
"id": "node-2",
"status": "UP",
"sessionCount": 1,
"maxSession": 1,
"slotCount": 1,
"stereotypes": "[{\"slots\": 1, \"stereotype\": {\"browserName\": \"chrome\", \"platformName\": \"linux\"}}]",
"sessions": [
{
"id": "session-2",
"capabilities": "{\"browserName\": \"chrome\", \"browserVersion\": \"91.0\", \"platformName\": \"linux\"}",
"slot": {
"id": "9ce1edba-72fb-465e-b311-ee473d8d7b64",
"stereotype": "{\"browserName\": \"chrome\", \"platformName\": \"linux\"}"
}
}
]
}
]
},
"sessionsInfo": {
"sessionQueueRequests": [
"{\"browserName\": \"chrome\", \"platformName\": \"linux\"}",
"{\"browserName\": \"chrome\"}",
"{\"browserName\": \"chrome\", \"platformName\": \"any\"}"
]
}
}
}`),
browserName: "chrome",
sessionBrowserName: "chrome",
browserVersion: "",
platformName: "",
},
wantNewRequestNodes: 2,
wantOnGoingSessions: 1,
wantErr: false,
},
{
name: "1 active session with matching browsername and version should return count as 2",
args: args{
Expand Down

0 comments on commit 8a996cc

Please sign in to comment.