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

Tar home build #778

Closed
wants to merge 2 commits into from
Closed

Tar home build #778

wants to merge 2 commits into from

Conversation

danielhollas
Copy link
Contributor

@danielhollas danielhollas commented Jul 19, 2024

Alternative to #740

The main goal here is to reduce the complexity of status quo and of #740.

The strategy of archiving home directory and untarring it at startup actually allows for a lot of simplification of the Dockerfile since everything can be directly prepared in home folder, without intermediary steps, and this allows to get rid of the current startup scripts (70_, 71_)

All startup scripts from full-stack are preserved and reused, which minimizes duplication, resolves the SSH key issue and should be more maintainable

The only new startup script is the 00_untar_home.sh which is basically the same here as in

https://github.com/aiidalab/aiidalab-qe/pull/740.
I've also done some unrelated cleanup:
    Moved Dockerfile to the root dir, and got rid of build.json and docker-bake.hcl which were introducing unnecessary complexity. One can now build the image by simply running docker build . -t aiidalab/qe:newbuild . This also allowed me to use podman on my dev machine, which otherwise doesn't support the buildx backend.

I've done some quick benchmarking, at starting the container takes around 12s on my machine. The image takes around 6.5Gb. We could trade around 300Mb images size for extra 3s of startup time if we compressed the home.tar archive. (My timings seems roughly consistent with those observed in #740.

Reducing the image size will be done in subsequent PR

@unkcpz unkcpz mentioned this pull request Jul 19, 2024
Copy link

codecov bot commented Jul 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.28%. Comparing base (cce0c91) to head (9f30615).
Report is 36 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #778   +/-   ##
=======================================
  Coverage   68.28%   68.28%           
=======================================
  Files          45       45           
  Lines        4143     4143           
=======================================
  Hits         2829     2829           
  Misses       1314     1314           
Flag Coverage Δ
python-3.10 68.28% <ø> (ø)
python-3.9 68.31% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielhollas danielhollas marked this pull request as ready for review July 19, 2024 09:53
@unkcpz
Copy link
Member

unkcpz commented Jul 19, 2024

Deployed and alive on https://staging-demo.aiidalab.io/
Looks all good!

@danielhollas
Copy link
Contributor Author

I've opened #781 using a better strategy (multistage build).

@danielhollas danielhollas deleted the home-build-new branch July 22, 2024 21:24
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