8000 Make docker builder optional, and simplify local test step by Catoverflow · Pull Request #619 · anvil-verifier/anvil · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Make docker builder optional, and simplify local test step #619

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

Merged
merged 19 commits into from
May 6, 2025

Conversation

Catoverflow
Copy link
Collaborator
@Catoverflow Catoverflow commented May 3, 2025

This PR makes test workflow easier to run locally.

Changes:

  • Add Rust toolchain to cache
  • Add build option without docker builder and update existing CI workflows
  • Update local-test.sh with support for local builds
  • Add documentation in build.md
  • Update builder and controller base image to Ubuntu 22.04, same as Github CI environment. New builder image is already pushed.

The reason I want to deprecate the docker builder is vargo does not reuse cache and include some changes in built binary each run (even with identical source), and that causes docker to build a new image from start, which requires a new builder image. The problem is worse with vdeployment controller, which depends on vreplicaset controller, and result in 4 fresh-built docker images ((vd+vrs) * (builder+controller)) per run locally. This quickly eats up my disk and time and I have to run docker prune every time. For cases where builder is required (different runtime environment between host and controller base image), builder can still be used. Detailed usage is in build.md.

@Catoverflow Catoverflow force-pushed the ci_cache_more branch 4 times, most recently from 133854c to 446aa17 Compare May 3, 2025 01:23
@Catoverflow Catoverflow force-pushed the ci_cache_more branch 2 times, most recently from b209f31 to f1e1fc4 Compare May 3, 2025 23:01
@Catoverflow Catoverflow force-pushed the ci_cache_more branch 2 times, most recently from 0c8578d to 4ff0ee3 Compare May 4, 2025 00:08
@Catoverflow Catoverflow changed the title Draft PR of deprecating docker builder, and simplify local test step Deprecate docker builder, and simplify local test step May 5, 2025
@Catoverflow Catoverflow requested a review from marshtompsxd May 5, 2025 05:24
8000
@marshtompsxd marshtompsxd removed the draft label May 5, 2025
@Catoverflow Catoverflow requested a review from marshtompsxd May 5, 2025 22:14
Copy link
Collaborator

Choose a reason for hiding this comment

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

What are these changes for?

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh I just realized that each task in the previous version of CI does not skip installing the rust toolchain even if it's based on build-verus. Is that the problem?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I cached Rust toolchain together with Verus, then it's unnecessary.

@marshtompsxd
Copy link
Collaborator

I don't understand why you name it as "Deprecate docker builder...". I think you are still using the docker builder, no? Please correct me if my understanding is wrong.

@Catoverflow
Copy link
Collaborator Author
Catoverflow commented May 6, 2025

I don't understand why you name it as "Deprecate docker builder...". I think you are still using the docker builder, no?

Yes I forgot to update the title...it's now completed

@Catoverflow Catoverflow changed the title Deprecate docker builder, and simplify local test step Make docker builder optional, and simplify local test step May 6, 2025
@Catoverflow Catoverflow requested a review from marshtompsxd May 6, 2025 02:28
@marshtompsxd marshtompsxd added this pull request to the merge queue May 6, 2025
Merged via the queue into main with commit 877caba May 6, 2025
19 checks passed
@Catoverflow Catoverflow deleted the ci_cache_more branch May 6, 2025 15:41
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
0