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

hostname: consider systemd.hostname= static and give it precedence #25158

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hnez
Copy link

@hnez hnez commented Oct 27, 2022

The systemd.hostname= kernel commandline can for example be used to set a unique hostname on embedded devices with read-only root file systems, by letting the bootloader generate it from factory data.

The current semantics result in somewhat unexpected behaviour when doing so. Clean up the semantics of the systemd.hostname= kernel parameter to make it more useful and predictable.

The previous semantics were:

  • systemd.hostname= takes precedence over /etc/hostname during boot, even though it is technically considered a transient hostname.
  • In systemd-hostnamed however /etc/hostname takes precedence over systemd.hostname= as it is considered transient again and the normal priorities (static over transient over default) are followed. This means if a new transient hostname is set, the hostname of the system will be sethostname()'d to the one in /etc/hostname (if present) or the new transient hostname.
  • If /etc/hostname does not exist, systemd-hostnamed does not report a static hostname via dbus to consumers like the NetworkManager dhcp client. In the case of NetworkManager this results in no hostname being sent in dhcp requests.

The new semantics are:

  • systemd.hostname= is considered a static hostname during boot and in systemd-hostnamed and as such takes precedence over transient or default hostnames.
  • systemd.hostname= takes precedence over /etc/hostname.

@yuwata
Copy link
Member

yuwata commented Nov 1, 2022

Currently, systemd.hostname= is only used by PID1, but its value is 'propagated' to hostnamed through set/gethostname(), and announced in Hostname DBus property.

If hostnamed considered the hostname set through the kernel command line as static, then hostnamectl or any DBus clients cannot set a new transient hostname, as 'static' hostname is already set. I do not think it is expected.

  • If /etc/hostname does not exist, systemd-hostnamed does not report a static hostname via dbus to consumers like the NetworkManager dhcp client. In the case of NetworkManager this results in no hostname being sent in dhcp requests.

This should be handled by the client side, e.g. NetworkManager. Clients should use Hostname= property instead of StaticHostname=, or fallback to Hostname= if StaticHostname= is not specified.

  • In systemd-hostnamed however /etc/hostname takes precedence over systemd.hostname= as it is considered transient again and the normal priorities (static over transient over default) are followed. This means if a new transient hostname is set, the hostname of the system will be sethostname()'d to the one in /etc/hostname (if present) or the new transient hostname.

I think this is actually a bug, though.

@yuwata yuwata added the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Nov 1, 2022
@hnez
Copy link
Author

hnez commented Nov 1, 2022

Hi,

thanks for the feedback.

If hostnamed considered the hostname set through the kernel command line as static, then hostnamectl or any DBus clients cannot set a new transient hostname, as 'static' hostname is already set. I do not think it is expected.

I guess what to expect from the systemd.hostname= kernel parameter is not defined clearly enough yet, as that is exactly what I would expect ;).
If we were to introduce a HOSTNAME_CMDLINE HostnameSource I would expect the priorities to be (even though it is not what I have implemented as I wanted to keep the PID1 behavior as-is):

HOSTNAME_STATIC    # /etc/hostname
HOSTNAME_CMDLINE   # /proc/cmdline
HOSTNAME_TRANSIENT
HOSTNAME_DEFAULT

As that would benefit our usecase the most, where we want to be able to have a read-only /etc (without a /etc/hostname) but still want to have a static hostname configured via the bootloader, which derives it from a serial number in EEPROM.
What you describe would be (also the behavior of systemd-hostnamed if a new hostname is set):

HOSTNAME_STATIC    # /etc/hostname
HOSTNAME_TRANSIENT
HOSTNAME_CMDLINE   # /proc/cmdline
HOSTNAME_DEFAULT

What PID1 does:

HOSTNAME_CMDLINE   # /proc/cmdline
HOSTNAME_STATIC    # /etc/hostname
HOSTNAME_TRANSIENT
HOSTNAME_DEFAULT

A quick web search did not yield many results for current users of systemd.hostname= so it should still be possible to decide which priorities make the most sense. Or do you know of e.g. any distributions using it as-is?

  • If /etc/hostname does not exist, systemd-hostnamed does not report a static hostname via dbus to consumers like the NetworkManager dhcp client. In the case of NetworkManager this results in no hostname being sent in dhcp requests.

This should be handled by the client side, e.g. NetworkManager. Clients should use Hostname= property instead of StaticHostname=, or fallback to Hostname= if StaticHostname= is not specified.

In that case a new transient hostname could be set from the DHCP response, so the same client would send different hostnames in subsequent requests and depending on which networks it was connected previously. Also not quite what I would expect.

  • In systemd-hostnamed however /etc/hostname takes precedence over systemd.hostname= as it is considered transient again and the normal priorities (static over transient over default) are followed. This means if a new transient hostname is set, the hostname of the system will be sethostname()'d to the one in /etc/hostname (if present) or the new transient hostname.

I think this is actually a bug, though.

Yeah. There should be a defined list of priorities that should be the same in PID1 and hostnamed.

@poettering
Copy link
Member

hmm, interesting.

so recetnly the kernel acquired the functionality to set the hostname via a kernel cmdline option: See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5a704629f2c1ba33bbb444cb18e6957e97c76e8f

In light of that, I think it makes little sense to do exactly what the kernel supports anyway. Which makes me thinkg that systemd.hostname= should probably have precedence over /etc/hostname.

So that we come to the order of preference:

  • HOSTNAME_KERNEL_SYSTEMD (i.e. systemd.hostname=)
  • HOSTNAME_STATIC
  • HOSTNAME_TRANSIENT
  • HOSTNAME_KERNEL_NATIVE (i.e. kernel's own hostname=)
  • HOSTNAME_DEFAULT

Does that make sense?

That would mean hostname= would be more a "default" hostname, while systemd.hostname= would be a hard override.

If that works for you would be excellent to merge a patch like that.

And sorry for dropping the ball on this and being so slow to review this.

@github-actions github-actions bot added documentation tests util-lib please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jun 21, 2023
@hnez
Copy link
Author

hnez commented Jun 21, 2023

Hi,

great to see some activity on this PR again! I've finally had some time to look into your suggestion regarding systemd.hostname= and hostname= kernel parameter priorities.

I think the original PR already got us most of the way there, but I've identified one edge-case where the behavior would have been inconsistent (unsetting a hostname via empty SetHostname / SetStaticHostname DBus method calls and falling back to a default that's not hostname=).
I've added a commit that should fix this inconsistency. See the commit message for more details.

The priorities in PID1 and systemd-hostnamed are now:

  1. HOSTNAME_STATIC providers:
    1. systemd.hostname= kernel parameter
    2. /etc/hostname file
  2. HOSTNAME_TRANSIENT providers:
    1. DBus interface
  3. HOSTNAME_DEFAULT providers:
    1. SYSTEMD_DEFAULT_HOSTNAME= environment variable¹
    2. hostname= kernel parameter¹
    3. DEFAULT_HOSTNAME= parameter in the os-release file.
    4. Compiled-in FALLBACK_HOSTNAME

I've not yet done extensive testing on the hostname= behavior, as I think we should first agree if this is the correct approach.

I've done my development and testing on top of a recent main but have cherry-picked the changes back onto the branch I've opened the pull request with, because I do not know the policy with regards to rebasing a PR that is already being reviewed.
Should I push rebased version to get up to date CI results? Feel free to rebase it yourself if that is easier.


¹ Swapped in response to this comment.

@poettering
Copy link
Member

please always rebase on whatever is current main, and force push

@poettering
Copy link
Member

but yes, the priorities you proposed look good to me. we probably should put something like this somewhere in the docs, i.e. the systemd-hostnamed man page. frickin complex hierarchy ;-(

@hnez hnez force-pushed the static-cmdline-hostname branch 2 times, most recently from 5e32385 to 389fb39 Compare June 21, 2023 12:19
@hnez
Copy link
Author

hnez commented Jun 21, 2023

I've added a little snipped about the different hostname sources to man/systemd-hostnamed.service.xml now.

While at it I've also added an entry for hostname= to man/kernel-command-line.xml and updated the entry for systemd.hostname= to clarify that it takes precedence over everything else.

<term><varname>hostname=</varname></term>

<listitem><para>Accepts a hostname to set during early boot. It is used as a default until another
hostname is set via e.g. <filename>/etc/hostname</filename> or DHCP.</para></listitem>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have a comment somewhere that this is implemented by both the kernel itself and systemd, too

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the following sentence:

This parameter is intepreted by the kernel to set the initial hostname and by systemd as a fallback
if not other hostname is set.</para></listitem>

Does that sound good to you?

if (r < 0)
log_warning_errno(r, "Failed to retrieve default hostname from kernel command line, ignoring: %m");
else if (r > 0) {
if (hostname_is_valid(d, true)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uh, the 2nd argument is actually a flags argument, not a boolean one (i guess you copied this from the location this fixes → #28259, right?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(i guess you copied this from the location this fixes → #28259, right?)

Yes indeed. That change also resulted in a conflict when rebasing, as I've moved the code that was changed in #28259.

I've decided to use hostname_is_valid(h, 0) here instead of hostname_is_valid(h, VALID_HOSTNAME_TRAILING_DOT), because that's what the other checks in the function use.
Does that sound correct to you?


log_warning("Default hostname specified on kernel command line is invalid, ignoring: %s", d);
}

const char *e = secure_getenv("SYSTEMD_DEFAULT_HOSTNAME");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this explicit env var should still take precedence.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I've swapped the precedences.

@poettering
Copy link
Member

thinking about it i think 3.i and 3.ii should be swapped in yoour list above?

@poettering poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels Jul 5, 2023
@hnez hnez force-pushed the static-cmdline-hostname branch from 389fb39 to 4c1cfc7 Compare July 7, 2023 08:39
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jul 7, 2023
@github-actions
Copy link

github-actions bot commented Jul 7, 2023

We had successfully released a new major release. We are no longer in a development freeze phase.
We will try our best to get back to your PR as soon as possible. Thank you for your patience.

@hnez hnez force-pushed the static-cmdline-hostname branch from 4c1cfc7 to 4787996 Compare July 7, 2023 09:04
@hnez
Copy link
Author

hnez commented Jul 7, 2023

thinking about it i think 3.i and 3.ii should be swapped in your list above?

I went ahead and swapped the default hostname precedences in the code and also the comment just in case someone lands at this PR in the future looking for info on hostname precedences.

@yuwata yuwata added needs-rebase and removed please-review PR is ready for (re-)review by a maintainer labels Nov 11, 2023
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed needs-rebase labels Dec 15, 2023
Comment on lines +673 to +677

<xi:include href="version-info.xml" xpointer="v256"/>
</listitem>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've rebased the Pull Request (sorry for taking so long to do so - I assumed the rebase would include code changes that would require some time to resolve, but it ended up only affecting the documentation).

There is now a CI check that ensures all man page entries have a version set in which they were added.
I've just used the current version + 1 here. Is this the correct way to do it? What happens if the PR does not make it into the next release?

@yuwata yuwata added needs-rebase and removed please-review PR is ready for (re-)review by a maintainer labels Jan 11, 2024
@yuwata
Copy link
Member

yuwata commented Jan 11, 2024

Please rebase.

The systemd.hostname= kernel commandline can for example be used to set a
unique hostname on embedded devices with read-only root file systems,
by letting the bootloader generate it from factory data.

The current semantics result in somewhat unexpected behaviour when doing so.
Clean up the semantics of the systemd.hostname= kernel parameter to make it
more useful and predictable.

The previous semantics were:

 - systemd.hostname= takes precedence over /etc/hostname during boot,
   even though it is technically considered a transient hostname.
 - In systemd-hostnamed however /etc/hostname takes precedence over
   systemd.hostname= as it is considered transient again and the normal
   priorities (static over transient over default) are followed.
   This means if a new transient hostname is set, the hostname of the system
   will be sethostname()'d to the one in /etc/hostname (if present) or
   the new transient hostname.
 - If /etc/hostname does not exist, systemd-hostnamed does not report a static
   hostname via dbus to consumers like the NetworkManager dhcp client.
   In the case of NetworkManager this results in no hostname being set in dhcp
   requests.

The new semantics are:

 - systemd.hostname= is considered a static hostname during boot and in
   systemd-hostnamed and as such takes precedence over transient or default
   hostnames.
 - systemd.hostname= takes precedence over /etc/hostname.
…name

The hostname= kernel commandline parameter is interpreted by the Linux
kernel and used as the initial hostname of the system.

As of now systemd (when running as PID 1) will behave as follows with
respect to the hostname set via the kernel command line:

 - If a static hostname is configured via systemd.hostname= or /etc/hostname
   it will be set regardless of what was set via hostname=.
 - If no static hostname is configured but there is already _some_ hostname
   set (e.g. via the hostname= kernel command line), that hostname will be
   kept.
 - If no static hostname is configured and none is configured e.g. via
   hostname= a generic default is used based on e.g. the OS release.

If later on a new transient hostname is set via the `SetHostname` DBus call
to systemd-hostnamed, it will take precedence over the hostname provided via
the hostname= kernel command line, but not over the static hostname.
This means the hostname= parameter behaves like a default hostname.

There is however an inconsistency: when unsetting the transient hostname
with an empty `SetHostname` method call without a static hostname set.
In this case the hostname will not revert to the one set via the hostname=
parameter, but instead to a generic default hostname.

Fix this by considering hostname= as an actual default hostname when no
static or transient hostname is set.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
…ties

The way a hostname for the system is chosen has become quite complex
with three classes (static, transient, default) of hostnames and
multiple options per class, each with their own priorities w.r.t.
which hostname will be prefered over which other hostname.

Document the different hostname sources in the systemd-hostnamed man page.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed needs-rebase labels Jan 11, 2024
@hnez
Copy link
Author

hnez commented Jan 11, 2024

Hi, I've rebased the PR on top of recent main.

The last commit required a bit of tweaking because #30600 changed the format of the "See also" list, which this commit touched.

@poettering
Copy link
Member

lgtm.

@poettering
Copy link
Member

would love @keszybz's input before we merge this, but from my side I tink we should merge it as is

Copy link
Member

@keszybz keszybz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great that we're carefully considering the precedence here. I fully agree with making systemd.hostname= behave similarly to /etc/hostname.

But I don't understand why hostname= is used as the low-priority fallback. In the kernel patch, the intent is clearly to use the hostname= setting as a mechanism to make the actual static hostname available to the userspace earlier, before root is mounted. The example that is given is mdadm, and for this to be useful this must be not some "improved default for a fallback hostname", but the actual final hostname of the system.

In fact, it would seem that systemd.hostname= and hostname= should be almost equivalent and that we added our own setting only because the kernel didn't have its own. If there was already hostname= with those semantics on the kernel side in place, I'm pretty sure we would've just reused it instead of defining systemd.hostname=.

With the semantics described in #25158 (comment), hostname= has lower priority than both SYSTEMD_DEFAULT_HOSTNAME= which is documented as "override the compiled-in fallback hostname" and the transient hostname provided over dbus. In both cases this means that we're giving priority to explicit per-host config specified on the command line lower than fallbacks.

I would expect hostname= to be used as a source for a static hostname, with the priority order being systemd.hostname=, hostname=, /etc/hostname or systemd.hostname=, /etc/hostname, hostname=. (Though it seems strange to treat one kernel cmdline param as higher than the file and the other lower. So I like the first option better.)

tl;dr: I think we should say that the kernel grew a parameter that has the same semantics as our own "systemd.hostname=" and we're going to treat it as essentially equivalent.

Comment on lines +673 to +674
This parameter is intepreted by the kernel to set the initial hostname and by systemd as a fallback
if not other hostname is set.</para>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar: "This parameter is used by the kernel to set the initial hostname and by systemd as a fallback if there is no other configuration."

@poettering
Copy link
Member

It's great that we're carefully considering the precedence here. I fully agree with making systemd.hostname= behave similarly to /etc/hostname.

But I don't understand why hostname= is used as the low-priority fallback. In the kernel patch, the intent is clearly to use the hostname= setting as a mechanism to make the actual static hostname available to the userspace earlier, before root is mounted. The example that is given is mdadm, and for this to be useful this must be not some "improved default for a fallback hostname", but the actual final hostname of the system.

Hmm, the way I read that patch it's really just there to start out with something more reasonable than "(none)", "localhost", It's not supposed to be overriding everything, it's supposed to just close the gap until userspace takes over. At least that's my interpretation.

Hence I think this PR makes sense:

systemd.hostname= overrides, and hostname= is more of a fallback if there's nothing else.

@keszybz
Copy link
Member

keszybz commented Feb 15, 2024

We also had a discussion about this during a chat, and this changed my understanding somewhat.

  1. The kernel patch is completely unnecessary on systems using systemd, as long as systemd is also used in the initrd. Unfortunately, there are various distros that don't do this, hence the kernel parameter does make sense.
  2. We want hostnamectl hostname and the D-Bus API to be able to set the hostname.
  3. Before this patch, this was possible, with systemd.hostname= using in early boot, and then we'd switch to the hostname from /etc/hostname when systemd-hostnamed.service was started.

Hmm, the way I read that patch it's really just there to start out with something more reasonable than "(none)", "localhost", It's not supposed to be overriding everything, it's supposed to just close the gap until userspace takes over. At least that's my interpretation.

  1. As I wrote in 1. above, I think the goal is to have something exactly like systemd.hostname= for userspace where systemd is not (fully) used. In a sense this actually means "close the gap until userspace takes over". But it's not supposed to be some better fallback instead of "(none)" or "localhost", but the actual real hostname of the system. The patch description and the cover letter make this pretty clear.

With the proposed rework, we'd break the semantics of systemd.hostname=, where before it was overridable and now it'd not be.

I would expect hostname= to be used as a source for a static hostname, with the priority order being systemd.hostname=, hostname=, /etc/hostname or systemd.hostname=, /etc/hostname, hostname=.

So after the discussion, I'd change this to:
I would expect hostname= to be used as a source for a static hostname, with the priority order being /etc/hostname, systemd.hostname=, hostname=. Of course, during early boot /etc/hostname is not available, so the kernel params are used.

@keszybz
Copy link
Member

keszybz commented Feb 15, 2024

To add to this a bit: let's consider the case where you need hostname=. For example on systems like Debian, with some bash script in the initrd which doesn't use systemd.hostname= but starts udev and mdamd and whatnot. So you use hostname= and mdadm is happy and you get only one hostname in logs and things look nice in general. If you are in the same situation on a system with systemd in the initrd, then you can actually use systemd.hostname= to achieve the same effect. In both cases, after booting in the host system and starting systemd, the system should behave the same.

@poettering
Copy link
Member

@keszybz but what would systemd.hostname= be good for if it just has the same semantics as hostname=?

we usually say that kernel cmdline options we implement override userspace settings. why not just make "systemd.hostname=" work like that too?

@keszybz
Copy link
Member

keszybz commented Feb 20, 2024

@keszybz but what would systemd.hostname= be good for if it just has the same semantics as hostname=?

systemd.hostname= was added first, and later hostname= was added to support non-systemd initrds. Essentially, the kernel adopted our concept to implement the same idea. So yeah, they (should) have the same semantics.

we usually say that kernel cmdline options we implement override userspace settings. why not just make "systemd.hostname=" work like that too?

Because this breaks hostnamectl. It would also break backwards compat. (If we were adding systemd.hostname= now, then maybe we could make it have different semantics, and let plain hostname= have the semantics that systemd.hostname= has now. But that's not how things happened, and we can't just redefine what systemd.hostname= means just because the kernel added a compatible option.)

@keszybz keszybz added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels Feb 20, 2024
@michaelolbrich
Copy link
Contributor

So what started this whole thing for us was the search for a way to implement a device specific unique default hostname for embedded devices with a few properties:

  • used when the device is first shipped and after a factory reset
  • used for DHCP and MDNS to make the device discoverable in the local network
  • allow the user to overwrite it with a custom hostname

It's relatively easy to generate the hostname in the bootloader e.g. based on the serial number so using some kind of kernel command-line option made sense.

Please keep such a use-case in mind when you decide the semantics of the various sources for the hostname.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation hostname reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks tests util-lib
Development

Successfully merging this pull request may close these issues.

None yet

5 participants