Add NATS JetStream scaler

[goku321]

Add NATS JetStream scaler

Checklist

• Commits are signed with Developer Certificate of Origin (DCO - learn more)
• Tests have been added
• [N/A] A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
• A PR is opened to update the documentation on (repo) (if applicable)
• Changelog has been updated and is aligned with our changelog requirements

Fixes #2391

Documentation kedacore/keda-docs#881

[goku321]

I tested this scaler using kind and could only test with maxReplicaCount: 2 due to limited hardware resources. Will create a docs PR soon. Also, I realized that I've missed unit tests. Will add them as well.

[zroubalik]

@goku321 any update on this please?

[goku321]

@zroubalik I will work on the changes today/tomorrow and make it ready to be merged :)

[zroubalik]

@zroubalik I will work on the changes today/tomorrow and make it ready to be merged :)

@goku321 any updates please? we would need to merge it today in order to get it into the next release, if not that we will target 2.9.

[goku321]

Hi @zroubalik Going to push the changes very soon. Converting tests to go took some time and running tests locally in kind is a bit slower 😅

[zroubalik]

looking good, just a couple of minor nits.

[zroubalik]

@goku321 also please fix the static checks: https://github.com/kedacore/keda/runs/7720862248?check_suite_focus=true

[zroubalik]

/run-e2e nats_jetstream_test*
Update: You can check the progress here

[zroubalik]

@goku321 could you please rebase your branch agains main? There has been some updates recently (we haver removed npm) that are failing here.

[zroubalik]

@goku321 also linter is failing, you can check it locally by running: make golangci

[goku321]

@zroubalik linter is complaining for duplicate code b/w stan and nats_jetstream scaler. Some part is overlapping

[zroubalik]

@zroubalik linter is complaining for duplicate code b/w stan and nats_jetstream scaler. Some part is overlapping

In that case please add exception for your scaler to this file:

keda/.golangci.yml

Line 67 in acb03ab

- dupl

Also, plase squash the commits, so there are only relevant commits in this PR, otherwise the DCO will complain.

[goku321]

@zroubalik I merged from main into this branch a few times instead of rebasing so I'm not sure if it is possible to squash now. Is it possible to do "squash and merge" from the drop down while merging?

[zroubalik]

@goku321 yeah, I can squash those commits. I just don't want to have any unrelated commits here.

[goku321]

@zroubalik Also, I updated .golangci.yml for "nats_jetstream_scaler". Do I need to add a path for "stan_scaler" as well?

[zroubalik]

@zroubalik Also, I updated .golangci.yml for "nats_jetstream_scaler". Do I need to add a path for "stan_scaler" as well?

I am not sure, but given the error it seems like that you'd have to do that :( Try that locally and you will see.

Also please fix the package name in the e2e test, see the other linter error. Then we are hopefully good to test this scaler here in the PR and the only outstanding issue will be docs PR 🤞

[zroubalik]

/run-e2e nats_jetstream_test*
Update: You can check the progress here

[zroubalik]

LGTM

[zroubalik]

@goku321 just the Documentation is missing.

[goku321]

@zroubalik Working on the documentation 🙂

[zroubalik]

@zroubalik Working on the documentation 🙂

@goku321 what is the status? If we don't merge it today it won't get into the release unfortunately

[goku321]

@zroubalik Added the documentation. Thanks for your patience 😅

[zroubalik]

@goku321 I have just realized that the PR is missing a Unit test. I completely missed this somehow.

Could you please add one, it's quite an easy task, if you look at other scalers.

[goku321]

@zroubalik I had that in back of my mind but forgot about it. Will add for sure