Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Prometheus metrics handling for the operator. #555

Merged
merged 2 commits into from
Jan 18, 2024
Merged

Fix Prometheus metrics handling for the operator. #555

merged 2 commits into from
Jan 18, 2024

Conversation

mplzik
Copy link
Contributor

@mplzik mplzik commented Oct 30, 2023 •
edited
Loading

The current state of the Helm chart is slightly confusing, because:

  • There's no easy way to really disable prometheus metrics --
    --enable-prometheus-metrics defaults to true anthe current code
    either emits --enable-prometheus-metrics=true or nothing at all
    (making it true once again).
  • The http container port is actually a metrics port (by convention
    from .e.g. webhook), but is present regardless of whether Prometheus
    metrics are enabled or not. To make it less confusing, this PR proposes renaming it.

Signed-off-by: Milan Plzik [email protected]

Provide a description of what has been changed

Checklist

  • I have verified that my change is according to the deprecations & breaking changes policy
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • README is updated with new configuration values (if applicable) learn more
  • A PR is opened to update KEDA core (repo) (if applicable, ie. when deployment manifests are modified)

Fixes #

@mplzik mplzik requested a review from a team as a code owner October 30, 2023 09:31
@mplzik mplzik force-pushed the mplzik/operator-add-metrics-port branch from 64fd392 to 8c2e2c8 Compare October 30, 2023 09:33
@mplzik
Fix Prometheus metrics handling for the operator.
c3f22bc 
The current state of the Helm chart is slightly confusing, because:
- There's no easy way to really disable prometheus metrics --
  `--enable-prometheus-metrics` defaults to true anthe current code
  either emits `--enable-prometheus-metrics=true` or nothing at all
  (making it `true` once again).
- The `http` container port is actually a `metrics` port (by convention
  from .e.g. webhook), but is present regardless of whether Prometheus
  metrics are enabled or not. To make it less confusing, this PR
  proposes renaming it.

Signed-off-by: Milan Plzik <[email protected]>
@mplzik mplzik force-pushed the mplzik/operator-add-metrics-port branch from 8c2e2c8 to 0ab691b Compare October 30, 2023 12:50
@mplzik mplzik changed the title Add metrics port to the operator deployment. Fix Prometheus metrics handling for the operator. Oct 30, 2023
@mplzik mplzik force-pushed the mplzik/operator-add-metrics-port branch from 0ab691b to c3f22bc Compare October 30, 2023 12:51
@zroubalik zroubalik mentioned this pull request Jan 18, 2024
Closed
25 tasks
@JorTurFer JorTurFer merged commit 4d9518b into kedacore:main Jan 18, 2024
37 checks passed
JorTurFer added a commit to guicholeo/keda that referenced this pull request Jan 18, 2024
@mplzik @JorTurFer
Fix Prometheus metrics handling for the operator. (kedacore#555)
8c080b2 
The current state of the Helm chart is slightly confusing, because:
- There's no easy way to really disable prometheus metrics --
  `--enable-prometheus-metrics` defaults to true anthe current code
  either emits `--enable-prometheus-metrics=true` or nothing at all
  (making it `true` once again).
- The `http` container port is actually a `metrics` port (by convention
  from .e.g. webhook), but is present regardless of whether Prometheus
  metrics are enabled or not. To make it less confusing, this PR
  proposes renaming it.

Signed-off-by: Milan Plzik <[email protected]>
Signed-off-by: Jorge Turrado Ferrero <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
JorTurFer added a commit that referenced this pull request Jan 30, 2024
@zerodayyy @JorTurFer
@loispostula @congzhegao @Adarsh-verma-14 @yuvalweber @tomkerkhove @radekfojtik @QuentinBisson @SpiritZhou @zroubalik @eumel8 @aballman @Bhargav-InfraCloud @ferndem @mplzik @choisungwook @awalford16 @guicholeo @wozniakjan
feat(keda): Allow using own Cert-manager Issuer/ClusterIssuer for iss…
4cf42b3 
…uing KEDA TLS certificates (#530)

* feat(keda): ✨ Allow providing own cert-manager issuer in TLS certificate

Signed-off-by: Dmytro Kovalenko <[email protected]>

* docs(keda): 📝 Generate Helm docs

Signed-off-by: Dmytro Kovalenko <[email protected]>

* fix(keda): 🐛 Inject CA from cert-manager Certificate when providing own Issuer

Signed-off-by: Dmytro Kovalenko <[email protected]>

* refactor(keda): ♻️ Refactor values format

Signed-off-by: Dmytro Kovalenko <[email protected]>

* revert(keda): ⏪ Revert unnecessary auto-formatting

Signed-off-by: Dmytro Kovalenko <[email protected]>

* chore: Improve the CI on PRs to be more efficient (#540)

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* fix(http-add-on): Refactor the chart for next version (#523)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* feat(add-on): Supporting streamInterval configuration (#541)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* chore(add-on): Ship Release 0.6.0 (#543)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* chore: update versions in README.md (#546)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* feat: update crd to allow vault secret to handle write operation (#548)

Signed-off-by: Loïs Postula <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* Fix the svc name of webhook to avoid breaking istio (#551)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* Show only logs with a severity level of ERROR or higher in the stderr (#506)

Signed-off-by: Adarsh-verma-14 <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* Support profiling for keda components (#549)

Signed-off-by: yuval weber <[email protected]>
Signed-off-by: unknown <[email protected]>
Co-authored-by: Tom Kerkhove <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* Fix TriggerAuthentication - added configuration for validation webhook (#553)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* fix: Declare missing port in KEDA operator (#552)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* Allow image registry override for all keda components (#557)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* docs: Clarify that contributors do not have to ship Helm chart (#573)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* add  disable-compression arg for both operator and metrics-server (#554)

Signed-off-by: Adarsh-verma-14 <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* feat: Introduce CloudEventSources CRD and adding ClusterName parameter (#572)

* Add CloudEventSources Crd and ClustetName Parameter

Signed-off-by: SpiritZhou <[email protected]>

* Update

Signed-off-by: SpiritZhou <[email protected]>

* Update

Signed-off-by: SpiritZhou <[email protected]>

* Update keda/values.yaml

Co-authored-by: Tom Kerkhove <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>

* Fix

Signed-off-by: SpiritZhou <[email protected]>

* Update

Signed-off-by: SpiritZhou <[email protected]>

* Revert unnecessary update

Signed-off-by: SpiritZhou <[email protected]>

---------

Signed-off-by: SpiritZhou <[email protected]>
Co-authored-by: Tom Kerkhove <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* store 2.12.1 package at `main` (#577)

Signed-off-by: Zbynek Roubalik <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* fix: restore http-add-on chart 0.6.0 indexing (#579)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* fix(add-on): Use 'main' tag for KEDA installation during CI (#582)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* set securityContext for http-add-on chart (#561)

Co-authored-by: Tom Kerkhove <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* Fix http-add-on operator resources (#567)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* Fix http-add-on verbosity configuration (#568)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* chore: Adjust RBAC with code (#585)

* chore: Adjust RBAC with code

Signed-off-by: Jorge Turrado <[email protected]>

* fix typo

Signed-off-by: Jorge Turrado <[email protected]>

---------

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* fix: Don't recreate CA with 8 months until it expires (#586)

Signed-off-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* feat(ClusterRole): Add RBAC rule to allow access to `LimitRange` (#588)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* remove not required insecureSkipTLSVerify (#564)

Signed-off-by: Frank Kloeker <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* Update templates/webhooks deployment (#590)

Align deployment for extraVolumes and extraVolumesMount for fix problem Error: YAML parse error on keda/templates/webhooks/deployment.yaml: error converting YAML to JSON: yaml: line 96: did not find expected key

Signed-off-by: ferndem <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* Fix Prometheus metrics handling for the operator. (#555)

The current state of the Helm chart is slightly confusing, because:
- There's no easy way to really disable prometheus metrics --
  `--enable-prometheus-metrics` defaults to true anthe current code
  either emits `--enable-prometheus-metrics=true` or nothing at all
  (making it `true` once again).
- The `http` container port is actually a `metrics` port (by convention
  from .e.g. webhook), but is present regardless of whether Prometheus
  metrics are enabled or not. To make it less confusing, this PR
  proposes renaming it.

Signed-off-by: Milan Plzik <[email protected]>
Signed-off-by: Jorge Turrado Ferrero <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* Fix Remove app.kubernetes.io/instance label in crd (#556)

Signed-off-by: choisungwook <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* Support crd-specific annotations (#584)

* support crd-specific annotations

Signed-off-by: Adam Walford <[email protected]>

* update readme

Signed-off-by: Adam Walford <[email protected]>

* update docs using helm-docs

Signed-off-by: Adam Walford <[email protected]>

---------

Signed-off-by: Adam Walford <[email protected]>
Co-authored-by: Adam Walford <[email protected]>
Co-authored-by: Tom Kerkhove <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* Add ciliumnetworkpolicies (#558)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* Add tlsConfig for ServiceMonitor (#591)

Co-authored-by: guicholeo <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* Release 2.13.0 (#593)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* fix: Ship v2.13.1 with missing RoleBinding (#595)

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* chore(add-on): Apply HTTP Add-on changes on Helm chart (#598)

Signed-off-by: Dmytro Kovalenko <[email protected]>

* chore(add-on): Release v0.7.0 (#599)

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Dmytro Kovalenko <[email protected]>

* refactor: Unify cert-manager annotations

Signed-off-by: Dmytro Kovalenko <[email protected]>

---------

Signed-off-by: Dmytro Kovalenko <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Loïs Postula <[email protected]>
Signed-off-by: Adarsh-verma-14 <[email protected]>
Signed-off-by: yuval weber <[email protected]>
Signed-off-by: unknown <[email protected]>
Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: Zbynek Roubalik <[email protected]>
Signed-off-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Frank Kloeker <[email protected]>
Signed-off-by: ferndem <[email protected]>
Signed-off-by: Milan Plzik <[email protected]>
Signed-off-by: choisungwook <[email protected]>
Signed-off-by: Adam Walford <[email protected]>
Co-authored-by: Dmytro Kovalenko <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Co-authored-by: Loïs Postula <[email protected]>
Co-authored-by: Roy Gao <[email protected]>
Co-authored-by: Adarsh Verma <[email protected]>
Co-authored-by: yuval weber <[email protected]>
Co-authored-by: Tom Kerkhove <[email protected]>
Co-authored-by: Radek Fojtik <[email protected]>
Co-authored-by: Quentin Bisson <[email protected]>
Co-authored-by: SpiritZhou <[email protected]>
Co-authored-by: Zbynek Roubalik <[email protected]>
Co-authored-by: Frank Kloeker <[email protected]>
Co-authored-by: Andrew <[email protected]>
Co-authored-by: Bhargav Ravuri <[email protected]>
Co-authored-by: ferndem <[email protected]>
Co-authored-by: Milan Plžík <[email protected]>
Co-authored-by: choisungwook <[email protected]>
Co-authored-by: Adam Walford <[email protected]>
Co-authored-by: Adam Walford <[email protected]>
Co-authored-by: guicholeo <[email protected]>
Co-authored-by: Jan Wozniak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JorTurFer JorTurFer JorTurFer approved these changes

@zroubalik zroubalik zroubalik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mplzik @zroubalik @JorTurFer