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

Provider Improvements and Fixes #1249

Merged

Conversation

Bastian-Krause
Copy link
Member

Description
Collection of various provider fixes and improvements:

  • fix typo in the NFSProviderDriver class name
  • make the NFSProviderDriver usable
  • slim down NFSProvider resources (local and remote)
  • fix exporting host information for local provider resources
  • extend documentation

Contains breaking changes mentioned in CHANGES.rst.

Checklist

  • Documentation for the feature
  • [.] Tests for the feature
  • The arguments and description in doc/configuration.rst have been updated
  • PR has been tested

Fixes #762

The typo has been in the class name since its introduction. The
documentation contains the correct name "NFSProviderDriver" already.

Signed-off-by: Bastian Krause <[email protected]>
This allows the user to import these resources from labgrid.resource
directly.

Signed-off-by: Bastian Krause <[email protected]>
The TFTPProvider, HTTPProvider and NFSProvider don't have a host
property unlike the remote variants.

Fix the `get_export_vars()` method by distinguishing local and remote
resources: use the provider's host attribute for remote resources,
otherwise return "localhost".

Signed-off-by: Bastian Krause <[email protected]>
There is no need to overwrite this method to only call the same method
on super(). This is the default.

Signed-off-by: Bastian Krause <[email protected]>
This path will be used by the NFSProviderDriver in a future commit.

Signed-off-by: Bastian Krause <[email protected]>
During the introduction of the provider drivers, it was assumed all
provider drivers work the same way: by relying on the
`internal`/`external` attributes of their corresponding provider
resource and symlinking the required file (either from
/var/cache/labgrid/$USER/$HASH/file or the given path, in case it's
locally available) to the `internal` path. For the target, the file
is now expected to be available below `external` as returned by
`stage()`.

This, however, does not work for NFS: all NFS servers known to me
don't touch the symlinks, meaning in our case that they point to a
location not exported via NFS. Exporting and mounting these destinations
to the same location is impractical at best or does not work at all.
On top of that, the `internal`/`external` lingo does not fit the NFS
mount use case.

To improve this situation, introduce a custom `stage()` method for the
NFSProviderDriver: assume that /var/cache/labgrid is exported via NFS,
always copy the file to be staged there (even if it would be locally
available) and return an NFSFile object containing the information
required to mount and access the NFS share: host, export and the file
path relative to the export.

Signed-off-by: Bastian Krause <[email protected]>
…vider

Now that the NFSProviderDriver no longer relies on the
`internal`/`external` attributes of the (Remote)NFSProvider resource,
let the NFSProvider inherit from Resource directly, so specifying these
obsolete attributes is no longer required. The same applies to the
RemoteNFSProvider which now inherits from NetworkResource directly.

Signed-off-by: Bastian Krause <[email protected]>
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Patch coverage: 73.3% and project coverage change: -0.1% ⚠️

Comparison is base (3e1c0df) 62.9% compared to head (c345273) 62.9%.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1249     +/-   ##
========================================
- Coverage    62.9%   62.9%   -0.1%     
========================================
  Files         161     161             
  Lines       11861   11882     +21     
========================================
+ Hits         7471    7482     +11     
- Misses       4390    4400     +10     
Files Changed Coverage Δ
labgrid/driver/provider.py 76.5% <64.7%> (-8.3%) ⬇️
labgrid/resource/provider.py 91.6% <71.4%> (-8.4%) ⬇️
labgrid/driver/__init__.py 100.0% <100.0%> (ø)
labgrid/resource/__init__.py 100.0% <100.0%> (ø)
labgrid/resource/remote.py 82.7% <100.0%> (ø)
labgrid/util/managedfile.py 80.9% <100.0%> (+0.4%) ⬆️

... and 1 file with indirect coverage changes

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

@Emantor
Copy link
Member

Emantor commented Sep 6, 2023

@Bastian-Krause did you test this with a local NFS Server?

@Bastian-Krause
Copy link
Member Author

@Bastian-Krause did you test this with a local NFS Server?

Yes, I've tested this with an NFS server and I have a customer strategy using this that runs successfully with this PR.

@Emantor Emantor merged commit 6a11213 into labgrid-project:master Oct 12, 2023
8 of 9 checks passed
@Bastian-Krause Bastian-Krause deleted the bst/provider-improvements branch October 12, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants