search

sbt / sbt-native-packager

sbt Native Packager
https://sbt-native-packager.readthedocs.io/en/stable/
BSD 2-Clause "Simplified" License
1.6k stars 441 forks source link

java.nio.file.AccessDeniedException: /opt/docker/RUNNING_PID #1458

Closed christian-schlichtherle closed 3 years ago

christian-schlichtherle commented 3 years ago

I'm using the DockerPlugin. In my build.sbt, I have the following settings:

dockerBaseImage := "openjdk:11",
dockerExposedPorts := Seq(9000),
dockerUpdateLatest := true,

Docker / daemonUserUid := None,
Docker / daemonUser := "daemon",
Docker / maintainer := "???",

This generates the following Dockerfile:

FROM openjdk:11 as stage0
LABEL snp-multi-stage="intermediate"
LABEL snp-multi-stage-id="5c0621ba-1ab5-4977-bf0f-e3c2f2cee7e4"
WORKDIR /opt/docker
COPY opt /opt
USER root
RUN ["chmod", "-R", "u=rX,g=rX", "/opt/docker"]
RUN ["chmod", "u+x,g+x", "/opt/docker/bin/server"]

FROM openjdk:11
LABEL MAINTAINER="???"
WORKDIR /opt/docker
COPY --from=stage0 --chown=daemon:root /opt/docker /opt/docker
EXPOSE 9000
USER daemon
ENTRYPOINT ["/opt/docker/bin/server"]
CMD []

When running this image, I get this:

$ docker run -it --rm -p "9000:9000" server
Oops, cannot start the server.
java.nio.file.AccessDeniedException: /opt/docker/RUNNING_PID
[...]

When taking a closer look at what's going on, it turns out the order of the commands in the Dockerfile is wrong:

[...]
WORKDIR /opt/docker
COPY --from=stage0 --chown=daemon:root /opt/docker /opt/docker
[...]

This sequence causes the directory /opt/docker to be created and owned by root:root. The subsequent copy command does not change that because the directory already exists. Swapping the commands fixes this issue.

However... the sequence of these commands is not optimal anyway. They should be rearranged as follows:

FROM openjdk:11 as stage0
LABEL snp-multi-stage="intermediate"
LABEL snp-multi-stage-id="5c0621ba-1ab5-4977-bf0f-e3c2f2cee7e4"
USER root
WORKDIR /opt/docker
COPY opt /opt
RUN ["chmod", "-R", "u=rX,g=rX", "/opt/docker"]
RUN ["chmod", "u+x,g+x", "/opt/docker/bin/server"]

FROM openjdk:11
LABEL MAINTAINER="info@schlichtherle.de"
USER daemon
WORKDIR /opt/docker
EXPOSE 9000
ENTRYPOINT ["/opt/docker/bin/server"]
CMD []
COPY --from=stage0 --chown=daemon:root /opt/docker /opt/docker

The point is that the USER command should be running as early as possible to make sure everything after is using this user. And the COPY command should be running as late as possible to take advantage of the docker build cache. Mind you that every command creates a layer that somebody has to download when running docker pull. All these EXPOSE etc commands effectively don't contain data, but they still add overhead to the image layers.

christian-schlichtherle commented 3 years ago

Thanks, but this is an issue, not a feature request, please. The current image is unusable because of permission problems.

muuki88 commented 3 years ago

Hi @christian-schlichtherle

Thanks for the detailed issue request. I just labelled it and hadn't had the time for a response ( currently on vacation ).

Your description contains two things

  1. Permission issues
  2. Layer optimization

Permission issues

There were a few discussions on this before. The result was

This comes up regularly with play ( were you are encouraged to disable the PID file creation as it serves no purpose ) or logging.

Why do you need a PID file in your docker image? From my view docker images are stateless and as such don't need to be writable.

Layer optimization

Moving the copy to the end may safe a few KB and network calls. Is it worth it? Some users fiddle around with the docker file, changing the ordering may break their build setup.

christian-schlichtherle commented 3 years ago

As for why Play creates this PID file, you need to discuss this with the original authors. I'm just a user who wanted to use your plugin and found it to be unusable because of this issue.

As for the layer optimization: Yes, it does matter. It matters on Internet scale with millions of users. It matters for large corporations where clusters may have hundreds of nodes, even in the presence of container registry mirrors.

Why would a user want to mess with the generated Dockerfile if not for shortcomings like the issue I found? So it's totally in your control if you want to add more value to your plugin or not.

As for me, I decided to remove the plugin from my build because I have some more advanced use cases, e.g. cross platform builds.

muuki88 commented 3 years ago

Do you are using the Playframework? Then sbt-native-packager is already enabled and you don't need to add the plugin yourself.

Regarding the PID file. See https://www.playframework.com/documentation/2.8.x/ProductionConfiguration 

$ /path/to/bin/<project-name> -Dpidfile.path=/var/run/play.pid

And there's a section for play here too: https://sbt-native-packager.readthedocs.io/en/latest/recipes/play.html#build-configuration

Use

Universal / javaOptions += "-Dpid.file=/dev/null"
muuki88 commented 3 years ago

Why would a user want to mess with the generated Dockerfile if not for shortcomings like the issue I found?

To name a few

So it's totally in your control if you want to add more value to your plugin or not.

I maintain this plugin in my free time. Changes that may cause a lot of users to ask for help for a performance optimization that probably very few will profit from ( I'm sure Google, Twitter, Facebook, Amazon, etc. don't use native-packager ) are not worth the effort.

As for me, I decided to remove the plugin from my build because I have some more advanced use cases, e.g. cross platform builds.

Can you elaborate? Still docker or other formats?

christian-schlichtherle commented 3 years ago

I need to use docker buildx ... for some cross platform building options and so I tweaked the Dockerfile to my liking. I'm also using pidfile.path=... now to overwrite the default location.