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

Introduce --disable-entrypoint-overwrite option to allow to disable entrypoint overwrite #1138

Closed

Conversation

creydr
Copy link
Contributor

@creydr creydr commented Sep 11, 2023

Currently the entrypoint of the image is always set to /ko-app/<app-name>. This makes it hard to have some kind of wrapper around the app (e.g. in case you want to remote debug the app via Delve, which invokes the app e.g. via dlv exec <path-to-app>).

This PR addresses it and introduces a new option (--disable-entrypoint-overwrite), which allows to use the entrypoint of the base image. To be able to get the path to the apps binary, this PR also introduce the KO_APP_PATH var, which points to the apps binary.
That way a developer who wants to remote debug their app (without to adjust yamls with an updated command & arg) could simply use a base image with a pre setup entrypoint, like the following:

ENTRYPOINT "/usr/bin/dlv exec --listen=:40000 --headless=true --log=true --accept-multiclient --api-version=2 $APP_PATH"

and build/apply via:

ko apply apply --disable-entrypoint-overwrite --disable-optimizations -f <path-to-yaml>

@imjasonh
Copy link
Member

Thanks for this contribution!

I've never used Delve myself, but I'd definitely love to have better support for it, so having someone with that use case is definitely useful.

My main question is what should happen if the base image doesn't have an entrypoint when --disable-entrypoint-override is called. The default base image, for example, doesn't have an entrypoint set.

Would it make sense to have an --entrypoint flag that explicitly sets the image's entrypoint to the given path, and puts the binary there instead of /ko-app/foo? That seems like it would handle both --disable-entrypoint-override and the KO_APP_PATH env var, if I'm understanding correctly.

If it's possible I'd also be interested in more first-class support for dlv or other debugging tooling, something along the lines of a ko build --debug that adds the dlv wrapper directly and transparently. Is that something that could be useful?

@imjasonh
Copy link
Member

...And in fact that seems to be what #1128 starts to enable! 🎉

If that seems promising we can push on that PR and see if that helps with your use case as well.

@creydr
Copy link
Contributor Author

creydr commented Sep 11, 2023

Hello @imjasonh,
thanks for your response. In the following my thoughts to your comment:

My main question is what should happen if the base image doesn't have an entrypoint when --disable-entrypoint-override is called. The default base image, for example, doesn't have an entrypoint set.

IMO this flag should only be set intentionally. If the user uses a base image without an entrypoint and disables the override, we could log a warning (theoretically the app can still be called directly via comand in the yaml).

Would it make sense to have an --entrypoint flag that explicitly sets the image's entrypoint to the given path, and puts the binary there instead of /ko-app/foo? That seems like it would handle both --disable-entrypoint-override and the KO_APP_PATH env var, if I'm understanding correctly.

I though of this at the beginning too, but then "discarded" the idea to keep the parameter simple and not always have to pass the full entrypoint. Having this in a preconfigured base image seemed for me simpler. But for sure I could update it.

If it's possible I'd also be interested in more first-class support for dlv or other debugging tooling, something along the lines of a ko build --debug that adds the dlv wrapper directly and transparently. Is that something that could be useful?

This is a great idea. My only concerns are, that we would need to provide some kind of base image then (IIUC) and I am not sure, if the then provided Delve configuration parameters would fit for most users (e.g. port, should the app start or wait on startup (--continue flag)).

...And in fact that seems to be what #1128 starts to enable! 🎉

IIUC this does not allow to pass parameters for the binary too and then would require the user to specify them via args in the yaml (but I might misunderstood it).

So whatever helps most I would be happy to help and contribute, as I am really looking for some easy way to debug on ko built apps.

@creydr
Copy link
Contributor Author

creydr commented Oct 3, 2023

Putting this on hold, as #1148 might be a better approach
/hold

Copy link

github-actions bot commented Jan 2, 2024

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

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.

2 participants