feat(helm): Adding KEDA autoscaling support #7282

beatkind · 2024-02-03T15:40:33Z

What this PR does

This PR takes #4687 as base layer and tries to implement all added code comments. Please be aware that this PR does not cover the changes in the monitoring dashboards that may be needed.

Which issue(s) this PR fixes or relates to

Fixes #3430

Checklist

Tests updated.
Documentation added.
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
about-versioning.md updated with experimental features.

CLAassistant · 2024-02-03T15:40:39Z

All committers have signed the CLA.

beatkind · 2024-02-03T16:13:12Z

@dimitarvdimitrov I have opened this PR and took #4687 as a base. I have ported the changes from your PR #6971 into here, as you have mentioned in this comment.

I know that there is still documentation missing for this PR. Could you, nevertheless, have a look over the changes?

dimitarvdimitrov · 2024-02-06T18:09:23Z

many thanks for picking this up. I will set some time aside this week to review it

dimitarvdimitrov · 2024-02-07T21:44:36Z

Have you had the chance to test this out on a mimir cell? It would be nice to do that before we merge the PR

beatkind · 2024-02-07T21:52:06Z

I did tried it in a minikube setup, but not yet in a real world scenario.

Do you feel confident in merging it as experimental feature? This would give me the option to test it in our production cluster.

dimitarvdimitrov

First of all, great work @beatkind! Thank you for picking this up.

Review

The PR is in a really good shape. I left a few minor comments and I think they'll be quick to address. Can you take a look? I'm happy to merge this as an experimental feature after addressing them.

I apologise for the delayed review, but I couldn't find a large-enough block of time to review this (it's a hefty PR). I'll try to be more responsive for the next iterations.

Docs

Perhaps the major reason for writing docs is to cover the migration procedure for enabling autoscaling. At Grafana Labs we had to go through a migration procedure to make sure that there aren't any disruptions. The main problem was that removing the replicas field causes the deployment to scale to 1 replica if the HPA hasn't had the time to take control, which is very likely if you're removing replicas and creating the HPA in one go.

I think a reasonable migration procedure would be to add a config flag to kedaAutoscaling which preserves replicas (e.g. *.kedaAutoscaling.keepReplicas). If you'd like to add that in this PR you're welcome, but this PR is already quite large and is already a beneficial addition. I opened this issue to track the migration docs and configuration #7367

Do you agree with this? Maybe I'm missing something about the migration.

Future work

There some bits which are missing from this PR which I think we should add before calling this work complete. I've added them in another issue #7368. As the one opening this PR, would you like to take ownership of these follow-up actions? It's ok to say no, just decided to offer you the chance.

also cc @krajorama in case you want to review this too

operations/helm/charts/mimir-distributed/values.yaml

operations/helm/charts/mimir-distributed/templates/query-frontend/query-frontend-so.yaml

operations/helm/charts/mimir-distributed/templates/querier/querier-so.yaml

operations/helm/charts/mimir-distributed/templates/metamonitoring/_helpers.tpl

operations/helm/charts/mimir-distributed/values.yaml

operations/helm/charts/mimir-distributed/CHANGELOG.md

beatkind · 2024-02-12T20:03:54Z

@dimitarvdimitrov many thanks for the review! I am happy to add missing parts that you have mentioned.

I also totally agree with the migration path. You are right, this is already a big PR. I will add the things you have mentioned, let then merge it as experimental. I will also add a comment regarding to be careful regarding the mentioned replica situation.

I am happy to take ownership of the issues you have created and I plan to open another PR once that is merged introducing the migration to autoscaler and adding docs :)

Thanks you very much!

…perimental state of the feature, using the same autoscaling rules & behaviours as jsonnet, only supporting newest version of KEDA

dimitarvdimitrov

just one comment on the querier SO and a suggestion to satisfy the linter

operations/helm/charts/mimir-distributed/templates/query-frontend/query-frontend-so.yaml

operations/helm/charts/mimir-distributed/CHANGELOG.md

dimitarvdimitrov

LGTM, thank you for your work on this @beatkind 😄

dimitarvdimitrov · 2024-02-13T12:44:53Z

because of some drift with main, there are some changes which aren't reflected in the rendered manifests for keda-autoscaling. Can you rebase your branch on top of grafana:main or merge with it, rerun make build-helm-tests locally and push?

beatkind · 2024-02-13T12:45:25Z

@dimitarvdimitrov already on my way

…afana#7269) * helm: align grpc server connection lifetime settings with jsonnet Co-authored-by: Marco Pracucci <marco@pracucci.com> Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com> * helm: rebuild tests Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com> --------- Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com> Co-authored-by: Marco Pracucci <marco@pracucci.com>

…rafana#7290) * querymiddleware: race condition in shardActiveSeriesMiddleware s2.Writer is not goroutine-safe to reuse between concurrent requests. Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com> * querymiddleware: remove flaky ResponseBodyStreamed test from shardActiveSeriesMiddleware --------- Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com>

Update pkg/util/version/info.go Update pkg/mimirtool/client/client.go Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com> Co-authored-by: Andy Asp <90626759+andyasp@users.noreply.github.com>

…onfig (grafana#7298) Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com>

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Jsonnet / Helm: improve distributors graceful shutdown Signed-off-by: Marco Pracucci <marco@pracucci.com> * Fix Helm distributor termination grace period Signed-off-by: Marco Pracucci <marco@pracucci.com> --------- Signed-off-by: Marco Pracucci <marco@pracucci.com>

Co-authored-by: grafanabot <grafanabot@grafana.com>

…fault to true, and deprecate (grafana#7366) * Distributor: Change -distributor.enable-otlp-metadata-storage flag's default to true and deprecate --------- Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

…-distributor.limit-inflight-requests-using-grpc-method-limiter as stable and enable it by default (grafana#7360) * Mark -ingester.limit-inflight-requests-using-grpc-method-limiter and -distributor.limit-inflight-requests-using-grpc-method-limiter as stable and enable it by default Signed-off-by: Marco Pracucci <marco@pracucci.com> * Improved tests Signed-off-by: Marco Pracucci <marco@pracucci.com> * Improved tests Signed-off-by: Marco Pracucci <marco@pracucci.com> * Fixed unit tests Signed-off-by: Marco Pracucci <marco@pracucci.com> * Mark config as deprecated Signed-off-by: Marco Pracucci <marco@pracucci.com> --------- Signed-off-by: Marco Pracucci <marco@pracucci.com>

…rafana#7342) * Do not consider out-of-order blocks when filtering compactable jobs Co-authored-by: Marco Pracucci <marco@pracucci.com>

* mimir: inject span profiler into tracer Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com> * Update changelog Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com> --------- Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com>

* Add experimental partitions ring lifecycler support Signed-off-by: Marco Pracucci <marco@pracucci.com> Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> Co-authored-by: Peter Štibraný <pstibrany@gmail.com> * Use http.Error() Signed-off-by: Marco Pracucci <marco@pracucci.com> * Skip deprecated linter check for code that will be soon replaced Signed-off-by: Marco Pracucci <marco@pracucci.com> * Updated dskit Signed-off-by: Marco Pracucci <marco@pracucci.com> * Add partition id to logs Signed-off-by: Marco Pracucci <marco@pracucci.com> * Added comment Signed-off-by: Marco Pracucci <marco@pracucci.com> * Added TestIngester_ShouldNotCreatePartitionIfThereIsShutdownMarker Signed-off-by: Marco Pracucci <marco@pracucci.com> --------- Signed-off-by: Marco Pracucci <marco@pracucci.com> Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> Co-authored-by: Peter Štibraný <pstibrany@gmail.com>

dimitarvdimitrov · 2024-02-13T13:09:04Z

I believe this still needs one more make build-helm-tests

feat(helm): Adding KEDA autoscaling support

9ff0e92

beatkind requested a review from a team as a code owner February 3, 2024 15:40

beatkind added 2 commits February 3, 2024 16:41

fix: update changelog

f7d7afd

feat(helm): Porting the changes from grafana#6971 into helm chart

ce04869

beatkind force-pushed the add-helm-keda branch from b9fd5d2 to ce04869 Compare February 3, 2024 16:10

ChrisNoSim approved these changes Feb 5, 2024

View reviewed changes

dimitarvdimitrov self-requested a review February 12, 2024 17:36

This was referenced Feb 12, 2024

helm: autoscaling migration procedure #7367

Open

helm: promote autoscaling to stable #7368

Open

dimitarvdimitrov reviewed Feb 12, 2024

View reviewed changes

feat: Adding better changelog & values documentation outlining the ex…

9ad9f67

…perimental state of the feature, using the same autoscaling rules & behaviours as jsonnet, only supporting newest version of KEDA

beatkind requested review from jhesketh and dimitarvdimitrov February 13, 2024 11:21

dimitarvdimitrov reviewed Feb 13, 2024

View reviewed changes

operations/helm/charts/mimir-distributed/templates/query-frontend/query-frontend-so.yaml Outdated Show resolved Hide resolved

operations/helm/charts/mimir-distributed/CHANGELOG.md Outdated Show resolved Hide resolved

fix: Remove duplicate query field, add base url in CHANGELOG.md

c0f29e7

beatkind requested a review from dimitarvdimitrov February 13, 2024 12:37

dimitarvdimitrov approved these changes Feb 13, 2024

View reviewed changes

dimitarvdimitrov enabled auto-merge (squash) February 13, 2024 12:37

narqo and others added 4 commits February 13, 2024 13:47

version: add UserAgent() (grafana#7264)

21bbd22

Update pkg/util/version/info.go Update pkg/mimirtool/client/client.go Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com> Co-authored-by: Andy Asp <90626759+andyasp@users.noreply.github.com>

helm: remove -server.grpc.keepalive.max-connection-idle from common c…

916c340

…onfig (grafana#7298) Signed-off-by: Vladimir Varankin <vladimir.varankin@grafana.com>

renovate bot and others added 15 commits February 13, 2024 13:47

Update golang.org/x/exp digest to 2c58cdc (grafana#7352)

aa3813c

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Update module github.com/aws/aws-sdk-go to v1.50.15 (grafana#7353)

f7c3cb7

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Update module github.com/minio/minio-go/v7 to v7.0.67 (grafana#7354)

831f9e2

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Update dependency puppeteer to v21.11.0 (grafana#7355)

f720020

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Update helm/kind-action action to v1.9.0 (grafana#7357)

c5e9dfe

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Update module cloud.google.com/go/storage to v1.37.0 (grafana#7358)

271a805

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Release mimir-distributed Helm chart 5.3.0-weekly.277 (grafana#7362)

66a893a

Co-authored-by: grafanabot <grafanabot@grafana.com>

Distributor: Make -distributor.enable-otlp-metadata-storage flag de…

8ba0cad

…fault to true, and deprecate (grafana#7366) * Distributor: Change -distributor.enable-otlp-metadata-storage flag's default to true and deprecate --------- Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Do not consider out-of-order blocks when filtering compactable jobs (g…

f9e9d6f

…rafana#7342) * Do not consider out-of-order blocks when filtering compactable jobs Co-authored-by: Marco Pracucci <marco@pracucci.com>

feat(helm): Adding KEDA autoscaling support

188d181

chore: rebase branch with main

87add14

auto-merge was automatically disabled February 13, 2024 12:51
Head branch was pushed to by a user without write access

beatkind requested review from grafanabot, wilfriedroset, vaxvms, bubu11e and a team as code owners February 13, 2024 12:51

Merge branch 'grafana:main' into add-helm-keda

cf2e13e

chore: make build-helm-tests

c96d4c6

dimitarvdimitrov enabled auto-merge (squash) February 13, 2024 13:15

dimitarvdimitrov merged commit 55950ca into grafana:main Feb 13, 2024
30 checks passed

beatkind deleted the add-helm-keda branch February 13, 2024 13:37

This was referenced Feb 13, 2024

feat(helm): add keda autoscaling and fix dashboards #4687

Closed

Add support for HPA in mimir-distributed #3430

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(helm): Adding KEDA autoscaling support #7282

feat(helm): Adding KEDA autoscaling support #7282

beatkind commented Feb 3, 2024 •

edited by dimitarvdimitrov

Loading

CLAassistant commented Feb 3, 2024 •

edited

Loading

beatkind commented Feb 3, 2024

dimitarvdimitrov commented Feb 6, 2024

dimitarvdimitrov commented Feb 7, 2024

beatkind commented Feb 7, 2024

dimitarvdimitrov left a comment

beatkind commented Feb 12, 2024

dimitarvdimitrov left a comment

dimitarvdimitrov left a comment

dimitarvdimitrov commented Feb 13, 2024

beatkind commented Feb 13, 2024

dimitarvdimitrov commented Feb 13, 2024

feat(helm): Adding KEDA autoscaling support #7282

feat(helm): Adding KEDA autoscaling support #7282

Conversation

beatkind commented Feb 3, 2024 • edited by dimitarvdimitrov Loading

What this PR does

Which issue(s) this PR fixes or relates to

Checklist

CLAassistant commented Feb 3, 2024 • edited Loading

beatkind commented Feb 3, 2024

dimitarvdimitrov commented Feb 6, 2024

dimitarvdimitrov commented Feb 7, 2024

beatkind commented Feb 7, 2024

dimitarvdimitrov left a comment

Choose a reason for hiding this comment

Review

Docs

Future work

beatkind commented Feb 12, 2024

dimitarvdimitrov left a comment

Choose a reason for hiding this comment

dimitarvdimitrov left a comment

Choose a reason for hiding this comment

dimitarvdimitrov commented Feb 13, 2024

beatkind commented Feb 13, 2024

dimitarvdimitrov commented Feb 13, 2024

beatkind commented Feb 3, 2024 •

edited by dimitarvdimitrov

Loading

CLAassistant commented Feb 3, 2024 •

edited

Loading