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

exporter: Use fqdn instead of hostname only #1290

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

legraps
Copy link
Contributor

@legraps legraps commented Oct 24, 2023

Description
In distributed setups, the coordinator may point to places in different subnets. In order for clients to resolve resources behind the exporter, they need to know its FQDN instead of just the hostname.

Thus, set this to be the default when the exporter is invoked without the --hostname parameter.

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

@jluebbe
Copy link
Member

jluebbe commented Oct 24, 2023

This would break many existing setups where users have a .ssh/config which matches their exporters' host names. If you need to access resources which are only accessible via the exporter, use the proxy/isolated mechanism. The proxy property already uses getfqdn().

@legraps
Copy link
Contributor Author

legraps commented Oct 24, 2023

Interesting, thank you for your quick feedback!
The patch was motivated by my conception that the coordinator is designed to be the central reliable point to accumulate connectivity information rather than every developer maintaining their own config. Is this wrong somehow?

As for the proxy, yes this does work and some of my colleagues use it that way. But I also expect drawbacks in forcing all traffic through the exporter and thought it as solution for some use cases only (e.g. remote/VPN, "difficult" networks outside administration control) but not the standard where name resolution into different subnets works.

@legraps
Copy link
Contributor Author

legraps commented Nov 6, 2023

As suggested by @Bastian-Krause, I just pushed a rework introducing a flag --fqdn to change the default when no --hostname is given.

@Emantor
Copy link
Member

Emantor commented Nov 6, 2023

Please also update the man page under man/ and regenerate the files 👍

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d08687c) 63.0% compared to head (6fb3fa2) 63.0%.
Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1290     +/-   ##
========================================
- Coverage    63.0%   63.0%   -0.1%     
========================================
  Files         160     160             
  Lines       11901   11910      +9     
========================================
+ Hits         7502    7504      +2     
- Misses       4399    4406      +7     

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

labgrid/remote/exporter.py Outdated Show resolved Hide resolved
In distributed setups, the coordinator may point to exporters/places
in different subnets. In order for clients to resolve resources behind
the exporter, they need to know its FQDN instead of just the hostname.

Using an option to change the default avoids conflicts for configs which
rely on the hostname only.

Signed-off-by: Andreas Naumann <[email protected]>
@Emantor Emantor merged commit 24f81e9 into labgrid-project:master Nov 16, 2023
9 of 10 checks passed
@legraps legraps deleted the fqdn-exporter branch November 21, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants