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

Platform standardization #288

Open
sluongng opened this issue Feb 13, 2024 · 1 comment
Open

Platform standardization #288

sluongng opened this issue Feb 13, 2024 · 1 comment

Comments

@sluongng
Copy link
Contributor

Currently, folks are leveraging the generic key-value in Platform.properties to attach various forms of metadata to an Action execution. This could be as simple as requesting for a specific container image to run the action on, to a whole encoded JSON to give special instructions to the RBE worker.

Without taking away Platform.properties, I think we should try to consolidate some of these common use cases in specific fields. Clients could decide to set these specific fields to request a specific feature on the RBE worker.

For example: today, Bazel supports several tags https://bazel.build/reference/be/common-definitions#common-attributes. In which, we have:

  • Network capabilities properties: requires-network and block-network
  • Sandbox permission properties: requires-fakeroot

Instead of deciding on a specific "key" to use for these Platform properties, let's add concrete fields for the client and server to both agree on using

--- a/build/bazel/remote/execution/v2/remote_execution.proto
+++ b/build/bazel/remote/execution/v2/remote_execution.proto
@@ -773,6 +773,10 @@ message Platform {
   // be lexicographically sorted by name, and then by value. Sorting of strings
   // is done by code point, equivalently, by the UTF-8 bytes.
   repeated Property properties = 1;
+
+  bool with_network = 2;
+
+  bool with_root_access = 3;
 }

 // A `Directory` represents a directory node in a file tree, containing zero or
@EdSchouten
Copy link
Collaborator

EdSchouten commented Feb 14, 2024

Even though I'm a fan of strong typing and proper schemas, I'm not convinced that doing the above is feasible. with_network and with_root_access are fairly specific attributes that are only relevant in the domain of UNIX-like systems that are hooked up to Ethernet. REv2 isn't limited to that.

What if I want to use the protocol to build something on a Windows system? Should we also provide a with_administrator_access field? Or what if I want to run this on a system that's hooked up to a CAN bus? Should we also add a with_can_bus option? Maybe some BSD users wants to have a special option to request that System V IPC is enabled. It gets out of hand pretty quickly, and there wouldn't be an end in sight.

There are also some implementation level concerns about such an approach. A scheduler/worker might use an older schema of the protocol, meaning it ends up ignoring these options. That would cause the action to be routed incorrectly.

If the issue is:

There is currently no way for us to get access to tags like requires-network and requires-fakeroot on the server side, as they are not propagated to Platform.

Then this is something that should be discussed with the Bazel maintainers. Maybe they can patch up platform() to provide more flexibility?

platform(
    name = "linux_x86_64",
    constraint_values = [
        "@platforms//os:linux",
        "@platforms//cpu:x86_64",
    ],
    remote_execution_properties = """
        properties: {
          name: "arch"
          value: "x86_64"
        }
        properties: {
          name: "os"
          value: "ubuntu"
        }
        properties: {
          name: "os_version"
          value: "jammy"
        }
        properties: {
          name: "required_network"
          value: "${tags_of_the_target.requires_network}"           <----- Notice this here.
        }
        """,
)

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

No branches or pull requests

2 participants