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

ci: reorg docker images #33815

Merged
merged 4 commits into from
Nov 3, 2023
Merged

ci: reorg docker images #33815

merged 4 commits into from
Nov 3, 2023

Conversation

yihau
Copy link
Member

@yihau yihau commented Oct 23, 2023

Problem

we mount home directory and cargo directory when we run docker. actually we can just install those dependencies into our docker image.

Summary of Changes

build our Docker images on Ubuntu.

(will upload these images when I get approve. I have already test them and all green 🤞 but I will still run them with our ci agents again to ensure everything is fine)

@yihau yihau requested a review from t-nelson October 23, 2023 13:08
@yihau
Copy link
Member Author

yihau commented Oct 23, 2023

btw, this one is a breaking change. will ping everyone when this one got merge.

it seems that it doesn't 🤔 I saw a pipeline success without this changes but with the latest images.

t-nelson
t-nelson previously approved these changes Oct 23, 2023
Copy link
Contributor

@t-nelson t-nelson 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!

i wonder if it'd make sense to eventually break out some of the RUN stuff to separate scripts that we can share with bare metal ci machine setup, etc

Comment on lines -42 to -47
# HACK: These are in our docker images, need to be removed from CARGO_HOME
# because we try to cache downloads across builds with CARGO_HOME
# cargo lacks a facility for "system" tooling, always tries CARGO_HOME first
cargo uninstall cargo-audit &>/dev/null || true
cargo uninstall svgbob_cli &>/dev/null || true
cargo uninstall mdbook &>/dev/null || true
Copy link
Contributor

Choose a reason for hiding this comment

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

🙌

@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

Merging #33815 (4de560c) into master (abf3b3e) will decrease coverage by 0.1%.
Report is 27 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #33815     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         807      807             
  Lines      217406   218026    +620     
=========================================
+ Hits       178043   178524    +481     
- Misses      39363    39502    +139     

@yihau
Copy link
Member Author

yihau commented Oct 24, 2023

i wonder if it'd make sense to eventually break out some of the RUN stuff to separate scripts that we can share with bare metal ci machine setup, etc

oh yeah! I think it's a good idea. If we have those scripts, we can simply copy and run them when building the Docker image. btw, I'm considering moving all tests to Docker. That way, maybe no one will need to set up their CI machines; they'll just need to install Docker 😈

@yihau yihau merged commit 662ac8b into solana-labs:master Nov 3, 2023
22 checks passed
@yihau yihau deleted the docker branch November 3, 2023 12:32
@yihau
Copy link
Member Author

yihau commented Nov 14, 2023

@t-nelson could you have a look at these 3 following PRs (#33942, #33943, #33944) by any chance 🙏

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