Determine Nic name in Bootstrap VM for ARM and x86 #5698

jeffdyoung · 2022-03-11T15:39:17Z

I dug into why arm/x86 were creating nic names:

By default libvirt the bootstrap x86 vm that gets created uses pci:

<controller type='pci' index='0' model='pci-root'>
<interface type='bridge'>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
 </interface>
<interface type='bridge'>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
 </interface>

By default libvirt the bootstrap arm64 vm that gets created uses pcie:

<controller type='pci' index='0' model='pcie-root'>
<interface type='bridge'>
     <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
</interface>
<interface type='bridge'>
     <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
</interface>

This causes RHCOS (and RHEL/Fedora) to rename eth0 and eth1 differently on startup:
x86:

# dmesg -t | grep eth
virtio_net virtio1 ens4: renamed from eth1
virtio_net virtio0 ens3: renamed from eth0

arm64:

# dmesg -t | grep eth
virtio_net virtio1 enp2s0: renamed from eth1
virtio_net virtio0 enp1s0: renamed from eth0

More on RHEL naming here.

According to this RHEL DOC using eth0/eth1 should be reliable as long as we continue to use virtio-net, and not mix or use with other nic types (like e1000).

This proposed fix seems to be the least painful way around this issue. I looked at trying to pass in mac address, and adding net.ifnames=0 as a kernel parameter, but didn't see a straight forward way to do so.

Let me know your thoughts, we've targeted IPI Metal support for 4.11. If you agree, I can get this PR cleaned up (the linters won't run on my arm dev machine).

Multi-Arch Ticket: https://issues.redhat.com/browse/ARMOCP-234
Similar PR: #5554

cc: @bn222

bn222

I only had 1 nit (see my comment). Otherwise, this lgtm.

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

ardaguclu · 2022-03-15T11:27:35Z

/uncc

bn222 · 2022-03-15T11:29:23Z

This looks good from my side

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

hardys · 2022-03-16T12:05:39Z

Thanks for the detailed analysis @jeffdyoung!

The log-parsing aspect of this solution seems a little ...ugly, so before committing to this it'd be good to ensure we've fully explored the alternatives:

I looked at trying to pass in mac address, and adding net.ifnames=0 as a kernel parameter, but didn't see a straight forward way to do so.

Yeah I don't see an obvious way to pass in the kernel param unless we modify the qcow image (there's a cmdline option to terraform/libvirt but I guess that only works for direct kernel boot)

However passing the MAC should be pretty simple I think, we already have platform parameters for ExternalMACAddress and ProvisioningMACAddress - we'd just need to calculate a default for those values here , then pass ProvisioningMACAddress in the templateData so the startironic.sh script can look up the NIC by MAC instead of name?

hardys · 2022-03-16T12:08:46Z

/label platform/baremetal
/cc @sadasu

jeffdyoung · 2022-03-16T15:48:24Z

Hey @hardys Happy to use mac addresses. Are you good with hard-coding the default macs (say...External=52:54:00:01:02:03 and Provisioning=52:54:00:0A:0B:0C), and documenting what the defaults are here and how to override?

pkg/types/baremetal/defaults/platform.go

pkg/asset/ignition/bootstrap/baremetal/template.go

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

hardys · 2022-03-31T10:48:16Z

Hey @hardys Happy to use mac addresses. Are you good with hard-coding the default macs (say...External=52:54:00:01:02:03 and Provisioning=52:54:00:0A:0B:0C), and documenting what the defaults are here and how to override?

Sorry just catching up - I was thinking we'd generate the default MACs with some random component (but using the libvirt prefix) - e.g similar to this in the libvirt terraform provider

Then we remove the risk that someone tries to run two IPI installs on the same L2 segment and runs into issues - this could be quite likely in developer multi-cluster scenarios, and also there is Hive support for BM IPI where multiple bootstrap VMs run on a common hypervisor

jeffdyoung · 2022-03-31T21:56:08Z

Hey @hardys Happy to use mac addresses. Are you good with hard-coding the default macs (say...External=52:54:00:01:02:03 and Provisioning=52:54:00:0A:0B:0C), and documenting what the defaults are here and how to override?

Sorry just catching up - I was thinking we'd generate the default MACs with some random component (but using the libvirt prefix) - e.g similar to this in the libvirt terraform provider

Then we remove the risk that someone tries to run two IPI installs on the same L2 segment and runs into issues - this could be quite likely in developer multi-cluster scenarios, and also there is Hive support for BM IPI where multiple bootstrap VMs run on a common hypervisor

No worries @hardys, was able to use the code you linked with some modifications. I ran a few installs with it on our arm machines and verified that random macs addresses were generated on every bootstrap vm. Let me know if you'd like more changes.

jeffdyoung · 2022-04-06T20:22:28Z

/retest

jeffdyoung · 2022-04-07T22:17:29Z

/retest

jeffdyoung · 2022-04-08T12:23:19Z

/retest

hardys · 2022-04-08T15:05:57Z

/approve

The latest version lgtm, thanks!

openshift-ci · 2022-04-08T15:06:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hardys

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~data/data/bootstrap/baremetal/OWNERS~~ [hardys]
~~pkg/asset/ignition/bootstrap/baremetal/OWNERS~~ [hardys]
~~pkg/types/baremetal/OWNERS~~ [hardys]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2022-04-21T23:14:13Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-21T23:26:17Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-22T00:02:13Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-22T03:38:13Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-22T04:02:16Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-22T04:38:13Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-22T11:39:14Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

jeffdyoung · 2022-04-22T12:10:39Z

@zaneb It doesn't look like prow did the right thing, but I think we're good now?

openshift-bot · 2022-04-22T12:43:16Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-22T13:22:16Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-22T14:27:15Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-22T14:40:17Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-22T15:19:14Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-22T15:45:15Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

zaneb · 2022-04-22T17:12:19Z

/override ci/prow/e2e-metal-ipi-ovn-ipv6-required
Job configs have changed and this job no longer exists. ci/prow/e2e-metal-ipi-ovn-ipv6 passed.

openshift-ci · 2022-04-22T17:15:38Z

@zaneb: zaneb unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file.

Details

In response to this:

/override ci/prow/e2e-metal-ipi-ovn-ipv6-required
Job configs have changed and this job no longer exists. ci/prow/e2e-metal-ipi-ovn-ipv6 passed.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

zaneb · 2022-04-22T17:32:19Z

Oh, heh, forgot what repo I am in. @patrickdillon could you override the deleted job?

patrickdillon · 2022-04-22T17:48:15Z

/override ci/prow/e2e-metal-ipi-ovn-ipv6-required

openshift-ci · 2022-04-22T17:48:56Z

@patrickdillon: Overrode contexts on behalf of patrickdillon: ci/prow/e2e-metal-ipi-ovn-ipv6-required

Details

In response to this:

/override ci/prow/e2e-metal-ipi-ovn-ipv6-required

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

patrickdillon · 2022-04-22T23:30:27Z

/override ci/prow/e2e-metal-ipi-ovn-ipv6-required

openshift-ci · 2022-04-22T23:31:42Z

@patrickdillon: Overrode contexts on behalf of patrickdillon: ci/prow/e2e-metal-ipi-ovn-ipv6-required

Details

In response to this:

/override ci/prow/e2e-metal-ipi-ovn-ipv6-required

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

patrickdillon · 2022-04-23T00:14:51Z

/override ci/prow/e2e-metal-ipi-ovn-ipv6-required

openshift-ci · 2022-04-23T00:15:31Z

@patrickdillon: Overrode contexts on behalf of patrickdillon: ci/prow/e2e-metal-ipi-ovn-ipv6-required

Details

In response to this:

/override ci/prow/e2e-metal-ipi-ovn-ipv6-required

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

patrickdillon · 2022-04-23T00:16:28Z

/retest

openshift-ci · 2022-04-23T04:42:09Z

@jeffdyoung: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-e2e-aws	9ed64ce231b5bcd35f107aa22bf20cadd80b15b1	link	false	`/test okd-e2e-aws`
ci/prow/e2e-libvirt	9ed64ce231b5bcd35f107aa22bf20cadd80b15b1	link	false	`/test e2e-libvirt`
ci/prow/e2e-alibaba	9ed64ce231b5bcd35f107aa22bf20cadd80b15b1	link	false	`/test e2e-alibaba`
ci/prow/e2e-azurestack	9ed64ce231b5bcd35f107aa22bf20cadd80b15b1	link	false	`/test e2e-azurestack`
ci/prow/e2e-crc	9ed64ce231b5bcd35f107aa22bf20cadd80b15b1	link	false	`/test e2e-crc`
ci/prow/e2e-gcp-shared-vpc	9ed64ce231b5bcd35f107aa22bf20cadd80b15b1	link	false	`/test e2e-gcp-shared-vpc`
ci/prow/e2e-gcp-upi-xpn	9ed64ce231b5bcd35f107aa22bf20cadd80b15b1	link	false	`/test e2e-gcp-upi-xpn`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-bot · 2022-04-23T04:46:46Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-23T05:14:17Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci bot requested review from andfasano and ardaguclu March 11, 2022 15:40

bn222 reviewed Mar 11, 2022

View reviewed changes

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template Outdated Show resolved Hide resolved

jeffdyoung commented Mar 11, 2022

View reviewed changes

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template Outdated Show resolved Hide resolved

openshift-ci bot removed the request for review from ardaguclu March 15, 2022 11:27

bn222 mentioned this pull request Mar 15, 2022

Add ARM support openshift-metal3/dev-scripts#1350

Merged

elfosardo suggested changes Mar 16, 2022

View reviewed changes

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template Outdated Show resolved Hide resolved

openshift-ci bot requested a review from sadasu March 16, 2022 12:08

openshift-ci bot added the platform/baremetal IPI bare metal hosts platform label Mar 16, 2022

bn222 mentioned this pull request Mar 21, 2022

Add a way to override the interface used by bootstrap node #5554

Closed

zaneb reviewed Mar 22, 2022

View reviewed changes

jeffdyoung force-pushed the armipi branch from 90bbfa1 to f7bf274 Compare March 23, 2022 03:06

jeffdyoung force-pushed the armipi branch from 511edbc to fc85caf Compare March 31, 2022 21:44

jeffdyoung force-pushed the armipi branch 2 times, most recently from c671422 to 474c8d6 Compare April 7, 2022 14:55

Determine PROVISIONING_NIC base on mac address.

c7f162f

jeffdyoung force-pushed the armipi branch from 7cc3166 to c7f162f Compare April 7, 2022 17:16

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 8, 2022

openshift-merge-robot merged commit abac9a1 into openshift:master Apr 23, 2022

jeffdyoung deleted the armipi branch May 4, 2022 13:44

hardys mentioned this pull request May 9, 2022

baremetal: static IP for bootstrap node #5787

Merged

Determine Nic name in Bootstrap VM for ARM and x86 #5698

Determine Nic name in Bootstrap VM for ARM and x86 #5698

Uh oh!

Conversation

jeffdyoung commented Mar 11, 2022

Uh oh!

bn222 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ardaguclu commented Mar 15, 2022

Uh oh!

bn222 commented Mar 15, 2022

Uh oh!

Uh oh!

hardys commented Mar 16, 2022

Uh oh!

hardys commented Mar 16, 2022

Uh oh!

jeffdyoung commented Mar 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hardys commented Mar 31, 2022

Uh oh!

jeffdyoung commented Mar 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeffdyoung commented Apr 6, 2022

Uh oh!

jeffdyoung commented Apr 7, 2022

Uh oh!

jeffdyoung commented Apr 8, 2022

Uh oh!

hardys commented Apr 8, 2022

Uh oh!

openshift-ci bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 21, 2022

Uh oh!

openshift-bot commented Apr 21, 2022

Uh oh!

openshift-bot commented Apr 22, 2022

Uh oh!

openshift-bot commented Apr 22, 2022

Uh oh!

openshift-bot commented Apr 22, 2022

Uh oh!

openshift-bot commented Apr 22, 2022

Uh oh!

openshift-bot commented Apr 22, 2022

Uh oh!

jeffdyoung commented Apr 22, 2022

Uh oh!

openshift-bot commented Apr 22, 2022

Uh oh!

openshift-bot commented Apr 22, 2022

Uh oh!

openshift-bot commented Apr 22, 2022

Uh oh!

openshift-bot commented Apr 22, 2022

Uh oh!

openshift-bot commented Apr 22, 2022

Uh oh!

openshift-bot commented Apr 22, 2022

Uh oh!

zaneb commented Apr 22, 2022

Uh oh!

openshift-ci bot commented Apr 22, 2022

Uh oh!

zaneb commented Apr 22, 2022

Uh oh!

patrickdillon commented Apr 22, 2022

Uh oh!

openshift-ci bot commented Apr 22, 2022

Uh oh!

jeffdyoung commented Mar 16, 2022 •

edited

Loading

jeffdyoung commented Mar 31, 2022 •

edited

Loading

openshift-ci bot commented Apr 23, 2022 •

edited

Loading