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

fqdn fix #535

Closed
wants to merge 2 commits into from
Closed

fqdn fix #535

wants to merge 2 commits into from

Conversation

po1vo
Copy link
Contributor

@po1vo po1vo commented Nov 21, 2023

Sys::Hostname::hostname returns a short name, thus the fix.

@g-bougard
Copy link
Member

Hi @po1vo

indeed, on my system (Fedora 38), Sys::Hostname::hostname() still returns a FQDN. So before to consider your PR, I first need a context explaining why in your case you get a short name.

@g-bougard
Copy link
Member

g-bougard commented Nov 22, 2023

One problem I see with your PR is it may imply many changes in the inventory process as you're change can modify the way GLPI::Agent::Tools::Hostname::getHostname() api returns a value. This api is called from many point, for example it is used to compute the agent deviceid. But it is also used to set HARDWARE->NAME in inventory. What funny fact, I can see the api you're proposing is still used to set OPERATING_SYSTEM->FQDN from this module: GLPI::Agent::Task::Inventory::Generic::OS.

So can you explain first what is your concern to ask for this change ?

@po1vo
Copy link
Contributor Author

po1vo commented Nov 22, 2023

indeed, on my system (Fedora 38), Sys::Hostname::hostname() still returns a FQDN. So before to consider your PR, I first need a context explaining why in your case you get a short name.

The problem with Sys::Hostname is that it executes hostname which is system-dependent. It can either be a part of net-tools package returning short hostname by default, or it can be a part of inetutils package returning fqdn by default.

@po1vo
Copy link
Contributor Author

po1vo commented Nov 22, 2023

This api is called from many point, for example it is used to compute the agent deviceid.

deviceid must be based on fqdn, in my opinion, so there's no confusion between agents on host1.com and host1.org (for example).

@g-bougard
Copy link
Member

g-bougard commented Nov 22, 2023

This is more complicated than that:

Sys::Hostname(3pm)                                                                                                       Perl Programmers Reference Guide                                                                                                       Sys::Hostname(3pm)

NAME
       Sys::Hostname - Try every conceivable way to get hostname

SYNOPSIS
           use Sys::Hostname;
           $host = hostname;

DESCRIPTION
       Attempts several methods of getting the system hostname and then caches the result.  It tries the first available of the C library's gethostname(), "`$Config{aphostname}`", uname(2), "syscall(SYS_gethostname)", "`hostname`", "`uname -n`", and the file /com/host.  If
       all that fails it "croak"s.

       All NULs, returns, and newlines are removed from the result.

AUTHOR
       David Sundstrom <[email protected]>

       Texas Instruments

       XS code added by Greg Bacon <[email protected]>

perl v5.36.1

The hostname command execution may be part of the process, but this is not the first try. I agree hostname execution can be system-dependent. But if the problem is system-dependent, we probably need a system-dependent solution, not a solution forcing every one to switch to another solution that may have a very large and unwanted side-effect. Most people may don't care, but I don't want to see people starts to complain as we made this big change. This is why I'm asking you to share about the context.

In your last comment, you're sharing about your opinion related to the deviceid itself. Does your concern only related to the deviceid so ?

@po1vo
Copy link
Contributor Author

po1vo commented Nov 22, 2023

I've been digging into hostname and here's what i found: all implementations execute gethostname() syscall, which in turn excecutes uname() syscall.

uname man page states:

Part of the utsname information is also accessible via /proc/sys/kernel/{ostype, hostname, osrelease, version, domainname}.

So, i decided to compare /proc/sys on two hosts with different gethostname() results. Here it is self-explanatory:

host1 $ hostname
host1.local
host1 $ cat /proc/sys/kernel/hostname
host1.local
host1 $ cat /proc/sys/kernel/domainname
(none)
host2 $ hostname
host2
host2 $ cat /proc/sys/kernel/hostname
host2
host2 $ cat /proc/sys/kernel/domainname
local

@po1vo
Copy link
Contributor Author

po1vo commented Nov 22, 2023

In your last comment, you're sharing about your opinion related to the deviceid itself. Does your concern only related to the deviceid so ?

Right. We have hosts with the same hostname but different domain name.

@g-bougard
Copy link
Member

So can we make a try ?

I introduced a new option in GLPI-Agent v1.5. Here is what you can see in the Changes file:

* Feature: support assetname-support as option for agent on most unix
  if 1 (the default), the short hostname is used as asset name
  if 2, the as-is hostname (can be fqdn) is used as asset name
  this feature does not apply on MacOS or Windows

By default, it was kept to 1. So can you try to set it to 2 in conf ?
If this is not sufficient, I'll add a 3 config value support to force FQDN and then I'll use your code.

I need to fix this documentation to talk about this option: https://glpi-agent.readthedocs.io/en/latest/configuration.html

@g-bougard
Copy link
Member

g-bougard commented Nov 22, 2023

Anyway, this may be wrong for you if your concern is the agent deviceid, not the asset name. So does the asset name also a concern ?

@po1vo
Copy link
Contributor Author

po1vo commented Nov 22, 2023

I introduced a new option in GLPI-Agent v1.5. Here is what you can see in the Changes file:

I've seen it, all it does is chooses to strip or not to strip the domain part from fqdn. But it still relies on GLPI::Agent::Tools::Hostname to get fqdn.

Anyway, this may be wrong for you if your concern is the agent deviceid, not the asset name. So does the asset name also a concern ?

It is a concern.

@g-bougard
Copy link
Member

So, what is really your concern, the agent deviceid or the asset name or both ?
To be honest, agent deviceid is only an agent id and the only place I know having the domain may help to select an agent is when looking for the agent to be used by a task in GlpiInventory plugin. I really don't think this is a big deal.
So can I suppose your major concern is the asset name ? In that case, adding the 3 value support for asset-name is the best answer imho.

@po1vo
Copy link
Contributor Author

po1vo commented Nov 22, 2023

So, what is really your concern, the agent deviceid or the asset name or both ?

Both.

To be honest, agent deviceid is only an agent id and the only place I know having the domain may help to select an agent is when looking for the agent to be used by a task in GlpiInventory plugin.

Exactly.

I really don't think this is a big deal. So can I suppose your major concern is the asset name ? In that case, adding the 3 value support for asset-name is the best answer imho.

Yes, asset name. It's seems overcomplicated for such a "simple" task as getting fqdn, but it's up to you. We've been using the proposed patch for over a year with no problems.

@po1vo
Copy link
Contributor Author

po1vo commented Nov 22, 2023

Btw, Net::Domain::hostfqdn() is not perfect either. It gets domain part from the search option in /etc/resolv.conf (!)

@g-bougard
Copy link
Member

g-bougard commented Nov 22, 2023

Nothing is perfect. This just means you can set a different domain than the real hostname one... what a possible mess ! :-) This will remain a computer administration problem, not a glpi-agent one.

So I'll try to add another asset-name option value and I'll also have to use this value to define how deviceid should be set too as this seems normal to have the same base name for deviceid and asset name.

@g-bougard
Copy link
Member

As I merged #538, you'll be able to use assetname-support = 3 in agent config to achieve the same purpose.

Thank you for the suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants