Sidekiq: Enforce separate ports for metrics and health checks
What does this MR do?
We recently merged !2479 (merged), which sets an explicit health check port for Sidekiq. This is good and expected since falling back to the metrics server port is deprecated behavior and removed in 15.0.
We now also enforce this behavior by explicitly running a config check to ensure these ports are distinct.
I also removed the enabled
toggle for health checks; this is a remnant from using the same set of settings keys as the exporter, but it does not make sense to disable health checks since Sidekiq would never come up. In fact, even if someone disabled this prior, we would still point the kubelet to /readiness
, all we would do is not start the server, which doesn't make a whole lot of sense.
Note: we do allow ports to match if metrics are disabled since then only one server will start.
Related issues
Test plan
Set server ports to the same value as the metrics server default port:
helm upgrade -i gitlab gitlab/gitlab \
--set gitlab.sidekiq.health_checks.port=3807 \
--set certmanager-issuer.email=me@example.com
Error: UPGRADE FAILED: template: gitlab/templates/NOTES.txt:126:3: executing "gitlab/templates/NOTES.txt" at <include "gitlab.checkConfig" .>: error calling include: template: gitlab/templates/_checkConfig.tpl:101:54: executing "gitlab.checkConfig" at <fail>: error calling fail:
CONFIGURATION CHECKS:
sidekiq:
metrics.port and health_checks.port must not be equal.
See https://docs.gitlab.com/charts/charts/gitlab/sidekiq/index.html#configuration
However, it should work when metrics are disabled:
helm upgrade -i gitlab gitlab/gitlab \
--set gitlab.sidekiq.metrics.enabled=false \
--set gitlab.sidekiq.health_checks.port=3807 \
--set certmanager-issuer.email=me@example.com
Checklist
See Definition of done.
For anything in this list which will not be completed, please provide a reason in the MR discussion.
Required
-
Merge Request Title and Description are up to date, accurate, and descriptive -
MR targeting the appropriate branch -
MR has a green pipeline on GitLab.com
Expected (please provide an explanation if not completing)
-
Test plan indicating conditions for success has been posted and passes -
Documentation created/updated -
Tests added -
Integration tests added to GitLab QA -
Equivalent MR/issue for omnibus-gitlab opened: gitlab-org/omnibus-gitlab!6065 (merged)