-
Notifications
You must be signed in to change notification settings - Fork 126
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
Adding hostile v1.1.0 #1111
Adding hostile v1.1.0 #1111
Conversation
The tests appeared to have worked:
|
This is looking great! Some changes:
|
ARG HOSTILE_VER="1.1.0" | ||
|
||
# Stage 1: Build Dockerfile | ||
FROM ubuntu:focal AS builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setting this as ubuntu:jammy might be helpful for you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made all the changes you suggested besides this one. For some reason i was getting a different error when using ubuntu:jammy
, so just switched it back but if its a good switch you think I'm more than willing to go through and fix the error, but wasn't sure if it was worth it or not in terms for the docker build.
Thank you for looking into changing the base. I was hopeful it would be helpful. It looks like the tests still work
|
hostile/1.1.0/Dockerfile
Outdated
LABEL maintainer.email="[email protected]" | ||
|
||
COPY --from=builder /usr/ /usr/ | ||
COPY --from=builder /hostile-1.1.0/tests/data/sars-cov-2/ /data/test/sars-cov-2/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these files needed at runtime or are they used for testing/demo-ing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those files I had to do the testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are they beneficial to keep in the final image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no reason to keep them in the final image, just using for test. This program was a bit difficult (at least for me) to build a container for. So any suggestions on what you think i should include or modify for the final image would be appreciated!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these files aren't used at runtime, then we can drop this line to decrease the image size.
If they're helpful, though, they can stay in.
hostile/1.1.0/Dockerfile
Outdated
LABEL base.image="ubuntu:focal" | ||
LABEL dockerfile.version="1" | ||
LABEL software="hostile" | ||
LABEL software.version="${HOSTILE_VER}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add the following to line 41-ish (a line after the app stage starts, but before the LABEL lines).
ARG HOSTILE_VER
Right now, the software.version
label isn't registering and is causing a GA action test to fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah must have missed that, fixed now!
hostile/1.1.0/Dockerfile
Outdated
LABEL maintainer.email="[email protected]" | ||
|
||
COPY --from=builder /usr/ /usr/ | ||
COPY --from=builder /hostile-1.1.0/tests/data/sars-cov-2/ /data/test/sars-cov-2/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change
COPY --from=builder /hostile-1.1.0/tests/data/sars-cov-2/ /data/test/sars-cov-2/
to something like the following
COPY --from=builder /hostile-${HOSTILE_VER}/tests/data/sars-cov-2/ /data/test/sars-cov-2/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, missed that change as well, now fixed.
Test still work:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no additional changes to recommend.
Thank you for putting this together!!!
I've started the GA to build and deploy this image to dockerhub and quay. You can check the status of the deployment here : https://github.com/StaPH-B/docker-builds/actions/runs/12244563176 Let us know if you run into problems!!! |
Pull Request (PR) checklist:
docker build --tag samtools:1.15test --target test docker-builds/samtools/1.15
)spades/3.12.0/Dockerfile
)shigatyper/2.0.1/test.sh
)spades/3.12.0/README.md
)