[kong] remove default controller binary arg and add debug no-probe setting for controller #481
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove the hard-coded /kong-ingress-controller arg from the controller container arguments. This argument is no longer correct as of 2.0, where the binary is instead /manager. It is apparently ignored. Removing it does not interfere with normal start or configured arguments This argument appears unnecessary for older versions of the controller. We've had /kong-ingress-controller or /manager as our entrypoint since we created the Dockerfile, so it's always invoked anyway. Removing this arg allows for alternative images that do not use the contorller binary and whose binary /is/ confused by the extra argument (namely Delve-based images).
Add a .ingressController.disableProbes value. When set to any value, the controller container will not have readiness or liveness probes. This setting is intentionally omitted from values.yaml and documentation. It is for use only with debuggers and should not be used under normal circumstances.
rainest
changed the title
Co-authored-by: Shane Utt <shaneutt@linux.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this PR does / why we need it:
Two changes to allow running the controller via Delve without local chart modifications:
/kong-ingress-controllerarg from the controller. It's currently wrong and should never have been necessary due to our images' entrypoints..ingressController.disableProbesvalue to disable readiness and liveness probes entirely. Debugger pauses break the probes and force a restart, and they can't be configured away because Helm will always add in the default HTTP probe unless you set your own HTTP probe (and no HTTP probes work, since they're all paused). While enabling this does allow you to use a debugger, it also causes all hell to break loose because the controller becomes ready immediately, even if it fails to start and exits. It usually fails to start because Kong takes a bit to come online. Don't know of any way around that other than manually killing PID 1 to restart the container (we have no admin API retry config), so it's quite janky and really should not be exposed to users.Special notes for your reviewer:
AFAIK we no longer test anything with CLI args since the chart uses envvars for everything. They do work as expected, however (testing on 2.0):
I'm fairly sure no older versions would actually require this arg given the entrypoint, but worst case you can add it back via user-configured args.
Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]
nextbranch and targetsnext, notmain[kong])