[spec2x] Persist the first-boot hostname if not otherwise set #156

darkmuggle · 2020-03-06T20:31:48Z

This resolves the problem where ip=... defines a hostname but the hostname is not persisted.

laenion

Is this service supposed to be CoreOS specific? If not I'd suggest renaming coreos-persist-hostname.service to ignition-persist-hostname.service..

cgwalters · 2020-03-06T21:40:15Z

Is this service supposed to be CoreOS specific? If not I'd suggest renaming coreos-persist-hostname.service to ignition-persist-hostname.service..

The spec2x branch is currently really RHCOS specific, as is this "persist kargs into ifcfg files". The vision for FCOS is that we use NetworkManager in the initrd, which would solve this problem.

We already have another unit with the coreos- naming here; so it has precedent. (But it could be rhcos- I guess)

cgwalters · 2020-03-06T21:41:58Z

dracut/30ignition/coreos-persist-hostname.service

+
+# Only trigger when the hostname is NOT A default.
+ConditionHost=!localhost
+ConditionHost=!localhost.localdomain


Hmm why a separate unit versus doing this as part of the existing logic for coreos-teardown-initramfs-network.service ?

Which currently only triggers effectively when the kernel cmdline has a non-default ip= - i.e. basically a static IP.

The separate unit made the logic simplifier, e.g. ConditionHost=localhost and because this also applies to the DHCP situation. For RHCOS we don't want transient names to be used -- the firstboot hostname should be persisted. This ensures that.

Are we really sure this applies to DHCP too? That scopes this code into running in e.g. AWS/GCP etc. as well as the baremetal environments that use DHCP.

Now, I don't think it's at all supported to change the hostnames returned from DHCP for OpenShift/Kubernetes today. So, it's probably not going to hurt to persist it and thereafter ignore what DHCP says, but...no one has been asking us to do that either - the filed bug was about bonding specified on the kernel cmdline. The bugs in DHCP cases just boiled down to kubelet needing to start after NM had finished DHCP.

One thing to bear in mind here is that Kubernetes also requires the hostname to be routable - which implies it has to be correct in the infrastructure and not just the OS. Hence, source-of-truth is the infrastructure.

For DHCP cases, there's already a node-external source of truth that people know how to manage.

For cases where the IP address (and hostname) are provided on the kernel cmdline...well, source of truth may need to be updated in two places (infra and hosts) if any IP addresses need to change. The core bug we're fixing here is ensuring that we persist that kernel cmdline across reboots (but we could also fix it by parsing the kernel commandline in the real root on subsequent boots).

The way I'd been thinking of this is that the argument for why this is RHCOS specific is that in the future in FCOS (and ideally RHCOS) with NM-in-initrd, it would parse the kernel arguments and handle this itself, correctly transferring state across the initrd.

Are we really sure this applies to DHCP too?

Better question: do we want to allow changing the host via DHCP for OCP?

I think we all agree that doesn't work. The question is more - does fixing the hostname for DHCP help anything either?

Now, I know for sure that if we had shipped this patch from the start it would have completely avoided the need for the kubelet waiting on NM patch. But, we are on track to ship that patch long before this one.

cgwalters · 2020-03-06T22:07:21Z

Finally, the use of transient hostnames is an anti-pattern for Ignition.
Changing an aspect of the machines identity (e.g. the hostname) after
provisioning can break the service running on-top the host.

I see the argument here, but the counter I think is:

Anyone who wants to persistently set the hostname today can do so by simply writing /etc/hostname into Ignition
Not all applications are going to break when the hostname changes - I mean, my desktop doesn't. The "point of no return" here is around when a node has joined the cluster in Kubernetes, but that is currently distinct from the Ignition boot for us. So we could have something on the host that screams loudly if the hostname returned from DHCP differs from the one in the kubeconfig or so that's written via the MCO (which would also apply to OKD+FCOS)

dcbw · 2020-03-06T22:15:44Z

Finally, the use of transient hostnames is an anti-pattern for Ignition.
Changing an aspect of the machines identity (e.g. the hostname) after
provisioning can break the service running on-top the host.

@darkmuggle I think that Ignition requirement is in direct conflict with expectations in the DHCP and DNS cases. Here the source of truth is external to the host, and for OpenShift we must have the host agree with the external source of truth about the hostname. If the hostname is locked on install, and the external source of truth changes but the node doesn't follow that change, we have a problem.

For DHCP your hostname may change any time the lease is renewed or re-acquired. In practice this doesn't happen often but is certainly a possibility. Same for DNS; that's entirely out of control of the host.

For OpenShift, if a node has a hostname out of sync with DHCP/DNS, kubelet will post that hostname as the node name, and other components in the cluster will try to resolve that hostname and fail. The workaround is to use IP addresses but that requires specific configuration on each node and is pretty ugly.

TLDR; my uninformed opinion is that it should work as such:

an ignition payload for /etc/hostname: persist this hostname because admin presumably knows what they are doing
ip=... kargs to explicitly set it: persist this hostname because admin presumably knows what they are doing
Hostname from DHCP: use this hostname until reboot
DNS PTR record lookup: use this hostname until reboot or the IPs change

dcbw · 2020-03-06T22:18:45Z

* I mean, my desktop doesn't.

@cgwalters we worked hard to make that happen like 10 years ago, IIRC we needed to make xauth not care about hostname by hardcoding localhost or something like that.

cgwalters · 2020-03-06T22:24:49Z

To be 100% clear I am not opposed to shipping this as is. I just would feel a lot more comfortable scoping things to only fixing the reported bug (static IP on kernel cmdline). It's a lot simpler to explain from an architectural PoV.

darkmuggle · 2020-03-06T23:23:02Z

Okay, I've repushed this to deal just with the ip=... use case.

The rationale for dropping the holistic approach makes sense. Reducing this to scope of asserted identity to ip= cases make sense; changing the world can come later.

The problem that I have with the use of transient names for OCP is simple: it breaks certificates and makes node identity external to the cluster. One of the selling points of OCP is that the cluster is self-driving, but we make the node identity external to the cluster.

cgwalters · 2020-03-08T21:19:12Z

One of the selling points of OCP is that the cluster is self-driving, but we make the node identity external to the cluster.

Yeah. I've mentioned a few times I want to support tying identity to TPM2 devices, but that isn't universal and will never be. We also have the core problem that the control plane is provisioned via Terraform and we don't have mechanisms yet to "adopt" that into being owned by machineAPI (where applicable). And further, there will always be non-machineAPI platforms (manual bare metal etc.) where there just aren't APIs to control the infrastructure.

So this doesn't seem likely to change really ever, but I am hopeful we can say in some cases we support doing something better.

cgwalters

LGTM apart from one comment.

Did you test this?

dracut/30ignition/persist-ifcfg.sh

darkmuggle · 2020-03-09T22:28:12Z

Okay, re-pushed with handling for all the weird edge-cases.

jlebon · 2020-03-10T14:30:33Z

dracut/30ignition/persist-ifcfg.sh

+    # defining the IP on the commandline.
+    hpath="/sysroot/etc/hostname"
+    hname=$(< /proc/sys/kernel/hostname)
+    for iface in $(ls /sys/class/net)


Need to skip this loop if hpath exists already

cgwalters · 2020-03-10T18:04:51Z

dracut/30ignition/persist-ifcfg.sh

+        if [ "${iface_hostname}" == "${hname}" ] && [ ! -f "${hpath}" ]; then
+            echo "${hname}" > "${hpath}"
+            echo -ne '/etc/hostname/\0' >> /sysroot/etc/selinux/ignition.relabel
+            info "persisting hostname set by kargs for ${iface}"


Suggested change

info "persisting hostname set by kargs for ${iface}"

info "persisting hostname '${hname}' set by kargs for ${iface}"

jlebon

One final suggestion, but LGTM as is too!

jlebon · 2020-03-10T19:05:12Z

dracut/30ignition/persist-ifcfg.sh

+        # Ensure that the hostname used by the kernel is the same one
+        # used by this interface. This guard is intended to protect against
+        # mixed `ip=` or where one is set via dhcp.
+        if [ "${iface_hostname}" == "${hname}" ] && [ ! -f "${hpath}" ]; then


Hmm, would be cleaner to do the [ ! -f "${hpath}" ] check upfront so we can avoid entering the loop entirely, but not a blocker!

Yeah, I was trying to avoid the nested if's, and figured the cost of the loop was trivial. I agree, though.

For RHCOS, if an admin uses the legacy kargs to set the hostname, then we need to persist the name into /sysroot. This change persist the hostname ONLY if the hostname originated from `ip=...<HOSTNAME>`.

cgwalters

LGTM

This is a forward port of 1f8184e (coreos#156). If the admin specified a hostname in the ip= karg static networking config then we'll want to make sure that gets applied to the real root (persistently) as well. Ideally in the future there will be better support for this in NetworkManager itself as the `network-legacy` dracut module did at least provide more support for setting the hostname via ip= kargs than `network-manager` currently does. The discussion about this problem is in [1]. The fix for that will most likely implicate changes to the propagation code introduced here. Fixes: coreos/fedora-coreos-tracker#466 [1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/419

This is a forward port of 1f8184e (coreos#156). If the admin specified a hostname in the ip= karg static networking config and no hostname was specified via Ignition then we'll make sure the hostname from the ip= karg gets applied to the real root (persistently). Ideally in the future there will be better support for this in NetworkManager itself as the `network-legacy` dracut module did at least provide more support for setting the hostname via ip= kargs than `network-manager` currently does. The discussion about this problem is in [1]. The fix for that will most likely implicate changes to the propagation code introduced here. Fixes: coreos/fedora-coreos-tracker#466 [1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/419

darkmuggle requested a review from cgwalters March 6, 2020 20:31

laenion reviewed Mar 6, 2020

View reviewed changes

cgwalters reviewed Mar 6, 2020

View reviewed changes

cgwalters approved these changes Mar 8, 2020

View reviewed changes

dracut/30ignition/persist-ifcfg.sh Outdated Show resolved Hide resolved

jlebon reviewed Mar 9, 2020

View reviewed changes

dracut/30ignition/persist-ifcfg.sh Outdated Show resolved Hide resolved

jlebon reviewed Mar 10, 2020

View reviewed changes

cgwalters approved these changes Mar 10, 2020

View reviewed changes

jlebon approved these changes Mar 10, 2020

View reviewed changes

persist-ifcfg: persist the hostname as well

25a9511

For RHCOS, if an admin uses the legacy kargs to set the hostname, then we need to persist the name into /sysroot. This change persist the hostname ONLY if the hostname originated from `ip=...<HOSTNAME>`.

cgwalters approved these changes Mar 10, 2020

View reviewed changes

darkmuggle merged commit 1f8184e into coreos:spec2x Mar 10, 2020

dustymabe added the spec2x Applies to the ignition spec2x branch label Mar 11, 2020

dustymabe changed the title ~~Persist the first-boot hostname if not otherwise set~~ [spec2x] Persist the first-boot hostname if not otherwise set Mar 11, 2020

cgwalters mentioned this pull request Mar 26, 2020

vsphere ipi: set hostname using vmtoolsd and VM extra config openshift/machine-config-operator#1579

Merged

cgwalters mentioned this pull request Apr 22, 2020

OKD 4.4 Baremetal UPI Install: FCOS does not persist its hostname after reboot. okd-project/okd#153

Closed

cgwalters mentioned this pull request May 4, 2020

Bug 1809345: templates: add etc-networkmanager-dispatcher.d-90-long-hostname.yaml openshift/machine-config-operator#1711

Merged

cgwalters mentioned this pull request Oct 28, 2020

Update hostname when transient hostname is fedora openshift/machine-config-operator#2160

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[spec2x] Persist the first-boot hostname if not otherwise set #156

[spec2x] Persist the first-boot hostname if not otherwise set #156

darkmuggle commented Mar 6, 2020 •

edited

Loading

laenion left a comment

cgwalters commented Mar 6, 2020

cgwalters Mar 6, 2020

darkmuggle Mar 6, 2020

cgwalters Mar 6, 2020 •

edited

Loading

darkmuggle Mar 6, 2020

cgwalters Mar 6, 2020

cgwalters commented Mar 6, 2020 •

edited

Loading

dcbw commented Mar 6, 2020 •

edited

Loading

dcbw commented Mar 6, 2020

cgwalters commented Mar 6, 2020

darkmuggle commented Mar 6, 2020

cgwalters commented Mar 8, 2020

cgwalters left a comment

darkmuggle commented Mar 9, 2020

jlebon Mar 10, 2020

darkmuggle Mar 10, 2020

cgwalters Mar 10, 2020

jlebon left a comment

jlebon Mar 10, 2020

darkmuggle Mar 10, 2020

cgwalters left a comment

	info "persisting hostname set by kargs for ${iface}"
	info "persisting hostname '${hname}' set by kargs for ${iface}"

[spec2x] Persist the first-boot hostname if not otherwise set #156

[spec2x] Persist the first-boot hostname if not otherwise set #156

Conversation

darkmuggle commented Mar 6, 2020 • edited Loading

laenion left a comment

Choose a reason for hiding this comment

cgwalters commented Mar 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgwalters Mar 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgwalters commented Mar 6, 2020 • edited Loading

dcbw commented Mar 6, 2020 • edited Loading

dcbw commented Mar 6, 2020

cgwalters commented Mar 6, 2020

darkmuggle commented Mar 6, 2020

cgwalters commented Mar 8, 2020

cgwalters left a comment

Choose a reason for hiding this comment

darkmuggle commented Mar 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlebon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgwalters left a comment

Choose a reason for hiding this comment

darkmuggle commented Mar 6, 2020 •

edited

Loading

cgwalters Mar 6, 2020 •

edited

Loading

cgwalters commented Mar 6, 2020 •

edited

Loading

dcbw commented Mar 6, 2020 •

edited

Loading