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

(DO NOT MERGE) Added non-root user to Dockerfile. #6525

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

samuelklee
Copy link
Contributor

@samuelklee samuelklee commented Mar 25, 2020

Closes #4782
Closes #5959

We should discuss a few issues, and modify/cleanup if necessary, before this goes in (if it does at all). In particular, I'd like to understand the original intent behind using the /root directory (e.g., in all of our WDLs) and make sure we don't break typical downstream uses by instead using /gatk; we could even consider "deprecating" this over a few releases. I think it's also worth discussing whether we want to continue to release a rootful image, but perhaps parameterize the Docker build script to easily allow the building of an image with a non-root user.

I must admit that I don't have good visibility on the various use cases of our Dockers (outside of our typical use with Terra/Cromwell), so if any users would like to chime in, that would certainly be appreciated. In particular, users may still have to do some work on their end to remap user namespaces.

In any case, this is at least a proof-of-principle that mounting resources is possible within our test framework with the option for a non-root user. So unless we have other good reasons for requiring a root user, it seems worthwhile to at least allow this option, even if we don't make that the new default. (EDIT: Just to clarify, at some point I was told by another developer that the need to mount testing resources within Travis was at least one reason why we needed a root user---turns out this isn't the case.)

@samuelklee samuelklee force-pushed the sl_docker_add_user branch 4 times, most recently from 0d70c0d to e1644aa Compare March 25, 2020 20:25
@gatk-bot

This comment has been minimized.

@droazen
Copy link
Collaborator

droazen commented Jun 17, 2020

@lbergelson How do you want to proceed on this one?

@wichadak
Copy link

wichadak commented Nov 1, 2021

I still got the problem using cromwell + singularity and a component complained with the following error:
GATK_LOCAL_JAR was set to: /root/gatk.jar but this file doesn't exist. Please fix your environment.
The pipeline I used is

cromwell-executions/GATKSVPipelineSingleSample/87d715e3-7dd9-4079-9109-adf536d99400/call-GatherSampleEvidence/GatherSampleEvidence/181ae713-8db5-4a5c-b709-ad5b73a544b8/call-CollectCounts/attempt-2/execution/

@samuelklee
Copy link
Contributor Author

@droazen @lbergelson any plans for this branch?

@samuelklee
Copy link
Contributor Author

@wichadak when you say “a component complained,” does that mean the other steps of your pipeline succeeded under Cromwell + Singularity? I would be surprised if the issue under discussion here only affected that particular task and not all of them.

@samuelklee
Copy link
Contributor Author

samuelklee commented Jan 5, 2022

@droazen @lbergelson if we're going to ultimately punt on this, just let me know and I'd be happy to close this. EDIT: Actually, let me just go ahead and close it, and if someone else wants to pick this up in the future, they can reopen.

@samuelklee samuelklee closed this Jan 5, 2022
@samuelklee samuelklee reopened this Jan 6, 2022
@samuelklee
Copy link
Contributor Author

Actually, Jessica Hekman just confirmed she ran into an issue when running the GATK SV WDLs on-prem with Singularity, namely being unable to access the GATK jar in the root directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants