kubevirt: Run afterburn-hostname service #1005

qinqon · 2023-10-23T12:51:34Z

The KubeVirt platform is already supported by afterburn to set the hostname, also there is an official fedora coreos container disk to run it. This change include it in the list of platforms allowed to run afterburn hostname feature at startup.

Close https://issues.redhat.com/browse/OCPBUGS-22259

travier · 2023-10-23T13:07:21Z

Can you also update the docs?
Can you also link where the hostname support has been added?

travier · 2023-10-23T13:08:21Z

Looks like everything was added in #725

jlebon · 2023-10-25T20:53:44Z

Won't this override the hostname if DHCP did provide it? Are they always guaranteed to be the same? If not, should the one in the metadata win?

qinqon · 2023-10-26T05:41:03Z

Won't this override the hostname if DHCP did provide it? Are they always guaranteed to be the same? If not, should the one in the metadata win?

Have to verify it, but my idea is that afterburn comes before ignition, so if ignition goes after and activate network interfaces DHCP will override the hostname.

jlebon · 2023-10-26T20:57:26Z

Won't this override the hostname if DHCP did provide it? Are they always guaranteed to be the same? If not, should the one in the metadata win?

Have to verify it, but my idea is that afterburn comes before ignition, so if ignition goes after and activate network interfaces DHCP will override the hostname.

afterburn-hostname.service runs after networking is up and before the Ignition files stage. The logic is that users can always override the auto-detected hostname via Ignition.

As is, with this PR the hostname from afterburn-hostname.service will take precedence over the DHCP-provided one. That may be the right thing to do, but I just want to make sure in case that wasn't considered.

qinqon · 2023-10-27T06:26:23Z

Won't this override the hostname if DHCP did provide it? Are they always guaranteed to be the same? If not, should the one in the metadata win?

Have to verify it, but my idea is that afterburn comes before ignition, so if ignition goes after and activate network interfaces DHCP will override the hostname.

afterburn-hostname.service runs after networking is up and before the Ignition files stage. The logic is that users can always override the auto-detected hostname via Ignition.

As is, with this PR the hostname from afterburn-hostname.service will take precedence over the DHCP-provided one. That may be the right thing to do, but I just want to make sure in case that wasn't considered.

Will post results here, I think we have something at ignition that triggers DHCP again.

qinqon · 2023-10-27T11:38:07Z

Won't this override the hostname if DHCP did provide it? Are they always guaranteed to be the same? If not, should the one in the metadata win?

Have to verify it, but my idea is that afterburn comes before ignition, so if ignition goes after and activate network interfaces DHCP will override the hostname.

afterburn-hostname.service runs after networking is up and before the Ignition files stage. The logic is that users can always override the auto-detected hostname via Ignition.

As is, with this PR the hostname from afterburn-hostname.service will take precedence over the DHCP-provided one. That may be the right thing to do, but I just want to make sure in case that wasn't considered.

You right, after attaching a dnsmasq to the interface with special hostname I see it's serving it

dnsmasq-dhcp: DHCPDISCOVER(br0) 92:32:d3:7a:da:b5 
dnsmasq-dhcp: DHCPOFFER(br0) 192.168.67.11 92:32:d3:7a:da:b5 
dnsmasq-dhcp: DHCPREQUEST(br0) 192.168.67.11 92:32:d3:7a:da:b5 
dnsmasq-dhcp: DHCPACK(br0) 192.168.67.11 92:32:d3:7a:da:b5 worker1-dnsmasq

But hostnamectl shows that we have the old name

$ hostnamectl status
     Static hostname: worker1
           Icon name: computer-vm
             Chassis: vm 🖴
          Machine ID: 8915d4e1a7b44981a77fec7f6455db69
             Boot ID: 7ad1e933e1ce41b8ba9d954c0612589d
      Virtualization: kvm
    Operating System: Fedora CoreOS 38.20231027.dev.1 
         CPE OS Name: cpe:/o:fedoraproject:fedora:38
      OS Support End: Tue 2024-05-14
OS Support Remaining: 6month 2w 2d
              Kernel: Linux 6.5.8-200.fc38.x86_64
        Architecture: x86-64
     Hardware Vendor: KubeVirt
      Hardware Model: None
    Firmware Version: 1.16.1-1.el9
       Firmware Date: Tue 2014-04-01

qinqon · 2023-10-27T11:55:07Z

@jlebon maybe for kubevirt we can set the "transient" hostname so it can be overriden by DHCP

qinqon · 2023-10-27T12:43:54Z

I have test the following

ExecStart=/usr/bin/afterburn --cmdline --hostname=/tmp/hostname                 
ExecStart=/sysroot/usr/bin/hostname -F /tmp/hostname

And it works as expected and the hostname is set if no DHCP hostname is received but overriden if DHCP hostname option is arrives.

But I don't know how to make this conditional to kubevirt.

We can see hostname is transient

Static hostname: (unset)                         
Transient hostname: worker1

UPDATE: An option would be to create a new afterburn-transient-hostname that do that for specific platform in this case kubevirt.

dustymabe · 2023-10-27T14:43:21Z

@jlebon
As is, with this PR the hostname from afterburn-hostname.service will take precedence over the DHCP-provided one. That may be the right thing to do, but I just want to make sure in case that wasn't considered.

I kind of think this should be the behavior. What is the behavior on other platforms?

DHCP can be unreliable, i.e. if there are multiple networks and all of them provide a hostname via DHCP, which one wins? I think the way NM works today is to choose the one with the default route, but if the hostname is set in the metadata for an instance I think we should just use that.

jlebon

As is, with this PR the hostname from afterburn-hostname.service will take precedence over the DHCP-provided one. That may be the right thing to do, but I just want to make sure in case that wasn't considered.

I kind of think this should be the behavior. What is the behavior on other platforms?

I'm not sure if any of the other providers in that unit would allow you to customize DHCP to that extent. But if any did, the metadata would clearly win since the unit would run. There are some providers that do provide metadata for the hostname that are not listed in the unit however: PowerVS, OpenStack, and Packet. I'm not sure if their omission from the unit is an oversight or on purpose.

DHCP can be unreliable, i.e. if there are multiple networks and all of them provide a hostname via DHCP, which one wins? I think the way NM works today is to choose the one with the default route, but if the hostname is set in the metadata for an instance I think we should just use that.

That sounds reasonable to me. My intuition is also that the metadata should win, but what made me hesitate is that because we shipped KubeVirt support without this from day 1, the status quo currently is that we're relying on DHCP hostnames. So this can be a noticeable behaviour change to users currently relying on the node defaulting to the DHCP hostname (if it differs from the metadata). And maybe that's OK but it's worth flagging.

@qinqon Can you add a release note item for this?

jlebon · 2023-10-27T15:52:25Z

dracut/30afterburn/afterburn-hostname.service

@@ -13,6 +13,7 @@ ConditionKernelCommandLine=|ignition.platform.id=hetzner
 ConditionKernelCommandLine=|ignition.platform.id=ibmcloud
 ConditionKernelCommandLine=|ignition.platform.id=scaleway
 ConditionKernelCommandLine=|ignition.platform.id=vultr
+ConditionKernelCommandLine=|ignition.platform.id=kubevirt


Note the list is in alphabetical order. Can you insert it in the right spot?

qinqon · 2023-10-30T07:10:45Z

As is, with this PR the hostname from afterburn-hostname.service will take precedence over the DHCP-provided one. That may be the right thing to do, but I just want to make sure in case that wasn't considered.

I kind of think this should be the behavior. What is the behavior on other platforms?

I'm not sure if any of the other providers in that unit would allow you to customize DHCP to that extent. But if any did, the metadata would clearly win since the unit would run. There are some providers that do provide metadata for the hostname that are not listed in the unit however: PowerVS, OpenStack, and Packet. I'm not sure if their omission from the unit is an oversight or on purpose.

DHCP can be unreliable, i.e. if there are multiple networks and all of them provide a hostname via DHCP, which one wins? I think the way NM works today is to choose the one with the default route, but if the hostname is set in the metadata for an instance I think we should just use that.

That sounds reasonable to me. My intuition is also that the metadata should win, but what made me hesitate is that because we shipped KubeVirt support without this from day 1, the status quo currently is that we're relying on DHCP hostnames. So this can be a noticeable behaviour change to users currently relying on the node defaulting to the DHCP hostname (if it differs from the metadata). And maybe that's OK but it's worth flagging.

I think there are not much users of the fcos/rhcos kubevirt provider, up until recently we were using openstack provider since kubevirt provide didn't exist there, so I doubt we would break a lot of installations if we change the status quos here.

Also if we honor the hostname metadata and we add knob at kubevirt so is not set on creation per VM then we can keep the old behaviour, we have to check with cloud-init too, since KubeVirt supports that.

@qinqon Can you add a release note item for this?

Sure.

qinqon · 2023-10-30T15:10:59Z

Have check what cloud-init is doing for rhel related to hostname metadata value and looks like they have an extra knob to make it static or transient if it's handled by systemd and we can use hostnamectl

https://github.com/number5/cloud-init/blob/76972b4a35681d2adbdb2bc0f95a3611eab146fb/cloudinit/distros/rhel.py#L110-L123

Maybe we can do similar at afterburn and read this extra knob.

jlebon

OK, good to know breakage isn't much of a concern.

Have check what cloud-init is doing for rhel related to hostname metadata value and looks like they have an extra knob to make it static or transient if it's handled by systemd and we can use hostnamectl

number5/cloud-init@76972b4/cloudinit/distros/rhel.py#L110-L123

Maybe we can do similar at afterburn and read this extra knob.

This is configured via a cloud-init config file I suppose? I don't think we want to have a dep on that here. And of course, it won't be configurable via Ignition since we run before those files are written. :) But also, currently afterburn-hostname.service runs just on first boot and transient hostnames don't survive a reboot.

I'm also unsure if there's a legitimate use case for doing this. Note that the hostname can always be overridden via Ignition. And having consistency cross-platform is valuable.

Let's stick with this and we can always tweak things in the future if a compelling use case shows up.

LGTM overall, just one tweak!

docs/release-notes.md

qinqon · 2023-10-31T08:15:32Z

@jlebon Can we put this on holdm ? We are going to discuss about it today at team's sync. Maybe we prefer different approach at MCO openshift/machine-config-operator#4005

Looks like there are other providers that has similar stuff under MCO

qinqon · 2023-10-31T08:15:40Z

/hold

qinqon · 2023-10-31T08:17:27Z

OK, good to know breakage isn't much of a concern.

Have check what cloud-init is doing for rhel related to hostname metadata value and looks like they have an extra knob to make it static or transient if it's handled by systemd and we can use hostnamectl
number5/cloud-init@76972b4/cloudinit/distros/rhel.py#L110-L123
Maybe we can do similar at afterburn and read this extra knob.

This is configured via a cloud-init config file I suppose? I don't think we want to have a dep on that here. And of course, it won't be configurable via Ignition since we run before those files are written. :) But also, currently afterburn-hostname.service runs just on first boot and transient hostnames don't survive a reboot.

I'm also unsure if there's a legitimate use case for doing this. Note that the hostname can always be overridden via Ignition. And having consistency cross-platform is valuable.

Let's stick with this and we can always tweak things in the future if a compelling use case shows up.

LGTM overall, just one tweak!

Ahh right, we can create the persistent hostname and then at ignition convert it to transient before NetworkManager, but at least we have a hostname set.

jlebon · 2023-11-03T16:35:52Z

We need to make sure we don't break HyperShift + KubeVirt before merging this (see discussions in openshift/machine-config-operator#4005).

qinqon · 2023-11-14T15:50:58Z

/hold cancel

@jlebon @dustymabe all good from hypershift to merge this and go with the solution we talk about

The KubeVirt platform is already supported by afterburn to set the hostname, also there is an official fedora coreos container disk to run it. This change include it in the list of platforms allowed to run afterburn hostname feature at startup. Signed-off-by: Enrique Llorente <[email protected]>

dustymabe

LGTM

jlebon reviewed Oct 27, 2023

View reviewed changes

qinqon force-pushed the kubevirt-run-afterburn-hostname-service branch from e7470fb to 68ab211 Compare October 30, 2023 07:13

jlebon approved these changes Oct 30, 2023

View reviewed changes

docs/release-notes.md Outdated Show resolved Hide resolved

jlebon mentioned this pull request Nov 2, 2023

kubevirt: Set transient hostname from metadata openshift/machine-config-operator#4005

Closed

jlebon added the status/on-hold label Nov 3, 2023

qinqon force-pushed the kubevirt-run-afterburn-hostname-service branch from e16fcb1 to 8783c39 Compare November 14, 2023 15:53

dustymabe approved these changes Nov 14, 2023

View reviewed changes

jlebon approved these changes Nov 14, 2023

View reviewed changes

jlebon enabled auto-merge November 14, 2023 16:15

jlebon removed the status/on-hold label Nov 14, 2023

jlebon merged commit b6cda33 into coreos:main Nov 14, 2023
10 checks passed

qinqon mentioned this pull request Nov 15, 2023

HOSTEDCP-1311: kubevirt, Add support for secondary networks openshift/hypershift#3066

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubevirt: Run afterburn-hostname service #1005

kubevirt: Run afterburn-hostname service #1005

qinqon commented Oct 23, 2023 •

edited

Loading

travier commented Oct 23, 2023

travier commented Oct 23, 2023

jlebon commented Oct 25, 2023

qinqon commented Oct 26, 2023

jlebon commented Oct 26, 2023

qinqon commented Oct 27, 2023

qinqon commented Oct 27, 2023

qinqon commented Oct 27, 2023

qinqon commented Oct 27, 2023 •

edited

Loading

dustymabe commented Oct 27, 2023

jlebon left a comment

jlebon Oct 27, 2023

qinqon commented Oct 30, 2023

qinqon commented Oct 30, 2023 •

edited

Loading

jlebon left a comment

qinqon commented Oct 31, 2023 •

edited

Loading

qinqon commented Oct 31, 2023

qinqon commented Oct 31, 2023

jlebon commented Nov 3, 2023

qinqon commented Nov 14, 2023

dustymabe left a comment

kubevirt: Run afterburn-hostname service #1005

kubevirt: Run afterburn-hostname service #1005

Conversation

qinqon commented Oct 23, 2023 • edited Loading

travier commented Oct 23, 2023

travier commented Oct 23, 2023

jlebon commented Oct 25, 2023

qinqon commented Oct 26, 2023

jlebon commented Oct 26, 2023

qinqon commented Oct 27, 2023

qinqon commented Oct 27, 2023

qinqon commented Oct 27, 2023

qinqon commented Oct 27, 2023 • edited Loading

dustymabe commented Oct 27, 2023

jlebon left a comment

Choose a reason for hiding this comment

jlebon Oct 27, 2023

Choose a reason for hiding this comment

qinqon commented Oct 30, 2023

qinqon commented Oct 30, 2023 • edited Loading

jlebon left a comment

Choose a reason for hiding this comment

qinqon commented Oct 31, 2023 • edited Loading

qinqon commented Oct 31, 2023

qinqon commented Oct 31, 2023

jlebon commented Nov 3, 2023

qinqon commented Nov 14, 2023

dustymabe left a comment

Choose a reason for hiding this comment

qinqon commented Oct 23, 2023 •

edited

Loading

qinqon commented Oct 27, 2023 •

edited

Loading

qinqon commented Oct 30, 2023 •

edited

Loading

qinqon commented Oct 31, 2023 •

edited

Loading