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

ADR #8: Container image expansion #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

moroten
Copy link

@moroten moroten commented May 5, 2022

No description provided.


Assuming that the container image files and the client provided inputs are generally located in different directories, most of the Merkle tree merging will happen close to the root level and therefore not consume much resources. Merging the layers will go deeper in the structures, so a local AC and Merkle tree cache (CAS) is probably necessary.

# Other considered solutions
Copy link
Member

Choose a reason for hiding this comment

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

I think there are a couple of other options we should consider/document:

  • Making Bazel itself responsible for emitting actions that have the right shape.
  • There is the ActionRouter API in bb-scheduler, which can already be used to do rudimentary matching/rewriting of platform properties. Maybe that subsystem could be extend to also be able to interpret container-image and rewrite the action before it's submitted to the worker?

Copy link
Author

Choose a reason for hiding this comment

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

Regarding Bazel, that would probably be the "correct" place to put it. Then Bazel itself could use the information when running locally. Maybe specifying a root_directory label in toolchains and/or platforms would be the solution. The problem is that the way Bazel put outputs would make it very impractical to specify a root tree. In practice, the only kind of root file system I can see is a container image. Referencing a container image built with Bazel would be something but I haven't looked into rules_docker yet.

Anyway, other remote executors supports the container-image platform property out of the box, so the feature might still be valuable within Buildbarn.

Am I missing something? I feel that a Buildbarn implementation would be easier than adding it to Bazel but I feel that I cannot find a good enough argument for not putting it in Bazel (more than too much effort).

Copy link
Author

Choose a reason for hiding this comment

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

The problem with ActionRouter is that it only extracts the keys, my plan was to rewrite the action, or at least the input root of it. Now, the action is already rewritten before sent over to the executor, actionWithCustomTimeout. Shall the ActionRouter be taught to return the actual_input_root_digest as well?

Copy link
Member

@EdSchouten EdSchouten May 6, 2022

Choose a reason for hiding this comment

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

Maybe we should just reach out to the Bazel folks and ask them what their vision is on this. If they are interested in solving this inside Bazel, then we should help them out. If they despise the idea, we could support it on our end.

With regards to ActionRouter: indeed! We should let that return a new input root digest.

@moroten moroten force-pushed the container-image-expansion branch from 71e3058 to d324740 Compare May 5, 2022 14:56
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