Add e2e test for IBM MQ Scaler

[zroubalik]

IBM MQ Scaler issue: #1253

[zroubalik]

@rcoppen @jessm12 @cpilton @NisheekaNynan1

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[JorTurFer]

There is a helm chart we can use to run IBM MQ inside testing clusters https://github.com/ibm-messaging/mq-helm/blob/main/charts/ibm-mq/README.md

[ayoyu]

Hi @zroubalik could you please assign this issue to me if it's not already being addressed. Thanks

[JorTurFer]

Thanks!

[ayoyu]

Hi @JorTurFer, After taking some time to play with the ibm-mq scaler in order to prepare the e2e test, I discovered some minor bugs and improvements that can be done on the current state of the ibm-scaler and I wanted to have your feedback about them before I apply them.

• Typo while parsing TriggerMetadata tlsDisabled field . The currently used key is config.TriggerMetadata["tls"]. See https://github.com/kedacore/keda/blob/main/pkg/scalers/ibmmq_scaler.go#L135 And as mentioned in the scaler documentation https://github.com/kedacore/keda-docs/blob/main/content/docs/2.15/scalers/ibm-mq.md tlsDisabled is the correct one. For this reason setting tlsDisabled: true to disable TLS is not taking into account.

• Embedding an error that will always be nil, see https://github.com/kedacore/keda/blob/main/pkg/scalers/ibmmq_scaler.go#L201
  As you can notice we already checked above if the err != nil and returning it just after, so the embedded error will always be nil. In this case I suggest probably to return the error that indicates that the []Response is empty. Example from the operator log:

2024-05-11T15:28:42Z	INFO	Reconciling ScaledObject	{"controller": "scaledobject", "controllerGroup": "keda.sh", "controllerKind": "ScaledObject", "ScaledObject": {"name":"ibmmq-consumer","namespace":"ibm-mq"}, "namespace": "ibm-mq", "name": "ibmmq-consumer", "reconcileID": "cc31d473-4c9a-4c2b-b5ae-7451e0e316a6"}
2024-05-11T15:28:42Z	INFO	Detected resource targeted for scaling	{"controller": "scaledobject", "controllerGroup": "keda.sh", "controllerKind": "ScaledObject", "ScaledObject": {"name":"ibmmq-consumer","namespace":"ibm-mq"}, "namespace": "ibm-mq", "name": "ibmmq-consumer", "reconcileID": "cc31d473-4c9a-4c2b-b5ae-7451e0e316a6", "resource": "apps/v1.Deployment", "name": "ibmmq-consumer"}
2024-05-11T15:28:42Z	INFO	Updated HPA according to ScaledObject	{"controller": "scaledobject", "controllerGroup": "keda.sh", "controllerKind": "ScaledObject", "ScaledObject": {"name":"ibmmq-consumer","namespace":"ibm-mq"}, "namespace": "ibm-mq", "name": "ibmmq-consumer", "reconcileID": "cc31d473-4c9a-4c2b-b5ae-7451e0e316a6", "HPA.Namespace": "ibm-mq", "HPA.Name": "keda-hpa-ibmmq-consumer"}
2024-05-11T15:28:42Z	INFO	Initializing Scaling logic according to ScaledObject Specification	{"controller": "scaledobject", "controllerGroup": "keda.sh", "controllerKind": "ScaledObject", "ScaledObject": {"name":"ibmmq-consumer","namespace":"ibm-mq"}, "namespace": "ibm-mq", "name": "ibmmq-consumer", "reconcileID": "cc31d473-4c9a-4c2b-b5ae-7451e0e316a6"}
2024-05-11T15:28:42Z	INFO	Reconciling ScaledObject	{"controller": "scaledobject", "controllerGroup": "keda.sh", "controllerKind": "ScaledObject", "ScaledObject": {"name":"ibmmq-consumer","namespace":"ibm-mq"}, "namespace": "ibm-mq", "name": "ibmmq-consumer", "reconcileID": "a8fac534-f7ef-4722-9c77-5bb10b262919"}
2024-05-11T15:28:42Z	INFO	Detected resource targeted for scaling	{"controller": "scaledobject", "controllerGroup": "keda.sh", "controllerKind": "ScaledObject", "ScaledObject": {"name":"ibmmq-consumer","namespace":"ibm-mq"}, "namespace": "ibm-mq", "name": "ibmmq-consumer", "reconcileID": "a8fac534-f7ef-4722-9c77-5bb10b262919", "resource": "apps/v1.Deployment", "name": "ibmmq-consumer"}
+2024-05-11T15:28:43Z	ERROR	scale_handler	error getting scale decision	{"scaledObject.Namespace": "ibm-mq", "scaledObject.Name": "ibmmq-consumer", "scaler": "IBMMQScaler", "error": "error inspecting IBM MQ queue depth: failed to parse response from REST call: %!w(<nil>)"}
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).getScalerState
	/workspace/pkg/scaling/scale_handler.go:783
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).getScaledObjectState.func1
	/workspace/pkg/scaling/scale_handler.go:636

• Not checking the response body for the POST MQSC command, see https://www.ibm.com/docs/en/ibm-mq/9.2?topic=adminactionqmgrqmgrnamemqsc-post-json-formatted-command#q133340___responseformat__title__1
  With the current function getQueueDepthViaHTTP https://github.com/kedacore/keda/blob/main/pkg/scalers/ibmmq_scaler.go#L168 even if the queue doesn't exists, we will return 0 as the CURDEPTH and this can lead to scale decision after on the targeted resource. Examples:

$ go run get_curr_depth.go bar
mqsc command: {"type": "runCommandJSON", "command": "display", "qualifier": "qlocal", "name": "bar", "responseParameters" : ["CURDEPTH"]}

{
  "commandResponse": [{
    "completionCode": 2,
    "reasonCode": 2085,
    "message": ["AMQ8147E: IBM MQ object bar not found."]
  }],
  "overallReasonCode": 3008,
  "overallCompletionCode": 2
}
0 <nil>
$ go run get_curr_depth.go DEV.QUEUE.1
mqsc command: {"type": "runCommandJSON", "command": "display", "qualifier": "qlocal", "name": "DEV.QUEUE.1", "responseParameters" : ["CURDEPTH"]}

{
  "commandResponse": [{
    "completionCode": 0,
    "reasonCode": 0,
    "parameters": {
      "curdepth": 0,
      "type": "QLOCAL",
      "queue": "DEV.QUEUE.1"
    }
  }],
  "overallReasonCode": 0,
  "overallCompletionCode": 0
}
0 <nil>

• The get_curr_depth.go used for the example above (similar to https://github.com/kedacore/keda/blob/main/pkg/scalers/ibmmq_scaler.go#L168)

package main

import (
	"bytes"
	"context"
	"crypto/tls"
	"encoding/json"
	"fmt"
	"io"
	"net/http"
	"os"
)

// CommandResponse Full structured response from MQ admin REST query
type CommandResponse struct {
	CommandResponse []Response `json:"commandResponse"`
}

// Response The body of the response returned from the MQ admin query
type Response struct {
	Parameters Parameters `json:"parameters"`
}

// Parameters Contains the current depth of the IBM MQ Queue
type Parameters struct {
	Curdepth int `json:"curdepth"`
}

// getQueueDepthViaHTTP returns the depth of the MQ Queue from the Admin endpoint
func getQueueDepthViaHTTP(ctx context.Context, url, queue string) (int64, error) {
	var requestJSON = []byte(`{"type": "runCommandJSON", "command": "display", "qualifier": "qlocal", "name": "` + queue + `", "responseParameters" : ["CURDEPTH"]}`)

	fmt.Printf("mqsc command: %s\n\n", string(requestJSON))

	req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewBuffer(requestJSON))
	if err != nil {
		return 0, fmt.Errorf("failed to request queue depth: %w", err)
	}
	req.Header.Set("ibm-mq-rest-csrf-token", "value")
	req.Header.Set("Content-Type", "application/json")
	req.SetBasicAuth("admin", "my-admin-password")

	client := http.Client{
		Transport: &http.Transport{
			TLSClientConfig: &tls.Config{
				InsecureSkipVerify: true,
			},
			Proxy: http.ProxyFromEnvironment,
		},
	}
	resp, err := client.Do(req)
	if err != nil {
		return 0, fmt.Errorf("failed to contact MQ via REST: %w", err)
	}
	defer resp.Body.Close()

	body, err := io.ReadAll(resp.Body)
	if err != nil {
		return 0, fmt.Errorf("failed to ready body of request: %w", err)
	}
	fmt.Println(string(body))
	var response CommandResponse
	err = json.Unmarshal(body, &response)
	if err != nil {
		return 0, fmt.Errorf("failed to parse JSON: %w", err)
	}

	if response.CommandResponse == nil || len(response.CommandResponse) == 0 {
		return 0, fmt.Errorf("failed to parse response from REST call: %v . %w",
			response.CommandResponse, err)
	}

	return int64(response.CommandResponse[0].Parameters.Curdepth), nil
}

func main() {
	queueName := os.Args[1]
	url := "https://192.168.49.2:30948/ibmmq/rest/v2/admin/action/qmgr/ibmmqnonativeha/mqsc"
	fmt.Println(getQueueDepthViaHTTP(context.TODO(), url, queueName))
}

Thanks in advance for your feedback.

[JorTurFer]

Hello!

• Typo while parsing TriggerMetadata tlsDisabled field . The currently used key is config.TriggerMetadata["tls"]. See main/pkg/scalers/ibmmq_scaler.go#L135 And as mentioned in the scaler documentation kedacore/keda-docs@main/content/docs/2.15/scalers/ibm-mq.md tlsDisabled is the correct one. For this reason setting tlsDisabled: true to disable TLS is not taking into account.

I'd update docs to reflects the current code, as the code is the real usage.

Embedding an error that will always be nil, see main/pkg/scalers/ibmmq_scaler.go#L201
As you can notice we already checked above if the err != nil and returning it just after, so the embedded error will always be nil. In this case I suggest probably to return the error that indicates that the []Response is empty. Example from the operator log:

It's a good idea :)

Not checking the response body for the POST MQSC command, see ibm.com/docs/en/ibm-mq/9.2?topic=adminactionqmgrqmgrnamemqsc-post-json-formatted-command#q133340___responseformat__title__1
With the current function getQueueDepthViaHTTP main/pkg/scalers/ibmmq_scaler.go#L168 even if the queue doesn't exists, we will return 0 as the CURDEPTH and this can lead to scale decision after on the targeted resource. Examples:

I'd say that not existing queue should return an error

WDYT about these points @zroubalik @tomkerkhove ?

[JorTurFer]

BTW, the docker image has been already created with this tag ghcr.io/kedacore/tests-ibmmq:latest

[zroubalik]

Hello!

• Typo while parsing TriggerMetadata tlsDisabled field . The currently used key is config.TriggerMetadata["tls"]. See main/pkg/scalers/ibmmq_scaler.go#L135 And as mentioned in the scaler documentation kedacore/keda-docs@main/content/docs/2.15/scalers/ibm-mq.md tlsDisabled is the correct one. For this reason setting tlsDisabled: true to disable TLS is not taking into account.

I'd update docs to reflects the current code, as the code is the real usage.

Embedding an error that will always be nil, see main/pkg/scalers/ibmmq_scaler.go#L201
As you can notice we already checked above if the err != nil and returning it just after, so the embedded error will always be nil. In this case I suggest probably to return the error that indicates that the []Response is empty. Example from the operator log:

It's a good idea :)

Not checking the response body for the POST MQSC command, see ibm.com/docs/en/ibm-mq/9.2?topic=adminactionqmgrqmgrnamemqsc-post-json-formatted-command#q133340___responseformat__title__1
With the current function getQueueDepthViaHTTP main/pkg/scalers/ibmmq_scaler.go#L168 even if the queue doesn't exists, we will return 0 as the CURDEPTH and this can lead to scale decision after on the targeted resource. Examples:

I'd say that not existing queue should return an error

WDYT about these points @zroubalik @tomkerkhove ?

agree