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

add dockerfile to containerize conman #10

Merged
merged 4 commits into from
Apr 15, 2024

Conversation

JarettBakerDunn
Copy link
Contributor

Also add git to build environment dockerfile.
Added the docker-devel script to make it easier to use conman inside the container.

also add git to docker build env.
Also added docker-devel script to make it easier to use conman inside the container.
Copy link
Member

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good already, thanks. Two things before we move forward:

  1. Can you make sure the created executable is in the PATH environment so that users can start models wherever without knowing the location of the executable?
  2. Can you upload the generated docker image to your account already? I would like to test the docker image before merging this PR.

docker-devel Outdated
@@ -0,0 +1,3 @@
#!/bin/sh

docker run -it --rm -v $HOME/conman:/home/conman_user/work conman
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a newline at the end of the file (VS code tends to not store with ending newline and github complains about it).


RUN make

WORKDIR /home/conman_user
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Comment on lines 13 to 17
WORKDIR /home/conman_user/conman/src

RUN make

WORKDIR /home/conman_user
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could simplify this without change of functionality as:

Suggested change
WORKDIR /home/conman_user/conman/src
RUN make
WORKDIR /home/conman_user
RUN cd src; make

Copy link
Member

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, almost ready except for my small comments.

However, when I try to execute your container after pulling from docker hub I get the following error message:

exec /home/conman_user/conman/conman: exec format error

if I run without a command, or

exec /bin/bash: exec format error

if I start bash explicitly.
This looks as if the image was built for the wrong architecture, but the tag on docker hub says it was build for linux/amd64. How did you create the linux/amd64 image? Does the image run on your system? For comparison your image for geodynamics/mag runs fine for me.

docker-devel Outdated
Comment on lines 1 to 3
#!/bin/sh

docker run -it --rm -v $HOME/conman:/home/conman_user/work conman
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this file?

@@ -0,0 +1 @@
This container hosts a built version of conman.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include command to run the container here (the line from the docker-devel file above).

@JarettBakerDunn
Copy link
Contributor Author

Very nice, almost ready except for my small comments.

However, when I try to execute your container after pulling from docker hub I get the following error message:

exec /home/conman_user/conman/conman: exec format error

if I run without a command, or

exec /bin/bash: exec format error

if I start bash explicitly. This looks as if the image was built for the wrong architecture, but the tag on docker hub says it was build for linux/amd64. How did you create the linux/amd64 image? Does the image run on your system? For comparison your image for geodynamics/mag runs fine for me.

I built both images the same way using the --platform=linux/amd64 tag. Both images run on my machine, and both give a warning that the images' platform was built for amd64. I wonder if the conman image won't run for you because I pushed an arm64 image to jbakerdunn/conman before later updating the image with one built for amd64. Maybe some of the layers were not updated? I'll try deleting jbakerdunn/conman and reuploading the conman image.

Co-authored-by: Rene Gassmoeller <rene.gassmoeller@mailbox.org>
@gassmoeller
Copy link
Member

This looks good now, thanks!

@gassmoeller gassmoeller merged commit 3fab883 into geodynamics:master Apr 15, 2024
1 check passed
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.

2 participants