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

[Idea] Some tips for efficient Dockerfiles. #1

Open
fmerinocasallo opened this issue Aug 26, 2024 · 2 comments
Open

[Idea] Some tips for efficient Dockerfiles. #1

fmerinocasallo opened this issue Aug 26, 2024 · 2 comments

Comments

@fmerinocasallo
Copy link

I wonder if you could achieve any efficiency gains by applying some of the tips mentioned here 🤔

For example, in this Dockerfile from your slicer-env, you may take advantage of the --chmod feature of the Dockerfile instruction COPY (see the Docker official Dockerfile reference for more details). The following code snippet:

COPY /desktop/slicer.desktop /config/Desktop/
RUN chmod 777 /config/Desktop/slicer.desktop

from lines 54 and 55 would translate into:

COPY --chmod=777 /desktop/slicer.desktop /config/Desktop/slicer.desktop

which should reduce the number of layers and, therefore, the size of the Docker image 💡 In this particular case, it will probably not result in huge gains because of the presumably reduced size of the slicer.desktop file. However, it might be something to consider in other cases.

The same should probably work in lines 74 and 75:

COPY /desktop/jupyter.desktop /config/Desktop/
RUN chmod 777 /config/Desktop/jupyter.desktop

which would translate into:

COPY --chmod=777 /desktop/jupyter.desktop /config/Desktop/jupyter.desktop

PS: Is it really necessary to assign 777 permissions to those file? 🤔

@gabnasello
Copy link
Owner

Well spotted! Yeah I will implement it in the next version of the image. I'm not 100% sure we need to assign 777 to those files, I saw that the "abc" user wouldn't access them otherwise and I was worried there could have been some issues once we change user ID and group ID. That was the easiest solution, and I wasn't too concerned about changing permissions of just those two files... What do you think?

@fmerinocasallo
Copy link
Author

Who is this abc user you have mentioned in your last comment? 🤔

I am curious about the user(s) involved in this image. Who are they?

I see several directories and files whose permissions are being changed to 777 in this Dockerfile:

  1. RUN chmod 777 -R /slicer (line 51)
  2. RUN chmod 777 /config/Desktop/slicer.desktop (line 55)
  3. RUN chmod 777 /config/Desktop/jupyter.desktop (line 74)
  4. RUN chmod 777 -R /config/ (line 76)
  5. RUN chmod 777 -R /tmp/Slicer-/ (line 80)

The user executing the content of this Dockerfile is root, isn't it?

I wonder:

  1. I have downloaded the content from SLICER_URL. It seems the content of the .tar.gz folder includes a Slicer executable file (with 755 permissions). Therefore, /tmp/Slicer* should only include /tmp/Slicer. Is it really necessary this RUN chmod 777 -R /slicer? Would it be enough something like RUN chmod 777 /slicer? Which user(s) would need to write and execute this file? Based on the response to this last question, it may be possible to use more restricted permissions (e.g., 755).
  2. I assume you need the +x permission to allow for execution of this shortcut. However, what is this /config/Desktop folder? Which user has access to this folder? Who is the owner?
  3. Same as (2).
  4. What is located in /config/ in addition to /config/Desktop/slicer.desktop and /config/Desktop/jupyter.desktop (which already had 777 permissions when execution reaches line 76)? If those two files are the only ones in /config/, could we remove this line 76 altogether?
  5. Couldn't this be simply removed? You had previously moved (line 49) /tmp/Slicer* to /slicer. What is still located in /tmp/Slicer-/ when execution reaches line 80?

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

No branches or pull requests

4 participants
@fmerinocasallo @gabnasello and others