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

workaround to failing actions for "no space left" #3785

Merged
merged 9 commits into from
Jul 24, 2023
Merged

Conversation

wileyj
Copy link
Contributor

@wileyj wileyj commented Jul 10, 2023

ex: https://github.com/stacks-network/stacks-blockchain/actions/runs/5510601197

not sure of the root cause here, but the last i had checked this docker image was around 8GB in size.
there should be plenty of space available on the runner (historically this has been true) - so either the free space is less now for some reason, or we're using more space than we have in the past.

this is a test workaround that does 3 things:

  1. printsdh -f output
  2. runs this action to reclaim space
  3. prints df -h output again

@wileyj wileyj requested review from jcnelson and obycode July 10, 2023 17:27
@codecov
Copy link

codecov bot commented Jul 10, 2023

Codecov Report

Merging #3785 (3122f6f) into master (d15a822) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3785   +/-   ##
=======================================
  Coverage    0.18%    0.18%           
=======================================
  Files         302      301    -1     
  Lines      277126   277117    -9     
=======================================
  Hits          512      512           
+ Misses     276614   276605    -9     

see 9 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@wileyj
Copy link
Contributor Author

wileyj commented Jul 11, 2023

trigger bump

@wileyj wileyj closed this Jul 11, 2023
@wileyj wileyj reopened this Jul 11, 2023
@wileyj wileyj closed this Jul 11, 2023
@wileyj wileyj reopened this Jul 11, 2023
@wileyj wileyj closed this Jul 11, 2023
@wileyj wileyj reopened this Jul 11, 2023
@wileyj wileyj closed this Jul 11, 2023
@wileyj wileyj reopened this Jul 11, 2023
@wileyj
Copy link
Contributor Author

wileyj commented Jul 12, 2023

update:

  1. prints dh -f output
  2. runs this action to reclaim space
  3. prints df -h output again
  4. moves the docker image storage from tar -> tar.gz.

Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @wileyj!

.github/workflows/bitcoin-tests.yml Outdated Show resolved Hide resolved
Comment on lines 41 to 47

- name: Free Disk Space (Ubuntu)
uses: jlumbroso/free-disk-space@main
with:
tool-cache: true
docker-images: false

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this step necessary with the gzip change?

I'm pretty wary of using third-party actions.

Copy link
Contributor Author

@wileyj wileyj Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without that step, it's really close.
without cleaning up packages on the host, we may hit this issue again.

if this is a blocker, the only other thing we could do (outside of manually removing packages, which is what this action does) is as j2p2 suggested: docker system prune --force before our image is built.
there's about 3-4GB of images loaded by default on the runner that we don't use/need.

the free-disk-space workflow is pretty simple and straightforward: https://github.com/jlumbroso/free-disk-space/blob/main/action.yml but you're correct in that it is a 3rd party action and they could add some logic there to do something we don't like (the same can be said for other 3rd party actions we use though).

if you feel strongly about not using it, there should be just enough space by a combination of removing unnecessary docker images and using gzip vs tar. the caveat is that if the base packages change to use more space, we'll have this issue again (however, hopefully by then we won't even need this compressed docker image).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @kantai - any thoughts on my reply?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's simple enough that we could just vendor it if we're worried. I don't think Docker has the equivalent of Cargo.lock; if they truly don't, then that'd at least de-risk supply chain poisoning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless there is a strong reason to not use this action, i'd say leave it in and let it be the temporary workaround it was meant to be.
I'm also working on some methods to make this change moot; it is only meant to fix a very current and specific issue, and should not be something we use long term.

@jcnelson you do raise an interesting idea along with @kantai though - if there are concerns about using 3rd party workflows, we should have a discussion about recreating them ourselves or vendoring them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty scared of 3rd party workflows. They have created security issues in the past, and that was when managed by organizations committed to regular security audits and announcements -- workflows from solo developers and personal accounts don't have any of those processes and are probably simply too dangerous to use.

My understanding is that moving to cargo nextest would solve the space usage issue. If you're close to getting that working, I'd much prefer getting that in place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood - I do want to highlight that it feels like a "not built here" condition. But, I can refactor this PR to address your concerns and we can discuss what to do about 3rd party actions at some other time.

I'll have a commit doing what i suggested in the first reply to your comment later today. It should be enough for the short term as a workaround to this issue.

@wileyj wileyj merged commit eec2197 into master Jul 24, 2023
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Nov 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

5 participants