Augment Local Env With Coordinator-side Remote Env

[Bastian-Krause]

Description
In larger labs, maintaining and distributing environment configs for all the different places can become cumbersome. Defining resources, drivers, feature and options per place in the coordinator could help solve that problem.

This prototype adds a remote_env attribute to the Place class, a SetPlaceRemoteEnv method to the Coordinator class and a new sub command labgrid-client edit to edit the remote environment. The remote environment is printed in labgrid-client show and labgrid-client env. The RemotePlaceManager creates resources and drivers after expanding resource matches. If a local environment exists, it is augmented with the remote environment.

This is a prototype. Open tasks:

• rename "remote env" -> "remote place config"
• Allow defining options on target level (no features in remote place config, that's too intransparent, tags are already accessible via RemotePlace)
• Target.get_option() should combine local/remote options (if RemotePlace defined and not instantiated, raise)
  • deprecate config.get_target_option()
• drop commit remote/client: consider remote env resources/drivers in get_target_config()
• split labgrid.remote.client's edit_remote_env()
• Ignore RemotePlace resource in remote env
• Ignore/override drivers coming from the coordinator by specifying RemotePlace.ignore_drivers/RemotePlace.ignore_resources (class:name) in the local env (see resource/remote: add ignore_resources and ignore_drivers to RemotePlace Bastian-Krause/labgrid#15)
• Add tests
• Add documentation

Follow-up tasks could be

• add a coordinator DB storage backend
• Allow editing tags via labgrid-client edit
• Allow editing matches via labgrid-client edit
• Maybe allow imports in remote env

---

Minimal test with labgrid-client

$ export LG_PLACE=test
$ labgrid-client create
$ labgrid-client edit

Save this in your editor:

resources:          
- NetworkService:
    address: mydut.lab.example.com
    username: user
drivers:
- SSHDriver: {}

$ labgrid-client lock
$ labgrid-client ssh

---

Minimal test with pytest (using the place created above in "Minimal test with labgrid-client")
env.yaml:

targets:                          
  main:
    resources:
      RemotePlace:
        name: "test"

$ pytest --lg-env env.yaml

---

Checklist

• Documentation for the feature
• Tests for the feature

• The arguments and description in doc/configuration.rst have been updated

• Add a section on how to use the feature to doc/usage.rst

• Add a section on how to use the feature to doc/development.rst

• PR has been tested

• Man pages have been regenerated

Closes #1866

[codecov]

Codecov Report

❌ Patch coverage is 25.00000% with 78 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.8%. Comparing base (915b591) to head (2c4f2dc).
⚠️ Report is 91 commits behind head on master.

Files with missing lines	Patch %	Lines
labgrid/remote/client.py	6.8%	54 Missing ⚠️
labgrid/remote/coordinator.py	0.0%	16 Missing ⚠️
labgrid/remote/common.py	58.3%	5 Missing ⚠️
labgrid/factory.py	80.0%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #1889     +/-   ##
========================================
- Coverage    46.0%   45.8%   -0.2%     
========================================
  Files         180     180             
  Lines       14464   14561     +97     
========================================
+ Hits         6654    6674     +20     
- Misses       7810    7887     +77     

Flag	Coverage Δ
3.10	45.8% <25.0%> (-0.2%)	⬇️
3.11	45.8% <25.0%> (-0.2%)	⬇️
3.12	45.8% <25.0%> (-0.2%)	⬇️
3.13	?
3.14	45.8% <25.0%> (-0.2%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[ozan956]

Hi @Bastian-Krause,

Sorry for the late reply, I only found time to look at this now.

Overall, the approach makes sense to me. In the meeting I mentioned defining this kind of configuration via the exporters, mainly because I had the case in mind where the person changing the hardware is physically close to the setup and could update the configuration directly there. But if the remote environment can be edited via the client, then that requirement mostly goes away.

I have a few questions/comments:

First, I am wondering about the relationship between places and targets in this model. So far I have thought of a place mostly as a named group of hardware resources, while a target describes how that hardware is used for a specific use case. With the current implementation, is it possible to store more than one remote environment for a single place? I think this could be useful, for example if the same hardware setup can be used in different ways. On the CLI side, selecting one of multiple remote environments might be relatively straightforward, but I am not sure yet what the best interface would be.

Second, I would like to better understand the intended scope of this feature. Is the goal mainly to augment a local environment with configuration from the coordinator, or is it also intended to allow some users to avoid local env files completely? I think the latter is already achieved within the PR. If one user has configured the remote environment for a place, another user can directly use it without interacting with any env file at all. I think that would be a really nice property for larger labs.

One more related point: in the example, I saw a NetworkService with a username. Currently the username is required there, but in larger labs I think it would be better if user-specific details could stay local to each user. I opened a PR for that: #1863. With that change, a NetworkService could become more easily shareable within the environment, since the username can still be resolved from each user's local SSH config.

Regarding storage, I do not have a very strong opinion yet, but my first preference would probably be something versioned :)

Those were the main things that came to my mind. I would also be happy to help with some of the open tasks if needed.

[Bastian-Krause]

I have a few questions/comments:

First, I am wondering about the relationship between places and targets in this model. So far I have thought of a place mostly as a named group of hardware resources, while a target describes how that hardware is used for a specific use case. With the current implementation, is it possible to store more than one remote environment for a single place? I think this could be useful, for example if the same hardware setup can be used in different ways. On the CLI side, selecting one of multiple remote environments might be relatively straightforward, but I am not sure yet what the best interface would be.

Do you have an example how different use cases for a place could look like? If different drivers would interact with the same resources, you could simply add all of them to the place, but only activate the relevant subset of drivers for your use case? I can't really think of a remote environment making one use case incompatible. Maybe an example would help.

Second, I would like to better understand the intended scope of this feature. Is the goal mainly to augment a local environment with configuration from the coordinator, or is it also intended to allow some users to avoid local env files completely? I think the latter is already achieved within the PR. If one user has configured the remote environment for a place, another user can directly use it without interacting with any env file at all. I think that would be a really nice property for larger labs.

If your current local environment config consists only of target configurations (everything under the targets: key, see the env skeleton in the docs), I guess you don't need a local env at all.

However note that currently, at least for pytest, you need a minimal environment with the RemotePlace(s) (see "Minimal test with pytest" above). I guess a follow up PR could allow specifying one or more places directly. But that would need some restructuring of labgrid's pytest plugin fixtures currently relying on the env fixture.

One more related point: in the example, I saw a NetworkService with a username. Currently the username is required there, but in larger labs I think it would be better if user-specific details could stay local to each user. I opened a PR for that: #1863. With that change, a NetworkService could become more easily shareable within the environment, since the username can still be resolved from each user's local SSH config.

What's the use case for logging in into the DUT with you local username?

Regarding storage, I do not have a very strong opinion yet, but my first preference would probably be something versioned :)

Those were the main things that came to my mind. I would also be happy to help with some of the open tasks if needed.

Let's see what tasks I'll tackle in this PR and let's wait for it to be merged. Please let us know before you start working on any related tasks (by opening an issue or a discussion), so we don't implement things twice.

[ozan956]

Do you have an example how different use cases for a place could look like? If different drivers would interact with the same resources, you could simply add all of them to the place, but only activate the relevant subset of drivers for your use case? I can't really think of a remote environment making one use case incompatible. Maybe an example would help.

My concern is mostly about readability and CLI ergonomics, not about whether this is technically impossible to model in one environment. If a place has multiple resources/drivers of the same kind, a single combined remote environment means users need to know the internal names and pass them explicitly, for example via --name, depending on the command. If that would add too much complexity to the model or implementation, we should not have that, it is not a must anyways 👍

What's the use case for logging in into the DUT with you local username?

labgrid may open SSH connections to the exporter and copy files there, for example via ManagedFile or explicit put_file/rsync-style calls. ManagedFile currently uses a cache path by default, but users can also choose explicit remote destinations. So, I think it is useful that these exporter-side operations keep a per-user identity. Otherwise several labgrid users may effectively share the same remote context, which can cause collisions.

Let's see what tasks I'll tackle in this PR and let's wait for it to be merged. Please let us know before you start working on any related tasks (by opening an issue or a discussion), so we don't implement things twice.

Will do 👍

[ozan956]

Hey @Bastian-Krause, I implemented the following:

• Ignoring RemotePlace resources in the remote environment
• Ignoring/overriding drivers from the coordinator in the local environment via RemotePlace.ignore_drivers / RemotePlace.ignore_resources

You can see it here: Bastian-Krause#15

Maybe we could also go with (name, class) matches to ignore drivers/resources, but I think this approach is also fine, since the things that are usually ignored are single-instance resources like NetworkService.

[ozan956]

I can also work on more tests and documentation as well if you have not started yet.

[Bastian-Krause]

Discussion with @jluebbe and @Emantor:

• "remote env" should be named "remote place config"
• no features in remote place config, that's too intransparent, tags are already accessible via RemotePlace
• Target.get_option() combines local/remote (if RemotePlace defined and not instantiated, raise)
• deprecate config.get_target_option()

Edit: moved to PR description above.

[Bastian-Krause]

• drop commit remote/client: consider remote env resources/drivers in get_target_config()
• split labgrid.remote.client's edit_remote_env()

Edit: moved to PR description above.

[Bastian-Krause]

@ozan956 Thanks, I'll have a look. Please don't put any more effort into this until this we've merged #1889.

Also I'm not a huge fan of PRs in forks. No need to do anything about that now, I'll see if/how I can integrate your work. But for future work, please open a stacked draft PR and reference the base PR into the description.

[pamolloy]

@ozan956 Thanks, I'll have a look. Please don't put any more effort into this until this we've merged #1889.

Also I'm not a huge fan of PRs in forks. No need to do anything about that now, I'll see if/how I can integrate your work. But for future work, please open a stacked draft PR and reference the base PR into the description.

Hi @Bastian-Krause, do you have an idea when you might finish this? We're starting to role out Labgrid across the company and it isn't viable to have everyone manage the configurations locally (i.e. on the client), so we really need this functionality. That is why Ozan is trying to help contribute to the effort to get this across the finish line.

[Bastian-Krause]

I understand, but please try be patient. Contributions based on an in discussion draft PR are not helping, but rather slow down the process even further. We'd like to merge a solution that makes sense architecturally and does not behave surprisingly. This takes time.

[Emantor]

@sjg20 could you take a look here and let us know whether this also works for your use case?

[ozan956]

Hey @Bastian-Krause,

Any updates on that?

[ozan956]

LLM Review from our fork:

---

LLM review

This series adds a per-place "remote env": a free-form YAML block on Place
(remote_env) that lets a client instantiate extra resources/drivers
locally when a place is acquired, plus a new SetPlaceRemoteEnv RPC and a
labgrid-client edit subcommand to manage it.

run: 32356476708

c3b5bb9 - remote/client: add edit sub command

UnboundLocalError bug: in ClientSession.edit_remote_env
(labgrid/remote/client.py), key is only assigned inside the sanity-check
except Exception block. If the edited YAML passes the local sanity check
but the coordinator rejects the save (SetPlaceRemoteEnv returns
FAILED_PRECONDITION, e.g. the place changed concurrently — exactly what
the optimistic-locking changed field guards against), the
except grpc.aio.AioRpcError handler does
while key.lower() not in ("", "y", "n") with key never bound, raising
UnboundLocalError and crashing the CLI instead of prompting the user.
Reproduced by extracting the control flow into a standalone script and
running it, which raised exactly this UnboundLocalError.

62a17b9 - remote: add SetPlaceRemoteEnv

Typo: the FAILED_PRECONDITION message reads "Config was changed
elsewhere during during your edit" (duplicated word),
labgrid/remote/coordinator.py:618.

Documentation and test gaps

No files under doc/ were changed anywhere in this series — the new
remote_env concept, the SetPlaceRemoteEnv RPC, and labgrid-client edit
have no description beyond the one-line auto-generated argparse help.
doc/configuration.rst's RemotePlace section would be a natural place to
document the accepted YAML, its interaction with match-based resources, and
the "Save anyway"/optimistic-lock prompts in edit.

No test in tests/ references remote_env, SetPlaceRemoteEnv,
make_resources_from_config, or make_drivers_from_config — none of the six
commits in this series are covered by any test. Existing tests
(test_factory.py, test_remote.py) still pass (18 passed) with no
regression detected.

Verification data

• pip install -e . built cleanly.
• Ran tests/test_factory.py and tests/test_remote.py (after installing
  psutil, pytest-mock, pytest-asyncio): 18 passed, no regressions.
• Extracted the edit_remote_env retry control flow into a standalone
  script and executed it to confirm the UnboundLocalError occurs when the
  sanity check succeeds but the RPC fails.
• Traced Driver.__attrs_post_init__ to confirm the BindingError
  swallowing in TargetFactory.make_drivers_from_config(target=None, ...)
  is deliberate, and that it limits the edit sanity check to constructor
  argument validation only (not a bug, but a documented-nowhere limitation).

Suggested patches

Apply the suggested patches with:

cd path/to/repository
export GITHUB_TOKEN=ghp_***
apply-patches --repo=analogdevicesinc/labgrid 32356476708

Install instructions

The following one-liner installs the script if not present already:

curl -fSsL "https://raw.githubusercontent.com/analogdevicesinc/doctools/refs/heads/main/ci/scripts/apply-patches.sh"      -o ~/.local/bin/apply-patches.sh &&   grep -q "/apply-patches.sh" ~/.bashrc || echo "source ~/.local/bin/apply-patches.sh" >> $_ ; . $_

More information at AI Usage.

See : analogdevicesinc#4 (comment)

[Bastian-Krause]

FYI: I'll continue with this tomorrow.

[ozan956]

FYI: I'll continue with this tomorrow.

Glad to hear that! If you want, you can check my approach as well.
analogdevicesinc#5

Basically solves the problem described in prev. comment and address most of the stuff listed in that PR.