Skip to content

deploy(docker): Add prestart endpoint to docker-entrypoint#900

Merged
tonyo merged 2 commits into
masterfrom
deploy/prestart-endpoint-dockerfile
Jan 8, 2021
Merged

deploy(docker): Add prestart endpoint to docker-entrypoint#900
tonyo merged 2 commits into
masterfrom
deploy/prestart-endpoint-dockerfile

Conversation

@tonyo
Copy link
Copy Markdown
Contributor

@tonyo tonyo commented Jan 5, 2021

Did this to see how ugly it looks in the end.
This is a bit smarter hack to allow "service dependencies" when using image in environment like Kubernetes.
E.g. for PoP relays we want to make sure that the Relay container is started after Envoy sidecar is up and running, otherwise first auth requests to the upstream will just fail. They are retried, yes, but it doesn't make sense to even try them before Envoy is up.

By default, this doesn't change the entrypoint behavior. If RELAY_PRESTART_ENDPOINT is defined (e.g. RELAY_PRESTART_ENDPOINT=http://127.0.0.1:12345), the entrypoint will try to reach it before proceeding further.

#skip-changelog

@tonyo tonyo requested review from a team and beezz January 5, 2021 18:20
@jan-auer
Copy link
Copy Markdown
Member

jan-auer commented Jan 6, 2021

Q: Is it usual to do something like this? It seems quite excessive for something that I would expect to be handled outside of the container by orchestration.

@tonyo
Copy link
Copy Markdown
Contributor Author

tonyo commented Jan 7, 2021

Kubernetes unfortunately doesn't currently support the concept of sidecars properly, so we're on our own here. There are some hacks (e.g. this one) but they are, well, hacks.

The solution is not the prettiest, but supporting this in the entrypoint will also allow image users to reuse these patterns for self-hosted setups, if necessary.

Copy link
Copy Markdown
Member

@untitaker untitaker left a comment

Choose a reason for hiding this comment

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

If you're in the middle of trying things out and need this deployed, let's merge this for now

I would argue this should make RELAY_DELAY_STARTUP_SECONDS obsolete, and ideally we would bake this into the main healthcheck (i.e. relay checks upstream healthcheck as condition of own healthcheck) instead of writing shell wrappers

@tonyo tonyo merged commit 62bc3d2 into master Jan 8, 2021
@tonyo tonyo deleted the deploy/prestart-endpoint-dockerfile branch January 8, 2021 12:29
jan-auer added a commit that referenced this pull request Jan 13, 2021
* master:
  deploy(docker): Add prestart endpoint to docker-entrypoint (#900)
  ci(release): Enable the CalVer flag for release (#906)
  ci(release): it is Release
  ci(release): Fix release yaml indents
  ci(release): Upgrade action-prepare-release to latest version (#905)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants