labgrid-project / labgrid

Embedded systems control library for development, testing and installation
https://labgrid.readthedocs.io/
Other
328 stars 167 forks source link

Exporting named resources is broken #248

Closed ghost closed 5 years ago

ghost commented 6 years ago

Goal is to run tests on a distributed setup where an exporter provides multiple named resources of the same type that are bound to drivers on the client.

Example exporter config

board1:
  NetworkService:
    name: network-board1
    address: 1.1.1.1
    username: root
board2:
  NetworkService:
    name: network-board2
    address: 2.2.2.2
    username: root

Example environment config

targets:
  main:
    resources:
      RemotePlace:
        name: remoteboards
    drivers:
      SSHDriver:
        keyfile: 'riot'
        bindings:
          networkservice: network-board1

This setup exposes two potential errors, one at the client and one at the exporter

  1. client side error
self = <labgrid.factory.TargetFactory object at 0x7fbd457450b8>
target = Target(name='main', env=Environment(config_file='remote.yaml')), resource = 'NetworkService', name = 'NetworkService'
args = {'address': '2.2.2.2', 'name': 'network-board2', 'username': 'root'}

    def make_resource(self, target, resource, name, args):
        print('Making resource {} for target {} named {} with args {}'.format(resource,target,name,args))
        assert isinstance(args, dict)
        if not resource in self.resources:
            raise InvalidConfigError("unknown resource class {}".format(resource))
        try:
            cls = self.resources[resource]
            args = filter_dict(args, cls, warn=True)
            if 'name' in args:
               print('name given by args: {} name by function {}'.format(args['name'], name))
>           r = cls(target, name, **args)
E           TypeError: __init__() got multiple values for argument 'name'

When calling make_resource labgrid/resource/remote.py does not remove the name key from the arguments, as done in ./labgrid/factory.py

Here is a fix for the problem

index 6398550..7c5eabe 100644
--- a/labgrid/resource/remote.py
+++ b/labgrid/resource/remote.py
@@ -45,8 +45,10 @@ class RemotePlaceManager(ResourceManager):
         resource_entries = self.session.get_target_resources(place)
         expanded = []
         for resource_name, resource_entry in resource_entries.items():
+            args = resource_entry.args
+            name = args.pop('name', resource_name)
             new = target_factory.make_resource(
-                remote_place.target, resource_entry.cls, resource_name, resource_entry.args)
+                remote_place.target, resource_entry.cls, name, args)
             new.parent = remote_place
             new.avail = resource_entry.avail
             new.extra = resource_entry.extra
  1. Exporter side error

The following is observed for the setup

root@9f1bafc24e84:/opt/labgrid# labgrid-client -x ws://coordinator:20408/ws resources
remote/board1/NetworkService
remote/board2/NetworkService

root@9f1bafc24e84:/opt/labgrid# labgrid-client -x ws://coordinator:20408/ws -p remoteboards create
root@9f1bafc24e84:/opt/labgrid# labgrid-client -x ws://coordinator:20408/ws -p remoteboards add-match remote/*/NetworkService
root@9f1bafc24e84:/opt/labgrid# labgrid-client -x ws://coordinator:20408/ws -p remoteboards show
Place 'remoteboards':
  aliases: 
  comment: 
  matches:
    remote/*/NetworkService
  acquired: None
  acquired resources:
  allowed: 
  created: 2018-05-25 14:08:12.618946
  changed: 2018-05-25 14:08:17.078097
Matching resource 'NetworkService' (remote/board1/NetworkService/NetworkService):
  {'acquired': None,
   'avail': True,
   'cls': 'NetworkService',
   'params': {'address': '1.1.1.1', 'name': 'network-board1', 'username': 'root'}}
Matching resource 'NetworkService' (remote/board2/NetworkService/NetworkService):
  {'acquired': None,
   'avail': True,
   'cls': 'NetworkService',
   'params': {'address': '2.2.2.2', 'name': 'network-board2', 'username': 'root'}}

We observe that the services are named remote/board1/NetworkService/NetworkService and remote/board2/NetworkService/NetworkService, while we expect remote/board1/NetworkService/network-board1 and remote/board2/NetworkService/network-board2

The client uses the same resource_name NetworkService as a key in directories for both resources, which makes only one of them available, when checking the environment

root@9f1bafc24e84:/opt/labgrid# labgrid-client -x ws://coordinator:20408/ws -p remoteboards env
OrderedDict([('name', 'network-board2'), ('address', '2.2.2.2'), ('username', 'root')])
targets:
  remoteboards:
    resources:
    - NetworkService:
        name: network-board2
        address: 2.2.2.2
        username: root

resource_name is NetworkService for all resources 

and when running a test

    def get_resource(self, cls, *, name=None, await=True):
        """
            Helper function to get a resource of the target.
            Returns the first valid resource found, otherwise None.

            Arguments:
            cls -- resource-class to return as a resource
            name -- optional name to use as a filter
            await -- wait for the resource to become available (default True)
            """
        found = []
        other_names = []
        if type(cls) is str:
            cls = self._class_from_string(cls)

        for res in self.resources:
            if not isinstance(res, cls):
                continue
            if name and res.name != name:
                other_names.append(res.name)
                continue
            found.append(res)
        if len(found) == 0:
            if other_names:
                raise NoResourceFoundError(
                    "all resources matching {} found in target {} have other names: {}".format(
>                       cls, self, other_names)
                )
E               labgrid.exceptions.NoResourceFoundError: all resources matching <class 'labgrid.resource.networkservice.NetworkService'> found in target Target(name='main', env=Environment(config_file='remote.yaml')) have other names: ['network-board2']

A fix is to check if a name for the resource is provided in the arguments at the exporter

--- a/labgrid/remote/exporter.py
+++ b/labgrid/remote/exporter.py
@@ -354,6 +354,7 @@ class ExporterSession(ApplicationSession):
                     if params is None:
                         continue
                     cls = params.pop('cls', resource_name)
+                    if 'name' in params: resource_name = params['name']
                     yield from self.add_resource(
                         group_name, resource_name, cls, params
                     )

In general the code is sometimes confusing, as resource_name as variable is often used for the type of the resource, for example NetworkService.

avoine commented 6 years ago

Indeed this would be useful for us too. Should I go on and create a PR?

Emantor commented 6 years ago

For the second part of the issue, the client has a function named 'add-named-match' this allows to set the match by name:

 ~/tmp/labgrid-test  labgrid-client -p remoteboards add-named-match dibooki/board1/NetworkService network-board1
 ~/tmp/labgrid-test  labgrid-client -p remoteboards show
Place 'remoteboards':
  aliases:
  comment:
  matches:
    dibooki/board1/NetworkService → network-board1
  acquired: None
  acquired resources:
  allowed:
  created: 2018-09-05 18:30:53.411019
  changed: 2018-09-05 18:32:10.518305
Matching resource 'network-board1' (dibooki/board1/NetworkService/NetworkService):
  {'acquired': None,
   'avail': True,
   'cls': 'NetworkService',
   'params': {'address': '1.1.1.1', 'name': 'network-board1', 'username': 'root'}}

The named match is indicated by the small arrow above. I think the first part may also be caused by using a normal match. Please test the function, I'll add documentation for the feature.

jluebbe commented 5 years ago

@cadenv and @avoine Is this still an issue for you? Otherwise I'd close this.

Emantor commented 5 years ago

@cadenv, @avoine I'll close this issue for now, feel free to open a new one if your still encounter a problem with named resources.