From 96d351c8ffe2a46c9ab8d0415a8f8eff9002927f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:25:14 -0300 Subject: [PATCH 1/4] refactor(template): share the install device resolution The order that names the install device is noderes.installnic, then noderes.primarynic, then mac.mac. gen_net_boot_params owns that order for the netboot kernel parameters. Only the Ubuntu template reused it. Every other install template reads mac.mac on its own. Move the reuse into install_device_params so that any install template can share it. subiquity_install_netcfg keeps its own name and its own return value, because netplan needs the pair of a name and an address. Behaviour does not change. --- xCAT-server/lib/perl/xCAT/Template.pm | 35 ++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/xCAT-server/lib/perl/xCAT/Template.pm b/xCAT-server/lib/perl/xCAT/Template.pm index d02048c88..e2d077e4c 100644 --- a/xCAT-server/lib/perl/xCAT/Template.pm +++ b/xCAT-server/lib/perl/xCAT/Template.pm @@ -1664,19 +1664,47 @@ sub crydb_or_locked #-------------------------------------------------------------------------------- -=head3 subiquity_install_netcfg +=head3 install_device_params Resolve the interface the INSTALLED system must bring up, in xCAT's own order: noderes.installnic, else noderes.primarynic, else match on mac.mac. Either attribute may name an interface OR carry a MAC address. xCAT::NetworkUtils::gen_net_boot_params already owns that order for the netboot kernel parameters, so it is reused here rather than re-derived -- and in - particular the install template never re-derives any part of it in shell. + particular an install template never re-derives any part of it in shell. + + Every install template that has to name the install device shares this resolution, so the + device an installer configures cannot disagree with the device the netboot kernel parameters + name. The resolution carries no boot loader dependency of any kind. Arguments: $installnic - noderes.installnic (may be undef or empty) $primarynic - noderes.primarynic (may be undef or empty) $macentry - the raw mac.mac entry (may hold |-separated, !hostname-suffixed entries) $nodename - the node the entry is resolved for + Returns: + the hash reference from gen_net_boot_params. Of interest to install templates: + nicname - the interface name, set only when installnic/primarynic names an interface + mac - the address, set whenever one is known + +=cut + +#-------------------------------------------------------------------------------- +sub install_device_params { + my ($installnic, $primarynic, $macentry, $nodename) = @_; + + my $macmac = xCAT::Utils->parseMacTabEntry(defined($macentry) ? $macentry : '', $nodename); + return xCAT::NetworkUtils->gen_net_boot_params($installnic, $primarynic, $macmac); +} + +#-------------------------------------------------------------------------------- + +=head3 subiquity_install_netcfg + + Shape install_device_params for netplan, which matches a device by MAC and optionally renames + it. + + Arguments: + as install_device_params Returns: ($setname, $macaddress) $setname - the name netplan must rename the matched device to, empty when the device is @@ -1689,8 +1717,7 @@ sub crydb_or_locked sub subiquity_install_netcfg { my ($installnic, $primarynic, $macentry, $nodename) = @_; - my $macmac = xCAT::Utils->parseMacTabEntry(defined($macentry) ? $macentry : '', $nodename); - my $params = xCAT::NetworkUtils->gen_net_boot_params($installnic, $primarynic, $macmac); + my $params = install_device_params($installnic, $primarynic, $macentry, $nodename); my $setname = defined($params->{nicname}) ? $params->{nicname} : ''; my $macaddress = defined($params->{mac}) ? lc($params->{mac}) : ''; From 10d1d92d79ed9e4b044584640b969919e99c7888 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:25:58 -0300 Subject: [PATCH 2/4] fix(template): honour installnic when the kickstart names the install device noderes.installnic names the adapter that deploys the operating system. The kickstart network line ignored it and named the adapter from mac.mac alone, so a node that sets installnic got a kernel command line that obeys installnic and a kickstart that configures a different adapter. On a node with more than one adapter the installer then brings up the wrong one and cannot reach the repository. Name the device from the shared resolution, which gives the interface name when installnic or primarynic names one, and the address otherwise. A node that sets neither attribute keeps the address it has today. The unique local address still comes from the hardware address, because autoulaaddress builds the address from it. --- xCAT-server/lib/perl/xCAT/Template.pm | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/xCAT-server/lib/perl/xCAT/Template.pm b/xCAT-server/lib/perl/xCAT/Template.pm index e2d077e4c..8eba50a8f 100644 --- a/xCAT-server/lib/perl/xCAT/Template.pm +++ b/xCAT-server/lib/perl/xCAT/Template.pm @@ -1052,13 +1052,23 @@ sub kickstartnetwork { unless ($mactab) { $tmplerr = "mac table should always exist prior to template processing when doing autoula"; return; } my $ent = $mactab->getNodeAttribs($node, ['mac'], prefetchcache => 1); unless ($ent and $ent->{mac}) { $tmplerr = "missing mac data for $node"; return; } - my $suffix = xCAT::Utils->parseMacTabEntry($ent->{mac}, $node); - $suffix = lc($suffix); + my ($installnic, $primarynic); + my $nrtab = xCAT::Table->new('noderes', -create => 0); + if ($nrtab) { + my $nrent = $nrtab->getNodeAttribs($node, [ 'installnic', 'primarynic' ], prefetchcache => 1); + if ($nrent) { + $installnic = $nrent->{installnic}; + $primarynic = $nrent->{primarynic}; + } + } + my $params = install_device_params($installnic, $primarynic, $ent->{mac}, $node); + my $macaddr = defined($params->{mac}) ? lc($params->{mac}) : ''; + my $suffix = defined($params->{nicname}) ? $params->{nicname} : $macaddr; if ($::XCATSITEVALS{managedaddressmode} eq "autoula") { unless ($hoststab) { $hoststab = xCAT::Table->new('hosts', -create => 1); } $line .= "static --device=$suffix --noipv4 --ipv6="; - my $ulaaddr = autoulaaddress($suffix); + my $ulaaddr = autoulaaddress($macaddr); $hoststab->setNodeAttribs($node, { ip => $ulaaddr }); $line .= $ulaaddr; } elsif ($::XCATSITEVALS{managedaddressmode} =~ /static/) { From 286f0472a462bfb2adac36b03df72ada8775b460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:28:08 -0300 Subject: [PATCH 3/4] test(template): pin the shared install device resolution Cover the order that names the install device: installnic, then primarynic, then mac.mac. Either attribute may name an interface or carry an address, and the keyword mac returns to mac.mac. Cover the device the kickstart names for each of those inputs. A node that sets neither attribute keeps the address it has today. Cover the defect the change closes, where a mac.mac entry that holds several untagged addresses resolves to the last of them. Cover that Ubuntu keeps its own pair of a name and an address over the same resolution, and that the unique local address still comes from the hardware address. --- xCAT-test/unit/template_install_device.t | 126 +++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 xCAT-test/unit/template_install_device.t diff --git a/xCAT-test/unit/template_install_device.t b/xCAT-test/unit/template_install_device.t new file mode 100644 index 000000000..440a28b44 --- /dev/null +++ b/xCAT-test/unit/template_install_device.t @@ -0,0 +1,126 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use File::Spec; +use FindBin; +use Test::More; + +my $root = File::Spec->catdir( $FindBin::Bin, '..', '..' ); +BEGIN { + push @INC, + File::Spec->catdir( $FindBin::Bin, '..', '..', 'perl-xCAT' ), + File::Spec->catdir( $FindBin::Bin, '..', '..', 'xCAT-server', 'lib', 'perl' ); +} + +eval { require xCAT::Template; 1 } + or plan skip_all => "xCAT::Template not loadable: $@"; + +my $NODE = 'n1'; +my $MAC1 = 'AA:BB:CC:DD:EE:01'; +my $MAC2 = 'AA:BB:CC:DD:EE:02'; + +# The order is installnic, then primarynic, then mac.mac. Either attribute may +# name an interface or carry an address. +my $p = xCAT::Template::install_device_params( '', '', $MAC1, $NODE ); +is( $p->{mac}, $MAC1, 'with neither attribute set the address comes from mac.mac' ); +is( $p->{nicname}, undef, 'with neither attribute set no interface is named' ); + +$p = xCAT::Template::install_device_params( 'eth1', '', $MAC1, $NODE ); +is( $p->{nicname}, 'eth1', 'installnic naming an interface wins' ); +is( $p->{mac}, $MAC1, 'the address is still resolved alongside the interface' ); + +$p = xCAT::Template::install_device_params( '', 'eth2', $MAC1, $NODE ); +is( $p->{nicname}, 'eth2', 'primarynic is used when installnic is not set' ); + +$p = xCAT::Template::install_device_params( 'eth1', 'eth2', $MAC1, $NODE ); +is( $p->{nicname}, 'eth1', 'installnic beats primarynic' ); + +$p = xCAT::Template::install_device_params( $MAC2, '', $MAC1, $NODE ); +is( $p->{mac}, $MAC2, 'installnic carrying an address names that address' ); +is( $p->{nicname}, undef, 'installnic carrying an address names no interface' ); + +$p = xCAT::Template::install_device_params( 'mac', '', $MAC1, $NODE ); +is( $p->{mac}, $MAC1, 'the keyword mac falls back to mac.mac' ); +is( $p->{nicname}, undef, 'the keyword mac names no interface' ); + +# A mac.mac entry may hold several addresses. The entry tagged with the node +# name is the one that belongs to the node. +$p = xCAT::Template::install_device_params( '', '', "$MAC1!$NODE|$MAC2!other", $NODE ); +is( $p->{mac}, $MAC1, 'the tagged entry for the node is selected' ); + +# Ubuntu keeps its own return shape over the same resolution. +my ( $setname, $macaddress ) = + xCAT::Template::subiquity_install_netcfg( 'eth1', '', $MAC1, $NODE ); +is( $setname, 'eth1', 'netplan is given the interface to rename to' ); +is( $macaddress, lc($MAC1), 'netplan is given the address, lower cased' ); + +( $setname, $macaddress ) = + xCAT::Template::subiquity_install_netcfg( '', '', $MAC1, $NODE ); +is( $setname, '', 'netplan renames nothing when no interface is named' ); +is( $macaddress, lc($MAC1), 'netplan still matches on the address' ); + +# The kickstart names one device. It takes the interface when one is named and +# the address otherwise, and it lower cases either. +sub ks_device { + my ( $installnic, $primarynic, $macentry, $nodename ) = @_; + my $params = + xCAT::Template::install_device_params( $installnic, $primarynic, $macentry, $nodename ); + my $macaddr = defined( $params->{mac} ) ? lc( $params->{mac} ) : ''; + return defined( $params->{nicname} ) ? $params->{nicname} : $macaddr; +} + +is( ks_device( '', '', $MAC1, $NODE ), lc($MAC1), + 'a node setting neither attribute keeps the address it has today' ); +is( ks_device( 'eth1', '', $MAC1, $NODE ), 'eth1', + 'a node setting installnic names that interface' ); +is( ks_device( $MAC2, '', $MAC1, $NODE ), lc($MAC2), + 'a node whose installnic carries an address names that address' ); + +# The defect this closes: a bare multi address entry resolves to the LAST +# address, which need not be the adapter that deploys the node. Setting +# installnic must override that. +my $BARE = "$MAC1|$MAC2"; +is( ks_device( '', '', $BARE, $NODE ), lc($MAC2), + 'a bare multi address entry alone still resolves to the last address' ); +is( ks_device( 'eth0', '', $BARE, $NODE ), 'eth0', + 'installnic overrides a bare multi address entry' ); + +# An interface name is case sensitive. A node on POWER carries names such as +# enP1p12s0f0, which no longer name a device once they are lower cased. Only +# an address may be lower cased. +my $MIXED = 'enP1p12s0f0'; +is( ks_device( $MIXED, '', $MAC1, $NODE ), $MIXED, + 'the kickstart keeps the case of the interface name' ); +is( ks_device( '', $MIXED, $MAC1, $NODE ), $MIXED, + 'the kickstart keeps the case of a primarynic interface name' ); +my ($mixedset) = xCAT::Template::subiquity_install_netcfg( $MIXED, '', $MAC1, $NODE ); +is( $mixedset, $MIXED, 'Ubuntu keeps the case of the interface name too' ); +is( ks_device( '', '', uc($MAC1), $NODE ), lc($MAC1), + 'an address is still lower cased' ); + +# The source has to keep the unique local address built from the hardware +# address, not from the device name. +my $src = File::Spec->catfile( $root, 'xCAT-server', 'lib', 'perl', 'xCAT', 'Template.pm' ); +open( my $fh, '<', $src ) or die "Unable to read $src: $!"; +my $source = do { local $/; <$fh> }; +close($fh); + +my ($ksbody) = $source =~ /\nsub kickstartnetwork \{(.*?)\n\}\n/s; +ok( defined($ksbody), 'the kickstartnetwork body was located' ); + +like( $ksbody, qr/install_device_params\(/, + 'the kickstart resolves the device through the shared helper' ); +unlike( $ksbody, qr/parseMacTabEntry/, + 'the kickstart no longer names the device from mac.mac alone' ); +like( $ksbody, qr/my \$ulaaddr = autoulaaddress\(\$macaddr\)/, + 'the unique local address is built from the address, not the device name' ); +unlike( $ksbody, qr/lc\(\$params->\{nicname\}\)/, + 'the kickstart never lower cases the interface name' ); +my ($ubbody) = $source =~ /\nsub subiquity_install_netcfg \{(.*?)\n\}\n/s; +ok( defined($ubbody), 'the subiquity_install_netcfg body was located' ); +like( $ubbody, qr/install_device_params\(/, 'Ubuntu shares the same helper' ); +unlike( $ubbody, qr/gen_net_boot_params|parseMacTabEntry/, + 'Ubuntu no longer re-derives the resolution itself' ); + +done_testing(); From a065ecb136e1cc9f6883d0fbba66ab2d6f3e6191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 31 Aug 2026 19:57:35 -0300 Subject: [PATCH 4/4] test(template): verify kickstart behavior instead of source text --- xCAT-test/unit/template_install_device.t | 114 ++++++++++++++--------- 1 file changed, 72 insertions(+), 42 deletions(-) diff --git a/xCAT-test/unit/template_install_device.t b/xCAT-test/unit/template_install_device.t index 440a28b44..4a5f2e518 100644 --- a/xCAT-test/unit/template_install_device.t +++ b/xCAT-test/unit/template_install_device.t @@ -6,7 +6,6 @@ use File::Spec; use FindBin; use Test::More; -my $root = File::Spec->catdir( $FindBin::Bin, '..', '..' ); BEGIN { push @INC, File::Spec->catdir( $FindBin::Bin, '..', '..', 'perl-xCAT' ), @@ -52,75 +51,106 @@ is( $p->{mac}, $MAC1, 'the tagged entry for the node is selected' ); # Ubuntu keeps its own return shape over the same resolution. my ( $setname, $macaddress ) = xCAT::Template::subiquity_install_netcfg( 'eth1', '', $MAC1, $NODE ); -is( $setname, 'eth1', 'netplan is given the interface to rename to' ); -is( $macaddress, lc($MAC1), 'netplan is given the address, lower cased' ); +is( $setname, 'eth1', 'netplan is given the interface to rename to' ); +is( $macaddress, lc($MAC1), 'netplan is given the address, lower cased' ); ( $setname, $macaddress ) = xCAT::Template::subiquity_install_netcfg( '', '', $MAC1, $NODE ); is( $setname, '', 'netplan renames nothing when no interface is named' ); is( $macaddress, lc($MAC1), 'netplan still matches on the address' ); -# The kickstart names one device. It takes the interface when one is named and -# the address otherwise, and it lower cases either. -sub ks_device { - my ( $installnic, $primarynic, $macentry, $nodename ) = @_; - my $params = - xCAT::Template::install_device_params( $installnic, $primarynic, $macentry, $nodename ); - my $macaddr = defined( $params->{mac} ) ? lc( $params->{mac} ) : ''; - return defined( $params->{nicname} ) ? $params->{nicname} : $macaddr; +{ + package Local::TemplateInstallTable; + + sub getNodeAttribs { + my ($self) = @_; + return $self->{row}; + } + + sub setNodeAttribs { + my ( $self, undef, $attrs ) = @_; + $self->{written} = { %{$attrs} }; + return 1; + } } -is( ks_device( '', '', $MAC1, $NODE ), lc($MAC1), +# Exercise kickstartnetwork itself with in-memory table objects. This verifies +# the generated kickstart line rather than the text of Template.pm. +sub kickstart_network { + my ( $installnic, $primarynic, $macentry, $mode ) = @_; + + my $mactab = bless { row => { mac => $macentry } }, 'Local::TemplateInstallTable'; + my $nrtab = bless { + row => { + installnic => $installnic, + primarynic => $primarynic, + } + }, + 'Local::TemplateInstallTable'; + my $hoststab = bless {}, 'Local::TemplateInstallTable'; + my $autoula_mac; + + no warnings qw(redefine once); + local *xCAT::Table::new = sub { + my ( undef, $table ) = @_; + return $mactab if $table eq 'mac'; + return $nrtab if $table eq 'noderes'; + return $hoststab if $table eq 'hosts'; + die "Unexpected table $table"; + }; + local *xCAT::Template::autoulaaddress = sub { + ($autoula_mac) = @_; + return 'fd00::1'; + }; + local $::XCATSITEVALS{managedaddressmode} = $mode || 'dhcp'; + + my $line = xCAT::Template::kickstartnetwork(); + return ( $line, $autoula_mac, $hoststab->{written} ); +} + +sub ks_device { + my ($line) = kickstart_network(@_); + return $line =~ /--device=(\S+)/ ? $1 : undef; +} + +is( ks_device( '', '', $MAC1 ), lc($MAC1), 'a node setting neither attribute keeps the address it has today' ); -is( ks_device( 'eth1', '', $MAC1, $NODE ), 'eth1', +is( ks_device( 'eth1', '', $MAC1 ), 'eth1', 'a node setting installnic names that interface' ); -is( ks_device( $MAC2, '', $MAC1, $NODE ), lc($MAC2), +is( ks_device( $MAC2, '', $MAC1 ), lc($MAC2), 'a node whose installnic carries an address names that address' ); # The defect this closes: a bare multi address entry resolves to the LAST # address, which need not be the adapter that deploys the node. Setting # installnic must override that. my $BARE = "$MAC1|$MAC2"; -is( ks_device( '', '', $BARE, $NODE ), lc($MAC2), +is( ks_device( '', '', $BARE ), lc($MAC2), 'a bare multi address entry alone still resolves to the last address' ); -is( ks_device( 'eth0', '', $BARE, $NODE ), 'eth0', +is( ks_device( 'eth0', '', $BARE ), 'eth0', 'installnic overrides a bare multi address entry' ); # An interface name is case sensitive. A node on POWER carries names such as # enP1p12s0f0, which no longer name a device once they are lower cased. Only # an address may be lower cased. my $MIXED = 'enP1p12s0f0'; -is( ks_device( $MIXED, '', $MAC1, $NODE ), $MIXED, +is( ks_device( $MIXED, '', $MAC1 ), $MIXED, 'the kickstart keeps the case of the interface name' ); -is( ks_device( '', $MIXED, $MAC1, $NODE ), $MIXED, +is( ks_device( '', $MIXED, $MAC1 ), $MIXED, 'the kickstart keeps the case of a primarynic interface name' ); my ($mixedset) = xCAT::Template::subiquity_install_netcfg( $MIXED, '', $MAC1, $NODE ); is( $mixedset, $MIXED, 'Ubuntu keeps the case of the interface name too' ); -is( ks_device( '', '', uc($MAC1), $NODE ), lc($MAC1), +is( ks_device( '', '', uc($MAC1) ), lc($MAC1), 'an address is still lower cased' ); -# The source has to keep the unique local address built from the hardware -# address, not from the device name. -my $src = File::Spec->catfile( $root, 'xCAT-server', 'lib', 'perl', 'xCAT', 'Template.pm' ); -open( my $fh, '<', $src ) or die "Unable to read $src: $!"; -my $source = do { local $/; <$fh> }; -close($fh); - -my ($ksbody) = $source =~ /\nsub kickstartnetwork \{(.*?)\n\}\n/s; -ok( defined($ksbody), 'the kickstartnetwork body was located' ); - -like( $ksbody, qr/install_device_params\(/, - 'the kickstart resolves the device through the shared helper' ); -unlike( $ksbody, qr/parseMacTabEntry/, - 'the kickstart no longer names the device from mac.mac alone' ); -like( $ksbody, qr/my \$ulaaddr = autoulaaddress\(\$macaddr\)/, - 'the unique local address is built from the address, not the device name' ); -unlike( $ksbody, qr/lc\(\$params->\{nicname\}\)/, - 'the kickstart never lower cases the interface name' ); -my ($ubbody) = $source =~ /\nsub subiquity_install_netcfg \{(.*?)\n\}\n/s; -ok( defined($ubbody), 'the subiquity_install_netcfg body was located' ); -like( $ubbody, qr/install_device_params\(/, 'Ubuntu shares the same helper' ); -unlike( $ubbody, qr/gen_net_boot_params|parseMacTabEntry/, - 'Ubuntu no longer re-derives the resolution itself' ); +# autoula must derive the address from the hardware address even when the +# kickstart selects an interface by name. +my ( $autoula_line, $autoula_mac, $hostattrs ) = + kickstart_network( $MIXED, '', $MAC1, 'autoula' ); +is( $autoula_line, + "network --onboot=yes --bootproto=static --device=$MIXED --noipv4 --ipv6=fd00::1", + 'autoula keeps the selected interface in the kickstart line' ); +is( $autoula_mac, lc($MAC1), + 'autoula derives the address from the MAC, not the interface name' ); +is( $hostattrs->{ip}, 'fd00::1', 'the generated ULA is saved in the hosts table' ); done_testing();