feat(manifests): add name on workflow-controller-metrics service port #2744
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While adding a name is optional from a Kubernetes standpoint since there
is only one port listed, the named port makes using Prometheus easier.
There's a targetPort option for Prometheus' ServiceMonitor Custom
Resource, but this targetPort doesn't relate to the service's
targetPort, but in fact the containers' targetPort pointed at by the
service [1]. And since the workflow-controller deployment doesn't
actually explicitly list the ports the targetPort configuration on a
ServiceMonitor does nothing. So having the named port on the service's
port makes it much easier to use for Prometheus.
1 - prometheus-operator/prometheus-operator#2515
I'm not actually sure how to use this service with Prometheus-operator without this named port. I considered this commit a feature, but might actually be a fix. Please let me know if it's preferred for me to use fix instead of feat for this.
Checklist:
Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore."fix(controller): Updates such and such. Fixes #1234"
.I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.My organization is added to USERS.md.