buildfarm/buildfarm

Wrapper script to allow execution on an external device

Closed this issue · 16 comments

nathaniel-brough commented

I am attempting to create a remote execution worker that works with a bare metal platform. I have created a wrapper that can be used to;

  1. Upload the firmware to the platform
  2. Run the firmware, with the correct arguments
  3. Pipe the output over serial and redirect to stdout, pipe stdin over serial to the device
  4. Return a wrapped return code from the device

For all intensive purposes this wrapper should allow the bare metal platform to execute in a similar way to how it would execute on linux.

The platforms api is working and the correct worker is selected. This is done using exec_properties attribute as can be seen here.

What I have tried so far;

  1. Modifying the worker.config.example to match the EXECUTOR exec_properties as shown here. This works as expected allowing for correct worker to be selected.
platform: {
  properties: {
     name: "EXECUTOR"
     value: "cortex_m7_fpu"
   }
}
  1. Modify the execution_policies configuration so a wrapper script is used. I have also tried "", "test" and "build" in the name attribute for execution_policies. I would expect the output of any command run with bazel run //path/to_target:main --remote_executor=grpc://localhost:8980 --platforms=@bazel_embedded//platforms:cortex_m7_fpu. To always print 'hello'. This however does not appear to happen. It seems that the wrapper script is only used when running build actions (which of course causes it to fail as I would expect it too if the action doesn't output anything). Instead it appears that the wrapper script is not called when running the executable resulting in an error cannot execute binary file: Exec format error (This error comes from /bin/bash, when trying to execute an ARM binary on x86). This shows that the wrapper attribute is being ignored in this case.
execution_policies: {
  name: "run"
  wrapper: {
    path: "/bin/echo"
    arguments: "'hello'"
}
}

Perhaps I am misunderstanding the intended usage. Regardless is there a way to acheive this?

werkt commented
nathaniel-brough commented

Replacing name: "run", with name: "", does in fact use the wrapper. As expected with this wrapper script just printing hello it doesn't produce the required outputs and causes errors. I would like to just use the wrapper when executing the resulting binary with bazel run or with bazel test, not with any of the actual build actions.

Also I am almost certain I am misunderstanding your comment but I also tried adding this;

platform: {
  # commented out here for illustrative purposes, a default empty
  # 'platform' is a sufficient starting point without specifying
  # any platform requirements on the actions' side
  ###
   properties: {
     name: "EXECUTOR"
     value: "cortex_m7_fpu"
   }
   properties: {
     name: "execution-policy"
     value: "run"
   }
}

This didn't have any effect.

I've just tried this configuration on the worker;

execution_policies: {
  name: "run"
  wrapper: {
    path: "/bin/echo"
    arguments: "'hello'"
}
}
execution_policies: {
  name: "test"
  wrapper: {
    path: "/bin/echo"
    arguments: "'hello'"
}
}

Which doesn't seem to work either. What is the intended usage for the name of the execution_policies to actions or events in bazel. Does execution_policies.name match with action mnemonics (i.e. CppCompile, CppLink, etc.), or the actual command line group (i.e. test, build, fetch etc.). Or is there something that I'm missing?

nathaniel-brough commented

So having a look through the unit tests, it does appear that my interpretation of what you mentioned is somewhat right. In particular this section of code tests the combination of a platform property with name:'execution-policy' and value:foo and a corresponding execution_policies with name:foo. Regardless, I can't find any documentation on how these execution policies are triggered and for what actions.

I have however found a workaround for the moment that seems like a bit of a hack but it should work for now. That is making use of the --run_under command line option which seems to acheive the same thing. Though specifying this per worker rather than for all workers on the same platform would be useful.

werkt commented

--run_under is definitely a workaround, but the policy application occurs for named (non-default) policies when an action, through its command, specifies a platform property as you've indicated, with name: "execution-policy", value: "foo"

werkt commented

We're definitely lacking in docs in this area, we have some brief mentions of ExecutionPolicies in Worker docs, but that can be expanded.

nathaniel-brough commented

Right, so the 'proper' way to acheive this is to find the 'action' for executing the resulting binary?

So far all I can find is this which seems to only be applicable during build, not execution of the resulting binary. It has the options;

  • exec_group
  • execution_requirements
  • mnemonic

Which look like they might be relevant. It does look like execution_requirements seem to meet the description closely.

I have also tried to use bazel aquery //:target_test on the target, which lists a bunch of actions with mnemonics. However none of the listed actions seem to be relevant to running the resulting executable, but rather look like they are just 'build' actions. E.g. the mnuemonics for the build of a target are below.

  • Middleman
  • CppCompile
  • CppLink
  • CcStrip
  • FileWrite
  • SourceSymlinkManifest
  • SymlinkTree
  • TestRunner

Of these the TestRunner action mnemonic seems relevant. But I'm not sure how mnemonics map to execution-policies, or if they do at all. Further there doesn't seem to be an equivalent action for just running an executable, only tests. This makes some sense with reference to the "Execution Phase" section of this page. I would settle for having only tests working with the execution_wrapper but would prefer to be able to use bazel run as well.

I'll have a deeper dive into the TestRunner action and see if there is an execution_requirements tag attached to this action. Or if it somehow defines an execution_property.

nathaniel-brough commented

I have found another promising approach, --modify_execution_info allows you to modify the ExecutionInfo provider, which might be sent as an execution property (I'm not sure). However it only allows you to add a key, not a value, which kinda makes it difficult to add it to the platform property and have it match something. I have tried;

  • --modify_execution_info='TestRunner=+execution-policy'
  • --modify_execution_info='TestRunner=+test'

Neither of which trigger, the execution_policies as above.

nathaniel-brough commented

Also just @werkt, I've just seen your open issue bazelbuild/bazel#10799, and PR bazelbuild/bazel#10866, that appears to be along the same lines as what I am having difficulty with. Once your PR is merged it will likely solve this issue.

werkt commented

Yup, that's an outstanding issue, and the indications from the bazel team are that my PR won't be accepted as is, since they preferred to create an exec_groups mechanism for filtering actions, based on some ability to specify 'test' actions.

Your workarounds will be the best path forwards - we also solved this to a lesser extent by making some platform requests (cpu cores min/max) applicable for heuristically detected tests only. I'd be open to creating generalized systems inside of buildfarm for respecting or ignoring platform properties and the policies they inspire if this goes on much longer.

The tracking issue for exec_groups is bazelbuild/bazel#11250, I have not yet tried to incorporate exec_groups, nor does there appear to be any documentation for it.

nathaniel-brough commented

Right no worries, I've actually settled on a different workaround for now. There are a bunch of environment variables that are defined when a test is run. They are setup here. So I have created a wrapper script that contains the logic;

if (EnvHasTestVars()) {
   TestWrapper();
} else if (EnvHasRunfileVars()) {
   RunWrapper();
else {
   BuildWrapper();
}

This means I can easily create independent wrapper scripts per worker and still differentiate between running, testing and building. It's a bit of a hack, but it will do the job until exec_groups is complete, it does look like it's a while off.

As an alternative would it make sense to make execution_policies map to mnemonics. It would allow for a similar level of granularity. I could see it working something like this.

execution_policies: {
  mnemonic: "TestRunner"
  wrapper: {
    //...
  }
}

It does look like exec_groups will have the concept of a mnemonic, so I believe it would still be a forward compatible change with the exec_groups. As a user I also found it really useful to be able to use bazel aquery to have a look at what actions where required for a target, most of which have mnemonics. Then once exec_groups, are implemented you could do something along the lines of;

execution_policies: {
  exec_group: "test"
  wrapper: {
    //...
  }
}

Also both options could coexist in a fairly logical way. e.g.

execution_policies: {
  exec_group: "test"
  mnemonic: "CppCompile"
  wrapper: {
    //...
  }
}

Would match all tests as well as a c++ compilation.

Just food, for thought. I am happy with the workarounds for now.

I'm happy to close this now, but I am also not apposed to leaving this open until a more 'tidy' solution is in place.

werkt commented

Unfortunately we don't know the mnemonic on the buildfarm side, but the detection of execution 'types' according to a build system heuristic (recognizing tests, or even compiles as you've done here) is tantalizing; I'm just not sure yet how to neatly box up the detection so that I get all the right operators.

nathaniel-brough commented

Understood, I am not familiar with bazel's remote execution system.

For reference, here is simple shell script that demonstrates my preferred workaround for a creating a wrapper script.

#!/bin/bash

if [[ -n "${TEST_TMPDIR}" ]]; then
        echo 'test action'
elif [[ -n "${RUNFILES_MANIFEST_FILE}" ]]; then
        echo 'run action'
else
        echo 'build action'
        exec "$@"
fi
werkt commented

The exec_groups code appears to have been completed upstream, which should allow full specification of the platform by sub-action type. If there are additional cases that appear to be uncovered by either your workaround or this feature of bazel, let me know. Otherwise I'll close this issue out soon.

nathaniel-brough commented

Hmm, I've been looking over the exec_group docs. I might be mistaken but I think this will only work with rules that are written in starlark. Not neccesarily the native rules i.e. cc_library, cc_binary etc.

nathaniel-brough commented

Apologies, it looks like I might have missed something here. I will investigate further.

werkt commented

Considering this closed with the support in bazel of exec_groups, execution_requirements, and exec_properties specification. This is ultimately not a buildfarm issue if there's any further concern, and is resolvable with execution policies as implemented.