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

Consider removing ENTRYPOINT from non-distroless uv Docker images #7030

Closed
zanieb opened this issue Sep 4, 2024 · 6 comments · Fixed by #7054
Closed

Consider removing ENTRYPOINT from non-distroless uv Docker images #7030

zanieb opened this issue Sep 4, 2024 · 6 comments · Fixed by #7054
Labels
needs-decision Undecided if this should be done

Comments

@zanieb
Copy link
Member

zanieb commented Sep 4, 2024

Using uv as an entrypoint is surprising (it differs from the base image) and broke the example at astral-sh/uv-docker-example#13 — I had to add:

ENTRYPOINT []

cc @samypr100

@zanieb zanieb added the needs-decision Undecided if this should be done label Sep 4, 2024
@samypr100
Copy link
Contributor

samypr100 commented Sep 4, 2024

Agreed, we can change it (just deleting that line) in the CI yaml.

Since its our own images I thought we'd aim for a consistent entrypoint experience across all of them rather than retain the upstream entrypoint which would differ depending on the tag. You see that consistency often on other images derived from base images (e.g. redis, etc.).

@zanieb
Copy link
Member Author

zanieb commented Sep 4, 2024

Ah interesting. Hm. I'm not sure — it's probably not going to be common to use uv as the entrypoint since it's not a "service"? It makes sense in the distroless image because there's nothing else to invoke.

@samypr100
Copy link
Contributor

samypr100 commented Sep 4, 2024

Ah interesting. Hm. I'm not sure — it's probably not going to be common to use uv as the entrypoint since it's not a "service"? It makes sense in the distroless image because there's nothing else to invoke.

It also applies CLI tools afaik. I do get the point though, normally by convention I expect tag variations offered by a specific project to default their entrypoint to their service/binary. If we want a different experience, we can do that as well easily.

There's two core use cases here

  • Experience when pulling the image via docker run (no args) - first time experience
  • Building using FROM

In the first case you immediately get uv rather than something else, and it delivers a nice new user experience (no surprises from parent image).

In the second case, its quite common to use multi-stage to solve if this becomes an issue.
On single stage you also get the benefit of leveraging CMD alongside the default ENTRYPOINT. For example CMD run xyz would end up becoming uv run xyz which I think also delivers a nice user experience focused on uv, rather than the upstream image's focus (as that could change over time).

@samypr100
Copy link
Contributor

samypr100 commented Sep 4, 2024

@zanieb For example, in the example from https://github.com/astral-sh/uv-docker-example

# Reset the entrypoint, don't invoke `uv`
ENTRYPOINT []

# Run the FastAPI application by default
# Uses `fastapi dev` to enable hot-reloading when the `watch` sync occurs
# Uses `--host 0.0.0.0` to allow access from outside the container
CMD ["fastapi", "dev", "--host", "0.0.0.0", "src/uv_docker_example"]

would become something like

# Run the FastAPI application by default
# Uses `fastapi dev` to enable hot-reloading when the `watch` sync occurs
# Uses `--host 0.0.0.0` to allow access from outside the container
CMD ["run", "fastapi", "dev", "--host", "0.0.0.0", "src/uv_docker_example"]

which showcases uv's capabilites further as you can see, but I might be biased towards uv everything 😆

@zanieb
Copy link
Member Author

zanieb commented Sep 4, 2024

People are weirdly against having uv run in their CMD but yeah I mean I think that makes sense too.

@samypr100
Copy link
Contributor

People are weirdly against having uv run in their CMD but yeah I mean I think that makes sense too.

I think the change in #7054 would satisfy most workflows, and addresses the original concerns and still gives us consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Undecided if this should be done
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants