squakez commented on code in PR #6893: URL: https://github.com/apache/camel-k/pull/6893#discussion_r4356974887
########## e2e/arkmq/arkmq_keda_test.go: ########## @@ -0,0 +1,78 @@ +//go:build integration +// +build integration + +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package arkmq + +import ( + "context" + "testing" + + . "github.com/onsi/gomega" + + . "github.com/apache/camel-k/v2/e2e/support" +) + +func TestArkMQKedaAutoDiscovery(t *testing.T) { + WithExistingNamedTestNamespace(t, func(ctx context.Context, g *WithT, ns string) { + t.Run("Auto-discovery ArkMQ queue", func(t *testing.T) { + ExpectExecSucceed(t, g, Kubectl("apply", "-f", "files/keda-arkmq-to-log-auto.yaml")) + integrationName := "keda-arkmq-to-log-auto" + + g.Eventually(ScaledObject(t, ctx, ns, integrationName), TestTimeoutMedium). + ShouldNot(BeNil()) + + scaledObj := ScaledObject(t, ctx, ns, integrationName)() + g.Expect(scaledObj).NotTo(BeNil()) + g.Expect(scaledObj.Spec.Triggers).To(HaveLen(1)) + g.Expect(scaledObj.Spec.Triggers[0].Type).To(Equal("artemis-queue")) + // Auto-discovered from the amqp:queue URI + g.Expect(scaledObj.Spec.Triggers[0].Metadata["queueName"]).To(Equal("my-queue")) + g.Expect(scaledObj.Spec.Triggers[0].Metadata["brokerAddress"]).To(Equal("my-queue")) + g.Expect(scaledObj.Spec.Triggers[0].Metadata["managementEndpoint"]).To(Equal("my-broker-hdls-svc.arkmq.svc:8161")) + // Merged from autoMetadata + g.Expect(scaledObj.Spec.Triggers[0].Metadata["brokerName"]).To(Equal("amq-broker")) + Review Comment: We miss a check to verify the application is running and scaled to zero properly. I think you can get the very same behavior we're testing in Kafka test. ########## e2e/arkmq/files/keda-arkmq-to-log-auto.yaml: ########## @@ -0,0 +1,43 @@ +# --------------------------------------------------------------------------- +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# --------------------------------------------------------------------------- + +# This pipe tests KEDA auto-discovery for an ArkMQ queue: +# - artemis-queue trigger is auto-discovered from the amqp:queue URI +# - management endpoint is derived from the broker url +# - broker name is merged via autoMetadata + +apiVersion: camel.apache.org/v1 +kind: Pipe +metadata: + name: keda-arkmq-to-log-auto + namespace: arkmq +spec: + source: + ref: + kind: ActiveMQArtemisAddress + apiVersion: broker.amq.io/v1beta1 + name: my-queue + sink: + uri: log:info + traits: + keda: + enabled: true + minReplicaCount: 0 + maxReplicaCount: 1 + autoMetadata: + artemis-queue: + brokerName: amq-broker Review Comment: Is there any possible to derive this param instead? ########## pkg/trait/keda/mapper.go: ########## @@ -51,7 +53,14 @@ func MapToKedaTrigger(rawURI string) (*traitv1.KedaTrigger, error) { if !found { return nil, nil } - kedaType, metadata := mapper.Map(pathValue, params) + prefix := "camel.component." + scheme + "." Review Comment: Why don't we provide instead the full map? Each scaler can filter the specific fields on its own. If we keep this generic we enable potentially other scalers with more capability. ########## pkg/trait/keda/scalers/artemis.go: ########## @@ -0,0 +1,54 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package scalers + +import ( + "net" + "net/url" + "strings" + + keda "github.com/apache/camel-k/v2/pkg/trait/keda" +) + +// ArtemisScaler maps Camel AMQP queue endpoints to the KEDA artemis-queue scaler. +type ArtemisScaler struct{} + +func init() { + keda.Register(&ArtemisScaler{}) +} + +func (a *ArtemisScaler) Component() string { + return "amqp" +} + +func (a *ArtemisScaler) Map(pathValue string, params map[string]string, props map[string]string) (string, map[string]string) { + metadata := make(map[string]string) + queueName := strings.TrimPrefix(pathValue, "queue:") + if queueName != "" && !strings.HasPrefix(queueName, "topic:") { + metadata["queueName"] = queueName + // For anycast queues created by ArkMQ, the address name matches the queue name. + metadata["brokerAddress"] = queueName + } + if brokerURL, ok := props["broker-url"]; ok { + if u, err := url.Parse(brokerURL); err == nil && u.Hostname() != "" { + metadata["managementEndpoint"] = net.JoinHostPort(u.Hostname(), "8161") Review Comment: I think we can derive the port as well from the "broker-url". Can you verify if that is expected to be part of the parameter? ########## pkg/trait/keda.go: ########## @@ -183,3 +185,22 @@ func (t *kedaTrait) autoDiscoverTriggers(e *Environment) error { return nil } + +func integrationProperties(e *Environment) map[string]string { Review Comment: nitpick. We should move this func into `trait.go` or other generic file as it can be reused in theory. It still needs to be private though. ########## e2e/arkmq/arkmq_keda_test.go: ########## @@ -0,0 +1,78 @@ +//go:build integration +// +build integration + +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package arkmq + +import ( + "context" + "testing" + + . "github.com/onsi/gomega" + + . "github.com/apache/camel-k/v2/e2e/support" +) + +func TestArkMQKedaAutoDiscovery(t *testing.T) { + WithExistingNamedTestNamespace(t, func(ctx context.Context, g *WithT, ns string) { + t.Run("Auto-discovery ArkMQ queue", func(t *testing.T) { + ExpectExecSucceed(t, g, Kubectl("apply", "-f", "files/keda-arkmq-to-log-auto.yaml")) + integrationName := "keda-arkmq-to-log-auto" + + g.Eventually(ScaledObject(t, ctx, ns, integrationName), TestTimeoutMedium). + ShouldNot(BeNil()) + + scaledObj := ScaledObject(t, ctx, ns, integrationName)() + g.Expect(scaledObj).NotTo(BeNil()) + g.Expect(scaledObj.Spec.Triggers).To(HaveLen(1)) + g.Expect(scaledObj.Spec.Triggers[0].Type).To(Equal("artemis-queue")) + // Auto-discovered from the amqp:queue URI + g.Expect(scaledObj.Spec.Triggers[0].Metadata["queueName"]).To(Equal("my-queue")) + g.Expect(scaledObj.Spec.Triggers[0].Metadata["brokerAddress"]).To(Equal("my-queue")) + g.Expect(scaledObj.Spec.Triggers[0].Metadata["managementEndpoint"]).To(Equal("my-broker-hdls-svc.arkmq.svc:8161")) + // Merged from autoMetadata + g.Expect(scaledObj.Spec.Triggers[0].Metadata["brokerName"]).To(Equal("amq-broker")) + + ExpectExecSucceed(t, g, Kubectl("delete", "pipe", integrationName, "-n", ns)) + }) + }, "arkmq") +} + +func TestArkMQKedaAutoDiscoveryIntegration(t *testing.T) { + WithExistingNamedTestNamespace(t, func(ctx context.Context, g *WithT, ns string) { + t.Run("Auto-discovery ArkMQ queue from Integration", func(t *testing.T) { + ExpectExecSucceed(t, g, Kubectl("apply", "-f", "files/keda-it-arkmq-to-log-auto.yaml")) + integrationName := "keda-it-arkmq-to-log-auto" + + g.Eventually(ScaledObject(t, ctx, ns, integrationName), TestTimeoutMedium). + ShouldNot(BeNil()) + + scaledObj := ScaledObject(t, ctx, ns, integrationName)() + g.Expect(scaledObj).NotTo(BeNil()) + g.Expect(scaledObj.Spec.Triggers).To(HaveLen(1)) + g.Expect(scaledObj.Spec.Triggers[0].Type).To(Equal("artemis-queue")) + g.Expect(scaledObj.Spec.Triggers[0].Metadata["queueName"]).To(Equal("my-queue")) + g.Expect(scaledObj.Spec.Triggers[0].Metadata["brokerAddress"]).To(Equal("my-queue")) + g.Expect(scaledObj.Spec.Triggers[0].Metadata["managementEndpoint"]).To(Equal("my-broker-hdls-svc.arkmq.svc:8161")) + g.Expect(scaledObj.Spec.Triggers[0].Metadata["brokerName"]).To(Equal("amq-broker")) + Review Comment: Ditto -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
