feat(Scaler): Adds Loki Scaler #3700

neelanjan00 · 2022-09-27T17:45:29Z

Provide a description of what has been changed

Adds Loki scaler for autoscaling based on Loki LogQL queries

Checklist

When introducing a new scaler, I agree with the scaling governance policy
Commits are signed with Developer Certificate of Origin (DCO - learn more)
Tests have been added
A PR is opened to update the documentation on (repo) (if applicable)
Changelog has been updated and is aligned with our changelog requirements

Relates to #3699

Relates to kedacore/keda-docs#948

Signed-off-by: Neelanjan Manna <[email protected]>

JorTurFer

First, thanks a ton for this awesome contribution! 🙇
In general looks good, I have left some comments inline. Apart from that, could you add an e2e test for this scaler?
All e2e test are in /tests folder and there is also a readme explaining how to add them :)

CHANGELOG.md

config/manager/kustomization.yaml

config/metrics-server/kustomization.yaml

pkg/generated/clientset/versioned/fake/register.go

pkg/generated/clientset/versioned/scheme/register.go

pkg/scalers/loki_scaler.go

… tenant name field Signed-off-by: Neelanjan Manna <[email protected]>

Signed-off-by: Neelanjan Manna <[email protected]>

neelanjan00 · 2022-10-05T15:01:39Z

Hi @JorTurFer, thanks for reviewing the PR, I have resolved all the points that you commented upon.

Regarding the addition of an E2E suite for the scaler, I have been trying to understand how it can be achieved by taking reference from the other scaler's tests and the provided README. While I have a rudimentary idea of how it can be done for Loki scaler, I would still like to discuss it through first.

Here's what I have understood so far regarding the pre-requisites:

Create helper functions for installing and cleaning up Loki in k8s.
Create an application deployment to be scaled using the scaler.
Create Jobs to simulate the event on which the application deployment should be scaled.

The logic of the test is about validating whether activation/scale-up/scale-down is occurring or not.

Based on my understanding I have a few doubts:

How can we install/uninstall Loki for the test without using k8s manifests? The docs page doesn't provide any information about using k8s manifests, only Helm is supported here: https://grafana.com/docs/loki/latest/installation/
Are there any special requirements for the demo application deployment container and the Job container to be used?

JorTurFer · 2022-10-05T19:59:59Z

There are several e2e test were resources are installed/removed during the tests, for instance in this test we install/remove Hashicorp Vault.
WRT to how to install loki, you can use helm, there are other e2e tests which use helm to install deps.
Finally, wrt to the requirements, we don't have any special requirement to this, you can use whatever you want but we try to keep it simpler as possible.
If you need to create a new image to be used in the test, you can do it but in that case we prefer to have the code in our test-tools repo and use our own image (to avoid issues in the future if the upstream deletes them). In that case, the requirement is that the code should be in golang if it's possible. As an example about how to add a new image, I merged this PR last week adding and image for event hub e2e tests

JorTurFer · 2022-10-05T20:02:06Z

WRT the e2e test itself, you are right, the process should be something like:

Create the namespace, install deps (loki) and spawn the needed resources
TestActivation
TestScaleUp
TestScaleDown
Clean up the resources

You can check any already existing e2e test to validate the process or ping us if you have any doubt 😄

v-shenoy · 2022-10-08T02:29:35Z

By the way, there are still some kustomization / generated files that are edited in this PR.

Signed-off-by: Neelanjan Manna <[email protected]>

CHANGELOG.md

go.mod

pkg/scalers/loki_scaler.go

tests/scalers/loki/loki_test.go

pkg/generated/clientset/versioned/scheme/register.go

pkg/generated/clientset/versioned/fake/register.go

neelanjan00 · 2022-10-10T15:07:45Z

Hey @JorTurFer, thank you so much for all the helpful feedback. I should mention that this PR is still a WIP, still trying to get the E2E to work, it's failing due to some mysterious FQDN issue. I will slowly resolve all of the issues by the end of this week (hopefully). Thank you so much once again for all the help! 🙏🏽

…dded create namespace function, minor fixes Signed-off-by: Neelanjan Manna <[email protected]>

Signed-off-by: Neelanjan Manna <[email protected]>

neelanjan00 · 2022-10-10T16:21:53Z

Resolved all the issues. However still facing issues with the execution of the E2E test itself: I am able to use the scaler otherwise manually, but when run through E2E, the test application doesn't scale up. I have checked for all the possible causes that may be responsible but in vain, including the operator pod logs and metrics-server pod logs. What would you suggest, @JorTurFer?

neelanjan00 · 2022-10-10T16:24:07Z

Attaching E2E logs for reference.
loki-e2e.log

JorTurFer · 2022-10-10T16:52:41Z

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

v-shenoy · 2022-10-11T15:48:48Z

Were you able to check the scaling manually @neelanjan00?
You can also manually check the metric that KEDA is fetching and ensure that it is correct.
https://keda.sh/docs/2.8/operate/metrics-server/

neelanjan00 · 2022-10-11T15:54:45Z

Were you able to check the scaling manually @neelanjan00?
You can also manually check the metric that KEDA is fetching and ensure that it is correct.
https://keda.sh/docs/2.8/operate/metrics-server/

Yes I am able to run the scaler manually.

JorTurFer · 2022-10-11T19:16:08Z

I have reviewed the execution log and there isn't any error so I guess that the issue is in the e2e test logic side. Let me pull your code and take a look deeper

tests/scalers/loki/loki_test.go

…he substring being searched in the query Signed-off-by: Neelanjan Manna <[email protected]>

Signed-off-by: Neelanjan Manna <[email protected]>

JorTurFer · 2022-10-12T15:56:23Z

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

JorTurFer

Looking good, only some nits inline

pkg/scalers/loki_scaler.go

tests/scalers/loki/loki_test.go

pkg/scalers/loki_scaler.go

…rmalization Signed-off-by: Neelanjan Manna <[email protected]>

Signed-off-by: Neelanjan Manna <[email protected]>

JorTurFer · 2022-10-12T20:49:06Z

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

JorTurFer

LGTM!
Thanks for this awesome contribution!
Let's wait till @zroubalik or @v-shenoy takes a look too

v-shenoy

I am not an expert when it comes to Loki, but LGTM. Nice work!

Left some minor comments on the docs PR.

neelanjan00 · 2022-10-13T10:46:49Z

I am not an expert when it comes to Loki, but LGTM. Nice work!

Left some minor comments on the docs PR.

Thanks for the review, I have updated the docs PR as well based on your comments.

Signed-off-by: 26tanishabanik <[email protected]>

neelanjan00 added 11 commits September 26, 2022 18:12

added loki scaler

e7fc13d

Signed-off-by: Neelanjan Manna <[email protected]>

Merge branch 'main' into loki-scaler

a33e49d

added correct context to newRequest

8b71e20

Signed-off-by: Neelanjan Manna <[email protected]>

kustomize images update

700b80e

Signed-off-by: Neelanjan Manna <[email protected]>

resolved merge conflict on mockgen:

9a12c33

Signed-off-by: Neelanjan Manna <[email protected]>

added function comments

449ab7e

Signed-off-by: Neelanjan Manna <[email protected]>

merged main

c9a451e

Signed-off-by: Neelanjan Manna <[email protected]>

added loki scaler unit tests

b78e97f

Signed-off-by: Neelanjan Manna <[email protected]>

updated changelog with issue

dc3fba7

Signed-off-by: Neelanjan Manna <[email protected]>

updated test cases with loki default port in server address

4e6455d

Signed-off-by: Neelanjan Manna <[email protected]>

added bearer auth

2b6194e

Signed-off-by: Neelanjan Manna <[email protected]>

neelanjan00 requested a review from a team as a code owner September 27, 2022 17:45

JorTurFer reviewed Sep 30, 2022

View reviewed changes

neelanjan00 added 4 commits October 2, 2022 22:01

reverted auto-generated code, updated changelog, updated multi-tenant…

956ca56

… tenant name field Signed-off-by: Neelanjan Manna <[email protected]>

removed metricName parameter

640f64e

Signed-off-by: Neelanjan Manna <[email protected]>

merged main

ae93432

Signed-off-by: Neelanjan Manna <[email protected]>

updated unit tests to remove metricName tests

a2e3d2d

Signed-off-by: Neelanjan Manna <[email protected]>

neelanjan00 added 3 commits October 9, 2022 18:15

added e2e test for Loki scaler, undo kustomization.yaml changes

27dcfc4

Signed-off-by: Neelanjan Manna <[email protected]>

updated trigger type

a532fc5

Signed-off-by: Neelanjan Manna <[email protected]>

reverted kustomization manifests

33fd3e5

Signed-off-by: Neelanjan Manna <[email protected]>

JorTurFer reviewed Oct 10, 2022

View reviewed changes

tests/scalers/loki/loki_test.go Outdated Show resolved Hide resolved

JorTurFer reviewed Oct 10, 2022

View reviewed changes

pkg/generated/clientset/versioned/scheme/register.go Outdated Show resolved Hide resolved

JorTurFer reviewed Oct 10, 2022

View reviewed changes

pkg/generated/clientset/versioned/fake/register.go Outdated Show resolved Hide resolved

grouped imports, replaced test-app deployment container with nginx, a…

67f1ab5

…dded create namespace function, minor fixes Signed-off-by: Neelanjan Manna <[email protected]>

updated changelog

6c51ae9

Signed-off-by: Neelanjan Manna <[email protected]>

JorTurFer reviewed Oct 11, 2022

View reviewed changes

tests/scalers/loki/loki_test.go Outdated Show resolved Hide resolved

updated logql query for excluding promtail pod logs, which also has t…

8a1da77

…he substring being searched in the query Signed-off-by: Neelanjan Manna <[email protected]>

neelanjan00 requested a review from JorTurFer October 12, 2022 15:51

neelanjan00 added 2 commits October 12, 2022 21:23

merged main branch

f3f442d

Signed-off-by: Neelanjan Manna <[email protected]>

resolved go.sum for godotenv package

1d0fb91

Signed-off-by: Neelanjan Manna <[email protected]>

JorTurFer reviewed Oct 12, 2022

View reviewed changes

pkg/scalers/loki_scaler.go Outdated Show resolved Hide resolved

tests/scalers/loki/loki_test.go Outdated Show resolved Hide resolved

tests/scalers/loki/loki_test.go Outdated Show resolved Hide resolved

tests/scalers/loki/loki_test.go Outdated Show resolved Hide resolved

JorTurFer reviewed Oct 12, 2022

View reviewed changes

pkg/scalers/loki_scaler.go Outdated Show resolved Hide resolved

JorTurFer reviewed Oct 12, 2022

View reviewed changes

pkg/scalers/loki_scaler.go Outdated Show resolved Hide resolved

neelanjan00 added 2 commits October 12, 2022 22:37

removed unused variables and templates from E2E test, removed name no…

9b5b46f

…rmalization Signed-off-by: Neelanjan Manna <[email protected]>

merged main branch

392860b

Signed-off-by: Neelanjan Manna <[email protected]>

neelanjan00 requested a review from JorTurFer October 12, 2022 17:10

JorTurFer requested review from v-shenoy and zroubalik October 12, 2022 20:49

JorTurFer approved these changes Oct 12, 2022

View reviewed changes

v-shenoy approved these changes Oct 12, 2022

View reviewed changes

JorTurFer merged commit 6e38c5e into kedacore:main Oct 20, 2022

JorTurFer mentioned this pull request Oct 21, 2022

Add Grafana Labs as user kedacore/keda-docs#961

Merged

26tanishabanik pushed a commit to 26tanishabanik/keda that referenced this pull request Nov 1, 2022

feat(Scaler): Adds Loki Scaler (kedacore#3700)

e5b1ce0

Signed-off-by: 26tanishabanik <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Scaler): Adds Loki Scaler #3700

feat(Scaler): Adds Loki Scaler #3700

neelanjan00 commented Sep 27, 2022 •

edited by JorTurFer

Loading

JorTurFer left a comment

neelanjan00 commented Oct 5, 2022

JorTurFer commented Oct 5, 2022

JorTurFer commented Oct 5, 2022

v-shenoy commented Oct 8, 2022

neelanjan00 commented Oct 10, 2022 •

edited

Loading

neelanjan00 commented Oct 10, 2022 •

edited

Loading

neelanjan00 commented Oct 10, 2022

JorTurFer commented Oct 10, 2022 •

edited by github-actions bot

Loading

v-shenoy commented Oct 11, 2022

neelanjan00 commented Oct 11, 2022

JorTurFer commented Oct 11, 2022

JorTurFer commented Oct 12, 2022 •

edited by github-actions bot

Loading

JorTurFer left a comment

JorTurFer commented Oct 12, 2022 •

edited by github-actions bot

Loading

JorTurFer left a comment •

edited

Loading

v-shenoy left a comment

neelanjan00 commented Oct 13, 2022

feat(Scaler): Adds Loki Scaler #3700

feat(Scaler): Adds Loki Scaler #3700

Conversation

neelanjan00 commented Sep 27, 2022 • edited by JorTurFer Loading

Checklist

JorTurFer left a comment

Choose a reason for hiding this comment

neelanjan00 commented Oct 5, 2022

JorTurFer commented Oct 5, 2022

JorTurFer commented Oct 5, 2022

v-shenoy commented Oct 8, 2022

neelanjan00 commented Oct 10, 2022 • edited Loading

neelanjan00 commented Oct 10, 2022 • edited Loading

neelanjan00 commented Oct 10, 2022

JorTurFer commented Oct 10, 2022 • edited by github-actions bot Loading

v-shenoy commented Oct 11, 2022

neelanjan00 commented Oct 11, 2022

JorTurFer commented Oct 11, 2022

JorTurFer commented Oct 12, 2022 • edited by github-actions bot Loading

JorTurFer left a comment

Choose a reason for hiding this comment

JorTurFer commented Oct 12, 2022 • edited by github-actions bot Loading

JorTurFer left a comment • edited Loading

Choose a reason for hiding this comment

v-shenoy left a comment

Choose a reason for hiding this comment

neelanjan00 commented Oct 13, 2022

neelanjan00 commented Sep 27, 2022 •

edited by JorTurFer

Loading

neelanjan00 commented Oct 10, 2022 •

edited

Loading

neelanjan00 commented Oct 10, 2022 •

edited

Loading

JorTurFer commented Oct 10, 2022 •

edited by github-actions bot

Loading

JorTurFer commented Oct 12, 2022 •

edited by github-actions bot

Loading

JorTurFer commented Oct 12, 2022 •

edited by github-actions bot

Loading

JorTurFer left a comment •

edited

Loading