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.

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

[kube-prometheus-stack] Implement Gateway API #4646

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

jkroepke
Copy link
Member

What this PR does / why we need it

The current template is just an approach. Once finalized, we add it anywhere in the chart.

Ref: #4622

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

  • fixes #

Special notes for your reviewer

Checklist

  • DCO signed
  • Chart Version bumped
  • Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
@@ -3251,6 +3251,25 @@ prometheus:
# hosts:
# - prometheus.example.com

gateway:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have other examples of this kind of flag upstream? Because gateway is used for a lot of other things right now liké Mimir gateways so maybe other upstream projects use better names liké gatewayapi? It could also be they use gateway😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I found nothing via GitHub Code search. Bitnami and Grafana did not have Gateway API yet, but I'm open for other names here.

But gatewayApi sounds more specific.

Copy link
Contributor

@rouke-broersma rouke-broersma Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would propose to skip the gateway level and use the httpRoute level only, this makes sense with gateway api terminology where the gateway resource is managed centrally by the cluster operator and the application developer only deals with routes. Alternatively if the extra level is needed because you envision that tlsRoute etc might also at some point be supported I would propose the following structure:

routes:
  http:
  tls:
  grpc:

...

But for a start I would suggest to keep it simple and start with:

httpRoute:
...

And if more route types are needed we can always introduce a breaking change at that point.

Copy link
Member Author

@jkroepke jkroepke Aug 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still prefix the values with gatewayAPI - who knows what comes?

Currently, only httpRoute is implemented and there are no plans to provide more.

Let me know, if that current design is good to go

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <github@jkroepke.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants