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

Improve docker-compose installation method: #274

Closed
wants to merge 1 commit into from

Conversation

umer936
Copy link

@umer936 umer936 commented May 9, 2024

  1. proper variable usage in docker-compose.yml
  2. put provision.sh inline of the Dockerfile
  3. make container not rely on root user -- fixes npm permissions issue
  4. define all versions at top of Dockerfile
  5. Bail early if /app not mounted

Feel free to modify as desired, but this means the container does not rely on npm being installed on the host system and it fixed the issues I was facing regarding npm failing permissions due to being run as root.

1) proper variable usage in docker-compose.yml
2) put provision.sh inline of the Dockerfile
3) make container not rely on root user -- fixes npm permissions issue
4) define all versions at top of Dockerfile
@umer936 umer936 marked this pull request as ready for review May 10, 2024 12:30
@umer936
Copy link
Author

umer936 commented May 10, 2024

hmm may need a note in the install instructions that the dpo-voyager folder needs to be writable by all (chmod a+w .) bc npm install makes node_modules, dist, and package-lock.json

alternatively could have the folders (e.g. files, node_modules, and dist) included in the repo with just a .gitkeep file in the folders

if you're ok with the first, I'll make the PR on the docs as well

@gjcope
Copy link
Collaborator

gjcope commented May 14, 2024

Thanks for the PR @umer936, we will take a look.

@gjcope
Copy link
Collaborator

gjcope commented May 21, 2024

hmm may need a note in the install instructions that the dpo-voyager folder needs to be writable by all (chmod a+w .) bc npm install makes node_modules, dist, and package-lock.json

Can you describe the install process you are following in more detail? None of the installs we have on Windows or RHEL have a project folder that is writable by all.

@umer936
Copy link
Author

umer936 commented May 22, 2024

So the issue I was trying to solve with this is that I don't have NPM installed on my host system, just in the docker container and it was giving a lot of permissions issues.

I had this happen on 2 machines by two of us, one on Windows and one of us on RockyLinux.

When I attempted to just run docker-compose up, I encountered errors like this (not using sudo):
MicrosoftTeams-image (4)

and Google led me to that being due to the user in the Dockerfile being root and then npm being upset that I'm trying to run it as root. I figured this was due to conflicts in the user being used to build the container, hence the rework of the Dockerfile/start.sh/provision.sh.

That said, I started from scratch just now and didn't have an issue at all... I'm not sure where the permissions issue was stemming from. I haven't run through the commits you've done since last I pulled. Maybe it was an npm package version issue, or maybe it was user error.

I'll close this issue/PR because I can't replicate it today.

Let me know if you want me to commit the integrating start.sh into the Dockerfile, but also feel free to ignore.

@umer936 umer936 closed this May 22, 2024
@gjcope
Copy link
Collaborator

gjcope commented May 23, 2024

@umer936 Thanks for the extra context. I'm definitely interested in this PR since if you encountered the issue twice, odds are that others have as well. I just want to understand the underlying issue before making the change. If you come across any more info on what might have been different between your working/not working deployments, please do reopen. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants