Solace Scaler: Enhancement to support a hostlist of Solace brokers

[dpochopsky]

Provide a description of what has been changed

Checklist

• [ x] When introducing a new scaler, I agree with the scaling governance policy
• [ x] I have verified that my change is according to the deprecations & breaking changes policy
• [ x] Tests have been added
• [ x] Changelog has been updated and is aligned with our changelog requirements
• [ x] A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
• [ x] A PR is opened to update the documentation on (repo) (if applicable)
• [ x] Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #

Relates to #

[JorTurFer]

Thanks for the contribution! A naive question, what's this for? I mean, which is the use case of multiple brokers?

[dpochopsky]

Thanks for the contribution! A naive question, what's this for? I mean, which is the use case of multiple brokers?

The use case is for Solace deployments with Disaster Recovery (DR) enabled. Solace DR introduces an additional Solace Broker and operates in an active standby mode, so the scaler needs to query two different brokers with different endpoints, verify connectivity status as well as determine which of the brokers are active, once that is deteremined, the scaler operates as it did before.

[JorTurFer]

Interesting point and rally nice addition! The only thing that I'm not sure is about querying all the brokers all the time (if the 1st one isn't the active, it'll be asked all the time). I think that we should just store which one is the active one and relaying of it. As KEDA will recreate the trigger on errors, if the active broker fails, the scaler will fail and KEDA will recreate it, refreshing the active broker

[dpochopsky]

I initially planned on maintaining the brokers' state as you suggested, but I was told there isn't a way to maintain state. Despite that I still gave it a try and couldn't figure out how to do it.

[JorTurFer]

I initially planned on maintaining the brokers' state as you suggested, but I was told there isn't a way to maintain state. Despite that I still gave it a try and couldn't figure out how to do it.

just store it as part of a scaler scoped variable

[dpochopsky]

I initially planned on maintaining the brokers' state as you suggested, but I was told there isn't a way to maintain state. Despite that I still gave it a try and couldn't figure out how to do it.

just store it as part of a scaler scoped variable

ok I'll give that a try, thanks

[dpochopsky]

I initially planned on maintaining the brokers' state as you suggested, but I was told there isn't a way to maintain state. Despite that I still gave it a try and couldn't figure out how to do it.

just store it as part of a scaler scoped variable

enhancement to track which of the hosts are active has been implemented as requested.

[JorTurFer]

You're storing the current index, but you're not using it, so each metric request is querying both solace all the time. I meant to store the current broker to just call to that broker only once it's been detected

[zroubalik]

Could you please open an issue to describe the intention behind this PR? Then link it here and in the Changelog.

Please also link here the related docs PR.

[dpochopsky]

You're storing the current index, but you're not using it, so each metric request is querying both solace all the time. I meant to store the current broker to just call to that broker only once it's been detected

Shown below is the function that initiates the query to retrieve the broker's status. The 'idx' is computed based off the s.curHostIdx from the previous invocation in the first line of the for loop. Upon a successful check of both getVpnState and getQueueState using 'idx' it will return the results to the calling function prior to completing the for loop. I don't see what you mean about not using the idx or it always querying both brokers.

`func (s *SolaceScaler) getSolaceQueueMetricsFromSEMP(ctx context.Context) (SolaceMetricValues, error) {
var metricValues SolaceMetricValues
var errorList []string

    s.stateLock.Lock()
    defer s.stateLock.Unlock()

    //      RETRIEVE METRICS FROM SOLACE SEMP API
    for i := 0; i < len(s.metadata.endpointURLsList); i++ {
            idx := (s.curHostIdx + i) % len(s.metadata.endpointURLsList)
            s.curHostIdx = idx
            sempQueueURL := s.metadata.endpointURLsList[idx]
            sempVpnStateURL := s.metadata.vpnStateURLsList[idx]

            vpnState, err := s.getVpnState(ctx, sempVpnStateURL)
            if err != nil {
                    errorList = append(errorList, "Host "+strconv.Itoa(idx+1)+" Error: "+err.Error())
                    continue
            }

            if vpnState != "up" {
                    errorList = append(errorList, "Host "+strconv.Itoa(idx+1)+" Error: Message vpn is not up ("+vpnState+") url: "+sempVpnStateURL)
                    continue
            }

            metricValues, err = s.getQueueMetrics(ctx, sempQueueURL)
            if err != nil {
                    errorList = append(errorList, "Host "+strconv.Itoa(idx+1)+" Error: "+err.Error())
                    continue
            }

            return metricValues, nil
    }

    return SolaceMetricValues{}, fmt.Errorf("unable to collect metrics, error(s): %s", strings.Join(errorList, "\n  "))

}`

[dpochopsky]

Could you please open an issue to describe the intention behind this PR? Then link it here and in the Changelog.

Please also link here the related docs PR.

Ok I'll open an issue as requested.

Here's the PR for the docs: kedacore/keda-docs#1536

[dpochopsky]

Could you please open an issue to describe the intention behind this PR? Then link it here and in the Changelog.
Please also link here the related docs PR.

Ok I'll open an issue as requested.

Here's the PR for the docs: kedacore/keda-docs#1536

Issue created and added to changelog: #6566