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

ncm-network core schema: allow realhostname to be a short hostname #1595

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

jouvin
Copy link
Contributor

@jouvin jouvin commented Jun 28, 2023

  • Allow better support of software like Ceph where it is the recommended setting

This change is backward compatible but relaxes the requirements on realhostname property. I also checked the template-library-standard, template-library-os, template_library_core and template-library-grid repositories, as well as configuration-modules-core. realhostname is not referenced in any of them (except ncm-network schema definition intemplate_library_core) . Only hostname is set in template-library-standard but this property is not affected by this PR.

@jouvin jouvin requested review from jrha and stdweird June 28, 2023 13:49
@jouvin jouvin force-pushed the network_allow_short_hostname branch from 666cf58 to b87ac87 Compare June 28, 2023 14:51
@jrha
Copy link
Member

jrha commented Jun 28, 2023

I'm not really sure why realhostname was added, but it's definitely supposed to be a FQDN, the component uses it like this:

my $hostname = $nwtree->{realhostname} || "$nwtree->{hostname}.$nwtree->{domainname}";

ncm-network/src/main/perl/network.pm#L1973C5-L1973C91

@jouvin jouvin force-pushed the network_allow_short_hostname branch from b87ac87 to 7a85f77 Compare June 28, 2023 15:42
@jrha
Copy link
Member

jrha commented Jun 28, 2023

We don't use realhostname at RAL, what's the use-case?

@jrha
Copy link
Member

jrha commented Jun 28, 2023

Also can you provide a reference to the recommendation for short hostnames in the Ceph docs please?

@jrha jrha added this to the 23.next milestone Jun 28, 2023
@jouvin
Copy link
Contributor Author

jouvin commented Jun 28, 2023

@jrha not sure why you are saying this. What the component does it to define the host name based either on the hostname property with the domain name appended or the realhostname property used litteraly. realhostname, as the name suggests, is something that must be used as it is provided without any processing. The only requirement is that it is a valid host name value and both forms are valid (and as pointed out by the docs I mentionned, FQDN is not favoured when a machine has multiple interfaces/adresses like it is almost always the case on a Ceph machine.

@jouvin
Copy link
Contributor Author

jouvin commented Jun 28, 2023

At the end I don't care whether we relax the types allowed or add a new property like shortrealhostname (it seems to me it is an overkill if we look at the current usage of realhostname). My main point is: the hostname and realhostname properties are used by the component to pass a valid value to define the actual host name but with the current schema we don't support all the valid values at the Linux level as there is no way to use a short hostname if we want, whatever the reason. IMO, this should be fixed.

@jouvin jouvin force-pushed the network_allow_short_hostname branch from 7a85f77 to a53897d Compare June 28, 2023 17:13
@jouvin
Copy link
Contributor Author

jouvin commented Jun 28, 2023

Unit tests are failing in ncm-spma so it doesn't seem to be related. ncm-network tests are ok.

@stdweird
Copy link
Member

i once proposed the idea to add methods to ccm to extract the fqdn hostname and make it similar to realhostname || hostname.domainname, so this could be used in components that now only use hostname.domainname.
so i don't care much tbh, we also don;t use it in our profiles. but be aware that some components use the fqdn as hostname.

- Allow better support of software like Ceph where it is the recommended
setting
@jrha jrha force-pushed the network_allow_short_hostname branch from a53897d to 80fdbf9 Compare September 29, 2023 10:51
@jrha jrha merged commit 9cc4da5 into quattor:master Oct 2, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants