chore: Ensure container image can be started correctly after build #32
Conversation
papey
force-pushed
the
add-curl-check-before-pushing-image
branch
from
974979b to
79d01f8
Compare
.github/workflows/ci.yml
Outdated
| cd neurow && docker build . -t neurow:latest | ||
| docker run -d --name neurow_test neurow:latest | ||
| sleep 5 | ||
| docker stop neurow_test |
There was a problem hiding this comment.
You will not detect anything without a curl. One way to do this is to run a docker compose which start a second container, and curl the first one.
There was a problem hiding this comment.
Yes sure, since curl is already included, maybe we can just docker exec and run the curl inside the started container ?
papey
force-pushed
the
add-curl-check-before-pushing-image
branch
from
26cab19 to
23a979e
Compare
papey
force-pushed
the
add-curl-check-before-pushing-image
branch
from
2507286 to
8a653a7
Compare
papey
force-pushed
the
add-curl-check-before-pushing-image
branch
2 times, most recently
from
4398261 to
0fa80ce
Compare
papey
force-pushed
the
add-curl-check-before-pushing-image
branch
from
0fa80ce to
0418eb9
Compare
| - run: cd neurow && docker build . | ||
| - run: | | ||
| cd neurow | ||
| docker build . -t neurow:${{ github.sha }} |
There was a problem hiding this comment.
Is there a way not to build the image twice ? (One time for the check + one time for the actual push)
There was a problem hiding this comment.
This workflow runs on every PR, and it's just about building the image (ci.yml). The one that pushes runs only on main (docker_push.yml).
There was a problem hiding this comment.
So this step ensure that container image from a PR can be build and started properly.
Changes
This PR adds an extra step on the docker build to ensure that the container is able to start correctly and that the internal API is responding on port 3000 on the ping endpoint.
Since the image is already embarking curl, probably for healthcheck reason, I run the command directly in the container context.
It's also possible to run it directly from the CI env but this will introduce coupling on port used and I think it's can mess up since the host port would not be able to be mapped more than once.