-
Notifications
You must be signed in to change notification settings - Fork 141
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
Allow exposed metrics port to be overridden #1518
Conversation
Rekor uses the same metrics port, so if both services are run with docker-compose they conflict with each other. Add an environment variable to expose the metrics service on a different docker port. Signed-off-by: Colleen Murphy <colleenmurphy@google.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1518 +/- ##
==========================================
- Coverage 57.74% 57.67% -0.07%
==========================================
Files 50 50
Lines 3112 3112
==========================================
- Hits 1797 1795 -2
- Misses 1156 1158 +2
Partials 159 159 ☔ View full report in Codecov by Sentry. |
@@ -34,7 +34,7 @@ services: | |||
ports: | |||
- "5555:5555" | |||
- "5554:5554" | |||
- "2112:2112" | |||
- "${FULCIO_METRICS_PORT:-2112}:2112" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would do
- "${FULCIO_METRICS_PORT:-2112}:2112" | |
- "${FULCIO_METRICS_PORT:-2113}:2112" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. You can also use set the metrics-port
flag to set a different metrics port for the service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cpanato I didn't want to change the default since it could be considered a breaking change. Do you think having a better default is worth that breaking change?
@haydentherapper I wanted to run this in docker-compose without editing docker-compose.yml
, so it doesn't really matter to me what port metrics is running on in the container, only what external port is exposed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't make the breaking change here, given that we already have a command line flag to change it.
Rekor uses the same metrics port, so if both services are run with docker-compose they conflict with each other. Add an environment variable to expose the metrics service on a different docker port.
Needed for an update to the cosign e2e tests.
Relates to sigstore/sigstore-probers#105
Summary
Release Note
Enables docker-compose to use an alternate port for metrics with
FULCIO_METRICS_PORT
.Documentation
Added in docs/setup.md.