Updating for v2 and adding prometheus options for scraping by tbickford · Pull Request #60 · kedacore/charts

tbickford · 2020-08-09T21:45:28Z

This PR updates the chart for V2. I also removed the CRD's from the templates directory and created a template file to reference the crds directory - this should allow for the use of both Helm 2 and Helm 3.

I add the prometheus.enabled option that allows for configuring the metrics port and path along with an option of also creating a PodMonitor for the Prometheus Operator.

I am wondering if there be a v2 branch that this PR should instead go against?

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

tomkerkhove

LGTM - Do we need keda/kubectl?

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

tomkerkhove · 2020-09-21T05:36:57Z

Can we rebase this since it has been changes since then please?

Might have some conflicting changes with #69 so we'll need to 👀 that.

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

tbickford · 2020-09-22T01:39:54Z

The Helm 3 lint check had me change the CRD apiVersion (see this https://github.com/kedacore/charts/runs/1147109297?check_suite_focus=true) from apiextensions.k8s.io/v1beta1 to apiextensions.k8s.io/v1 but it looks like that is now causing an issue when I try to install it locally:

Error: failed to install CRD crds/scaledjobs.keda.sh.yaml: CustomResourceDefinition.apiextensions.k8s.io "scaledjobs.keda.sh" is invalid: spec.versions[0].schema.openAPIV3Schema: Required value: schemas are required

It looks like there are some changes that would be needed to adapt to the updated api - https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/

Can we update KEDA to use this new schema?

These linting errors are only appearing in the most recent version of Helm 3 (3.3.3). I did not encounter the problems locally using version 3.2.3

tbickford · 2020-09-22T02:22:35Z

The other linting problem that appears to be introduced in Helm 3.3 is:

[ERROR] templates/21-metrics-clusterrolebinding.yaml: object name does not conform to Kubernetes naming requirements: "keda-operator:system:auth-delegator": invalid metadata name, must match regex ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$ and the length must not longer than 253

tomkerkhove · 2020-09-22T05:17:53Z

The Helm 3 lint check had me change the CRD apiVersion (see this kedacore/charts/runs/1147109297?check_suite_focus=true) from apiextensions.k8s.io/v1beta1 to apiextensions.k8s.io/v1 but it looks like that is now causing an issue when I try to install it locally:
Error: failed to install CRD crds/scaledjobs.keda.sh.yaml: CustomResourceDefinition.apiextensions.k8s.io "scaledjobs.keda.sh" is invalid: spec.versions[0].schema.openAPIV3Schema: Required value: schemas are required
It looks like there are some changes that would be needed to adapt to the updated api - kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning

Can we update KEDA to use this new schema?

These linting errors are only appearing in the most recent version of Helm 3 (3.3.3). I did not encounter the problems locally using version 3.2.3

Woops, missed this comment before reviewing. We decided to ignore it for now since we dont' want to force users to use Kubernetes 1.16 - Sorry should've told you.

If using Helm 3.2.3 doesn't show this we should maybe change our CI to use that?

zroubalik · 2020-09-22T08:10:59Z

Yeah, we discussed it here: kedacore/keda#701 (comment)

We can regenerate the CRDs for apiextensions.k8s.io/v1 and we should document that KEDA is supported only on k8s 1.16+

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

tbickford · 2020-09-30T12:48:47Z

@zroubalik I merged in the changes you did for the CRD API changes today.

zroubalik · 2020-09-30T13:27:50Z

@tbickford awesome, thanks. Just let us know, when this PR is ready :)

tbickford · 2020-09-30T13:32:51Z

I am still trying to dig into:

templates/21-metrics-clusterrolebinding.yaml: object name does not conform to Kubernetes naming requirements: "keda-operator:system:auth-delegator": invalid metadata name, must match regex ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$ and the length must not longer than 253

If we use a prior minor version of helm3 the lint command should work.

zroubalik · 2020-09-30T13:42:55Z

I am still trying to dig into:

templates/21-metrics-clusterrolebinding.yaml: object name does not conform to Kubernetes naming requirements: "keda-operator:system:auth-delegator": invalid metadata name, must match regex ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$ and the length must not longer than 253

If we use a prior minor version of helm3 the lint command should work.

If that's the biggest problem, I think that we could rename that cluster role binding to conform the requirements. But we should rename that in the keda repo first, so the resources are in sync

tbickford · 2020-09-30T15:48:16Z

@zroubalik FYI - helm/helm#8835

zroubalik · 2020-10-01T13:06:09Z

@zroubalik I merged in the changes you did for the CRD API changes today.

@tbickford just be sure that you are mergin in the patched version of ScaledJob CRD, ie the output of make release or make deploy.

The ScaledObject CRD is being patched due to the problems with the required fields in the generated resources. See kedacore/keda#1208 and kedacore/keda#927

tomkerkhove · 2020-10-02T06:31:48Z

@tbickford I will merge this with the current error so we can include it in the RC; would you mind creating an issue for your follow-up please?

@zroubalik I'll take a look at the CRD.

* All our tweaks should go here If there is another thing to add to our fork, pls add it here. Example: ``` echo -e "#foo?\n\n bar?" > foo-bar.md # Pls keep adding the changes to standalone files that are not present in the upstream to mitigate the risk of conflict add foo-bar.md git commit -s --ammend yada yada ``` Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> * ensure keda ns in smoketests before chart installation (kedacore#36) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * smoke_tests: wait for deployments availability (kedacore#37) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * fix keda chart deployment smoketest check missing ns (kedacore#38) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * Don't skip the publish step (kedacore#39) Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> * http-add-on: allow fetching certs from secrets (kedacore#40) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: disable explicit tls secret mount Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * restricted scc (kedacore#41) Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com> * ci: refactor kedify chart release trigger (kedacore#42) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * bump kube-rbax-proxy to v0.16.0 (kedacore#43) Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com> * ci: fix missing quote and pipe Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: attempt HTTP2 upgrade by default (kedacore#45) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: allow configuring prometheus and otel collectors (kedacore#46) * http-add-on: allow configuring prometheus and otel collectors Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * helm-docs Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> --------- Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * gh release chart: fix env var evaluation (kedacore#47) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: metrics service (kedacore#48) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: disable interceptor pdb (kedacore#49) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: RBAC to emit events (kedacore#50) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * First shot of the agent helm chart (kedacore#51) * All our tweaks should go here If there is another thing to add to our fork, pls add it here. Example: ``` echo -e "#foo?\n\n bar?" > foo-bar.md # Pls keep adding the changes to standalone files that are not present in the upstream to mitigate the risk of conflict add foo-bar.md git commit -s --ammend yada yada ``` Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> * ensure keda ns in smoketests before chart installation (kedacore#36) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * smoke_tests: wait for deployments availability (kedacore#37) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * fix keda chart deployment smoketest check missing ns (kedacore#38) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * Don't skip the publish step (kedacore#39) Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> * http-add-on: allow fetching certs from secrets (kedacore#40) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: disable explicit tls secret mount Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * restricted scc (kedacore#41) Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com> * ci: refactor kedify chart release trigger (kedacore#42) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * bump kube-rbax-proxy to v0.16.0 (kedacore#43) Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com> * ci: fix missing quote and pipe Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: attempt HTTP2 upgrade by default (kedacore#45) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: allow configuring prometheus and otel collectors (kedacore#46) * http-add-on: allow configuring prometheus and otel collectors Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * helm-docs Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> --------- Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * gh release chart: fix env var evaluation (kedacore#47) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: metrics service (kedacore#48) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: disable interceptor pdb (kedacore#49) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: RBAC to emit events (kedacore#50) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * First shot of the agent helm chart Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> * more granular rbac Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> * more granular rbac vol 2 Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> * Install also CRD and CR if requested, this is done using pre install hook (for crd) and post install hook for CR Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> * fixes for agent chart (kedacore#52) * fixes for agent chart Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * image tag proposal Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * add cluster name + option to disable kedifyconfig Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * simplify kedify cr template Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * narrow down autowiring rbac Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> --------- Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> --------- Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com> Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com> Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com> Co-authored-by: Jan Wozniak <jan@kedify.io> * add helm repos (templating failed because of dependent charts) (kedacore#53) Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> * Don't wait for agent to be ready (for now) (kedacore#54) Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> * Turn off linting for stefanprodan/helm-gh-pages action + add dependen… (kedacore#55) Turn off linting for stefanprodan/helm-gh-pages action + add dependencies + icon Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> * agent: configurable autowire and metrics RBAC (kedacore#56) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * agent: bump image to v0.1.1 (kedacore#57) Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * http-add-on: bump version to v0.8.1-0 (kedacore#58) * kedify-agent: disable istio sidecar injection for CR and CRD creating jobs (kedacore#60) kedifi-agent: disable istio sidecar injection for CR and CRD creating jobs Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * Add install mechanism for PRPs Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> --------- Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com> Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com> Signed-off-by: Jirka Kremser <535866+jkremser@users.noreply.github.com> Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com> Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com> Co-authored-by: Jan Wozniak <jan@kedify.io>

Updating for v2 and adding prometheus options for scraping

be68265

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

tbickford requested review from ahmelsayed, jeffhollan, tomkerkhove and zroubalik as code owners August 9, 2020 21:45

tomkerkhove approved these changes Aug 10, 2020

View reviewed changes

Comment thread keda/values.yaml

Comment thread keda/values.yaml Outdated

Changing promtheus enabled to default to false

3799424

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

tbickford marked this pull request as draft August 10, 2020 14:38

tbickford mentioned this pull request Sep 18, 2020

Helm 3 install failed #18

Closed

sstk-tbickford added 4 commits September 21, 2020 21:12

Resolving conflicts

35072fa

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

Removing old crd files

660825b

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

Updating api version for CRDs

8f3ac36

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

Removing crd files from templates dir

db39ba3

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

tomkerkhove reviewed Sep 22, 2020

View reviewed changes

Comment thread keda/crds/scaledjobs.keda.sh.yaml

sstk-tbickford added 2 commits September 30, 2020 08:43

Updating CRD's with keda api changes

8cb98bc

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

updating cluster role

f66125f

Signed-off-by: Travis Bickford <tbickford@shutterstock.com>

tbickford marked this pull request as ready for review September 30, 2020 13:32

tomkerkhove merged commit 5ff6f79 into kedacore:master Oct 2, 2020

tomkerkhove mentioned this pull request Oct 2, 2020

Use patched version of ScaledJob CRD #74

Merged

tomkerkhove mentioned this pull request Oct 19, 2020

Use apiextensions.k8s.io/v1 instead of apiextensions.k8s.io/v1beta1 #33

Closed

Conversation

tbickford commented Aug 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomkerkhove left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tomkerkhove commented Sep 21, 2020

Uh oh!

tbickford commented Sep 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tbickford commented Sep 22, 2020

Uh oh!

Uh oh!

tomkerkhove commented Sep 22, 2020

Uh oh!

zroubalik commented Sep 22, 2020

Uh oh!

tbickford commented Sep 30, 2020

Uh oh!

zroubalik commented Sep 30, 2020

Uh oh!

tbickford commented Sep 30, 2020

Uh oh!

zroubalik commented Sep 30, 2020

Uh oh!

tbickford commented Sep 30, 2020

Uh oh!

zroubalik commented Oct 1, 2020

Uh oh!

tomkerkhove commented Oct 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tbickford commented Aug 9, 2020 •

edited

Loading

tbickford commented Sep 22, 2020 •

edited

Loading