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

feat: explicitly specify containerPort in helm chart #1306

Closed

Conversation

hsudbrock
Copy link
Contributor

Fixes #1305

Description

Currently, the deployment from the connaisseur helm chart does not explicitly specify the containerPort. This is usually not a problem, because the port can still be used, even if it is not explicitly named.

Explicitly specifying the port has two benefits:

  • It documents the available ports for a user of the chart.
  • It enables to configure automatic scraping of metrics by prometheus using a PodMonitor from the prometheus operator (which requires the port to be explicitly specified). This helps in working around the issue described in Prometheus cannot connect to /metrics HTTPS endpoint #709, to scrape metrics even when they are provided via https only.

In principle, one could also name the port, but since there is only one exposed port and I did not want to use a generic name like thePort, I did not add a port name.

Checklist

  • [ x] PR is rebased to/aimed at branch develop
  • [ x] PR follows Contributing Guide
  • [ x] Added tests (if necessary)
  • [ x] Extended README/Documentation (if necessary)
  • [ x] Adjusted versions of image and Helm chart in Chart.yaml (if necessary)

Specifying the containerPort, on the one hand, has a documentary purpose, by making explicit which ports are exposed by the container. On the other hand, it permits the usage of other tools that rely on the containerPort, like PodMonitors from the prometheus operator.

Fix sse-secure-systems#1305
@phbelitz
Copy link
Member

merged in #1308, thank you @hsudbrock

@phbelitz phbelitz closed this Oct 13, 2023
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.

Helm chart: Explicitly describe the exposed port
2 participants