search

svalinn / DAGMC

Direct Accelerated Geometry Monte Carlo Toolkit
https://svalinn.github.io/DAGMC
Other
96 stars 63 forks source link

Streamline and simplify CI building #880

Closed gonuke closed 1 year ago

gonuke commented 1 year ago

With the success of #863, I started looking at updating our CI tests to take advantage of the new images. However, it occurred to me that with the effective caching of the multistage-docker-build-action, we don't need to generate images in a separate step. We can just do that always and have it take almost no time if the cached images are still valid.

This then also allows us to use some of the features of that action to just update the image for deployment when successful.

Hopefully this PR lets us test this a little... I'm testing it in my own repo to avoid polluting our images.

If successful, we'll be able to get rid of the deploy workflow entirely.

Expected behavior: Once successful images have been built/pushed to GHCR, this workflow should try to build every stage after first pulling and caching the previously build images. If the Dockerfile has not changed in the earlier stages, they should not require any action and will rely on the pulled/cached images. The dagmc build stage in the Dockerfile will force a full build when there is a change to the repo and then the testing stage will be run. If successful, it will push the dagmc_test stage to the repo and label it as latest.

If there are changes to earlier stages in the Dockerfile, it will execute those stages as well and continue until the tests, also.

gonuke commented 1 year ago

Converted to draft to consider which GHCR repo is the right one for doing this testing...??? In its current form, this will pull/push directly to svalinn during testing and before merge, including overwriting the existing CI images. I'm not sure that's a good plan...

ahnaf-tahmid-chowdhury commented 1 year ago

Converted to draft to consider which GHCR repo is the right one for doing this testing...??? In its current form, this will pull/push directly to svalinn during testing and before merge, including overwriting the existing CI images. I'm not sure that's a good plan...

I think, it's a good practice to consider the appropriate branch (testing-ci-update) for testing changes, especially when it involves pushing docker images to GHCR. For this, we need to modify the workflow to push docker images to the appropriate GHCR repo, ensuring it doesn't overwrite the existing CI images used in svalinn.

gonuke commented 1 year ago

Converted to draft to consider which GHCR repo is the right one for doing this testing...??? In its current form, this will pull/push directly to svalinn during testing and before merge, including overwriting the existing CI images. I'm not sure that's a good plan...

I think, it's a good practice to consider the appropriate branch (testing-ci-update) for testing changes, especially when it involves pushing docker images to GHCR. For this, we need to modify the workflow to push docker images to the appropriate GHCR repo, ensuring it doesn't overwrite the existing CI images used in svalinn.

We are somewhat constrained by the github action we are using. It is immensely valuable, but the author is open to our suggestions. I'm still feeling out the behavior to see what best to recommend for changes to that action

gonuke commented 1 year ago

This is now waiting on the merge of #893 so that full testing of a revised workflow can be performed.

gonuke commented 1 year ago

This will be ready for a full test after #894 is merged.

gonuke commented 1 year ago

The linux tests passing here are building and testing against the newly generated docker images that will be more firmly codified by #894

gonuke commented 1 year ago

For proof that this build process is using the new docker images, see the detailed test logs: Under step "Initialize containers", substep "Starting job container", on line 16 you'll see reference to: ghcr.io/svalinn/dagmc-ci-ubuntu-20.04-gcc-ext-hdf5_1.10.4-moab_5.3.0/moab:latest which is identical to the image pushed by the most recent merge of the docker deployment system.

gonuke commented 1 year ago

I updated the make parallelization per @shimwell 's suggestion. It did make the Ubuntu 20.04 clang build substantially faster, but the others were about the same. Ready for re-review (& merge?)

shimwell commented 1 year ago

ok I've poked around a bit, but can't see anything to improve on this PR. Thanks for running through that with me Paul.