-
-
Notifications
You must be signed in to change notification settings - Fork 636
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
GH Action for docker builds #1724
GH Action for docker builds #1724
Conversation
I think I finally got it how to bind particular step with certain dir update. |
Nope) |
e052f9f
to
6f69470
Compare
Thanks for testing that, @trsvchn ! |
@vfdev-5 probably, but we can verify it only after the merge, like we did for code-style |
660067c
to
c5558f8
Compare
Sounds good! Let's do it like for code-style. Just to make sure, the permission issue there is related to probably skipped |
I see that you are trying to find out if trigger next stops or not depending on modified. I did something like that here : https://github.com/pytorch/ignite/blob/master/.circleci/trigger_if_modified.sh . May help... |
it's hard to say, we need to try. I don't know how user defined tokens interact with |
oh, thanks, I will check it! |
@vfdev-5 ignite/.circleci/trigger_if_modified.sh Line 41 in 007d320
I've already tried:
and
But it didn't help I think it's because of checkout action and detached head checkout by default |
going to try prepared solutions) |
ae3763a
to
69f52e4
Compare
wow that action works! |
going to update some docker |
Oh, yeah it works! |
Interesting fact: |
Great! Currently, the problem will be with GA that wont be able to build our large HVD images...
|
@vfdev-5 so we are going to try to trigger Circle CI through |
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.
Thanks a lot @trsvchn !
@trsvchn oh, I just realized that we are not doing correctly here now:
Otherwise, buildling for example |
@vfdev-5 Oh, because of multi-stage builds, I have never tried it btw, how does it work? so |
Well, actually, it is not because of mutli-stage but just, for example, nlp image uses
in parallel
Yes, they should have access to that build image. |
Or, we can try smth like: jobs:
build-base:
needs: setup
# build base
# upload image as artifact
build-vision:
needs: build-base
# download image as artifact
# build vision
... |
Because I'm not sure about space |
Instead of uploading temp images, we can try if steps could reuse the same docker env between... Yes, space question can be blocking ... |
|
there is also https://github.community/t/whats-the-recommended-way-to-pass-a-docker-image-to-the-next-job-in-a-workflow/17225/4
|
We can use Package Registr to store our docker images |
I think we can pass docker env between steps in a single job, right ? Otherwise, yes, we can use GH Package registry.
but we can run out of memory... |
Yep, exactly! especially in hvd case |
OK, let's try either :
Maybe, 2) will be a more adapted solution, I'm just afraid of it could used publicly (what we would like to avoid) |
|
I am going to try this artifact experiment in my fork, to avoid massive artifact mess up here :) |
Sounds great! Thanks @trsvchn ! |
Well, I've configured artifacts exchange between jobs, and it works fine for small images. results: https://github.com/trsvchn/ignite/actions/runs/618227617 The problem is in the lack of space when making a tar archive for image, I'm talking about: docker save -o image.tar image:latest but we get: 2021-03-03T21:13:34.7276664Z ##[warning]You are running out of disk space. The runner will stop working when the machine runs out of disk space. Free space left: 74 MB I've inspected GH Actions runner space. This is default filesystem on start (before docker build):
I'm thinking to remove some packages to free up space for docker save. @vfdev-5 @ydcjeff @sdesrozis What heavy packages can we remove w/o effect on docker? Here is Top 50 944.4M ghc-8.10.4
924.8M ghc-9.0.1
761.9M azure-cli
471.9M google-cloud-sdk
308.9M adoptopenjdk-11-hotspot
282.0M libgl1-mesa-dri
280.4M hhvm
228.3M google-chrome-stable
211.8M firefox
208.5M dotnet-sdk-5.0
194.4M adoptopenjdk-8-hotspot
193.6M llvm-10-dev
189.5M llvm-9-dev
185.0M dotnet-sdk-3.1
170.2M powershell
163.1M llvm-8-dev
135.2M moby-containerd
120.5M snapd
118.9M mysql-server-core-8.0
114.5M moby-engine
109.5M mono-devel
81.0M podman
79.3M libllvm11
78.7M mongodb-org-server
70.3M libllvm10
69.4M dotnet-runtime-3.1
68.9M moby-cli
68.1M linux-azure-headers-5.4.0-1040
66.7M dotnet-runtime-5.0
66.1M libllvm9
62.3M linux-modules-5.4.0-1040-azure
61.8M mysql-client-core-8.0
61.5M mongodb-org-mongos
60.0M mono-llvm-tools
59.8M libllvm8
58.6M moby-buildx
56.9M gcc-10
55.3M ansible
55.1M libclang-common-10-dev
51.8M mecab-ipadic
51.0M mongodb-org-shell
49.1M containernetworking-plugins
46.5M postgresql-13
44.6M libclang-common-9-dev
44.5M msbuild
42.7M libclang-common-8-dev
42.6M libclang-cpp10
42.6M libicu-dev
38.4M kubectl
38.1M r-base-core |
@trsvchn thanks for the update ! I'm not sure if this is a good idea to remove system packages as github actions can rely on some of them ... |
@vfdev-5 Yes, I was thinking about it, but can we remove it? I've tried locally and got this Error response from daemon: conflict: unable to delete 7554ac65eba5 (cannot be forced) - image has dependent child images |
Yes, true, we can not remove it for msdp images, but should work for hvd. Let me check if we can use github registry without exposing it publicly ... |
From actions/runner-images#709 (comment)
sudo rm -rf "/usr/local/share/boost"
sudo rm -rf "$AGENT_TOOLSDIRECTORY" |
We can try that as well |
Regarding the packages below
do we need
Sorry if this is not relevant according to the current thread .... |
@sdesrozis Thanks for suggestions! But idea save/load docker image is not working, now we are building all 3 images in a one job and using sudo rm -rf "/usr/local/share/boost"
sudo rm -rf "$AGENT_TOOLSDIRECTORY" frees enough space for that purpose |
Hi, is this issue still not resolved. If not, can I get the issue note on the last updated work? |
@harshit2000 yes this PR is merged and fixed related issues, please check its description. |
@vfdev-5 Sure, thanks for the info |
Related to #1644 and #1721
Description:
Check list: