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

[kong] use args instead of command #377

Merged
merged 1 commit into from
Jun 25, 2021
Merged

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Jun 22, 2021

What this PR does / why we need it:

Uses args instead of command to invoke non-standard commands for Kong containers. This allows the standard image entrypoint to run rather than replacing it.

Inject the no daemonize environment variable directly into the wait-for-db command. There's currently a limitation in the entrypoint that overrides the container environment. Kong/docker-kong#474 intends to fix it but we'll need this for backwards compatibility.

See the commit message for a bit more detail on why we need this.

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • PR is based off the current tip of the next branch and targets next, not main
  • New or modified sections of values.yaml are documented in the README.md
  • Title of the PR and commit headers start with chart name (e.g. [kong])

Use args instead of command for containers that do not run the standard
image command (wait-for-db and migrations).

Standard Kong images built with https://github.com/Kong/docker-kong use
an entrypoint script. This script performs some initial bootstrap setup
and then executes whatever command was provided to Docker.

Kubernetes' handling of the ENTRYPOINT/CMD relationship differs from
Docker's. Docker always runs the image entrypoint, passing the command
as arguments to it. Kubernetes "command" in container templates instead
overrides the entrypoint entirely:
https://kubernetes.io/docs/tasks/inject-data-application/define-command-argument-container/#notes

We do want the entrypoint to run normally, because in some images it
injects bundled custom plugins into KONG_PLUGINS.

Additionally update the wait-for-db command to include an override
KONG_NGINX_DAEMON value inline. The stock entrypoint hardcodes this off,
which we don't want, and must override for wait-for-db.
@rainest rainest requested a review from a team as a code owner June 22, 2021 20:15
@Tieske
Copy link
Member

Tieske commented Jun 24, 2021

fyi: PR Kong/docker-kong#474 has been merged

@Tieske
Copy link
Member

Tieske commented Jun 24, 2021

@rainest will this work backward compatible? The fix in Kong/docker-kong#474 shows that old images will ALWAYS set the daemonize option to off. So even if you pass the env var in, the entrypoint will override it again and set it to off again.

Not sure how to fix this....

@rainest
Copy link
Contributor Author

rainest commented Jun 24, 2021

https://github.com/Kong/charts/pull/377/files#diff-3e8a71242505abb72903a338eef1cdea2cd2c674f877c59378eff807951a1fa8R513

The export there will run after the one in the entrypoint script. CI would fail otherwise (since the initcontainer would never complete).

Copy link
Member

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Once we're happy with this in testing, good to go 👍

@rainest rainest merged commit cf43ed8 into next Jun 25, 2021
@rainest rainest deleted the feat/preserve-entrypoint branch June 25, 2021 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants