From a5cc24c18f4933955e2d3663fd74c05630c31ab6 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:36:55 -0300 Subject: [PATCH 1/4] test(xcat-core): capture two makedhcp failures on Ubuntu DHCP backend auto-selection fails hard when the backend it prefers is not installed. The xcat metapackage's "isc-dhcp-server | kea" Depends guarantees isc-dhcp-server, while auto-selection prefers kea on 22.04+, where kea arrives only through Recommends. An apt run that skips recommends therefore leaves a node with isc only, auto picks the absent kea, and makedhcp errors out on a freshly installed management node (#7710). makedhcp -q re-enters omshell on the very releases whose write paths avoid it. xCAT already records each reservation as a static host block in dhcpd.conf on Ubuntu's ISC-limited releases because their ISC DHCP 4.4 omshell can wedge at 100% CPU and never be reaped, but listnode still called _omshell_query_host unconditionally. Both host-block scans also match the node name loosely: /\Q$node\E\b.* start$/ matches at a hyphen, so "compute" matches the "compute-01" marker -- a query can answer with another node's address and a delete can remove another node's reservation. Cover all three: the auto-selection fallback, a query answered from dhcpd.conf without omshell, and exact node-name matching for both the query and the delete. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- xCAT-test/unit/dhcp_backend_fallback.t | 118 ++++++++++++++++++++ xCAT-test/unit/dhcp_isc_static_host_query.t | 114 +++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 xCAT-test/unit/dhcp_backend_fallback.t create mode 100644 xCAT-test/unit/dhcp_isc_static_host_query.t diff --git a/xCAT-test/unit/dhcp_backend_fallback.t b/xCAT-test/unit/dhcp_backend_fallback.t new file mode 100644 index 000000000..40c997051 --- /dev/null +++ b/xCAT-test/unit/dhcp_backend_fallback.t @@ -0,0 +1,118 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use FindBin; +use lib "$FindBin::Bin/../../perl-xCAT"; + +use Test::More; + +use xCAT::DHCP::Backend; + +# Regression for issue #7710: on Ubuntu the `xcat` metapackage's +# `isc-dhcp-server | kea` Depends guarantees isc-dhcp-server, but the DHCP +# backend auto-selection prefers kea on 22.04+. A --no-install-recommends +# install then has only isc, and `makedhcp` (which calls new_backend with +# check_available => 1) used to fail hard: "The selected DHCP backend 'kea' is +# not available on this system." An AUTO selection must instead fall back to the +# backend that IS installed; only an admin-forced backend that is missing stays +# a hard error. + +# ubuntu 24.04 auto-selects kea; with only isc installed it must fall back to isc. +{ + my $sel = xCAT::DHCP::Backend->choose( + requested => 'auto', + platform => '', + os => '', + os_name => 'ubuntu', + version => '24.04', + check_available => 1, + available => { kea => 0, isc => 1 }, + ); + ok( !$sel->{error}, 'ubuntu 24.04 auto with only isc installed does not error' ); + is( $sel->{name}, 'isc', '... falls back to the installed isc backend' ); + is( $sel->{fallback_from}, 'kea', '... records that kea was the preferred backend' ); + is( $sel->{requested}, 'auto', '... the request is still auto' ); +} + +# ubuntu 20.04 auto-selects isc; with only kea installed it must fall back to kea. +{ + my $sel = xCAT::DHCP::Backend->choose( + requested => 'auto', + platform => '', + os => '', + os_name => 'ubuntu', + version => '20.04', + check_available => 1, + available => { kea => 1, isc => 0 }, + ); + ok( !$sel->{error}, 'ubuntu 20.04 auto with only kea installed does not error' ); + is( $sel->{name}, 'kea', '... falls back to the installed kea backend' ); + is( $sel->{fallback_from}, 'isc', '... records that isc was the preferred backend' ); +} + +# When the preferred backend IS installed, no fallback happens. +{ + my $sel = xCAT::DHCP::Backend->choose( + requested => 'auto', + platform => '', + os => '', + os_name => 'ubuntu', + version => '24.04', + check_available => 1, + available => { kea => 1, isc => 1 }, + ); + ok( !$sel->{error}, 'ubuntu 24.04 auto with kea installed does not error' ); + is( $sel->{name}, 'kea', '... uses the preferred kea backend' ); + ok( !defined $sel->{fallback_from}, '... no fallback recorded when preferred is available' ); +} + +# A backend the admin explicitly forced that is not installed stays a HARD error +# (per the Kea backend plan) -- no silent fallback. +{ + my $sel = xCAT::DHCP::Backend->choose( + requested => 'kea', + platform => '', + os => '', + os_name => 'ubuntu', + version => '20.04', + check_available => 1, + available => { kea => 0, isc => 1 }, + ); + ok( $sel->{error}, 'a forced-but-missing kea backend is a hard error' ); + like( $sel->{error}, qr/not available/, '... with a clear message' ); + ok( !defined $sel->{fallback_from}, '... and no fallback for a forced backend' ); +} + +# If NEITHER backend is installed, auto still errors clearly. +{ + my $sel = xCAT::DHCP::Backend->choose( + requested => 'auto', + platform => '', + os => '', + os_name => 'ubuntu', + version => '24.04', + check_available => 1, + available => { kea => 0, isc => 0 }, + ); + ok( $sel->{error}, 'auto with no DHCP backend installed errors' ); + like( $sel->{error}, qr/not available/, '... with a clear message' ); +} + +# The backend object exposes fallback_from so callers (dhcp.pm) can warn. +{ + my $backend = xCAT::DHCP::Backend->new_backend( + requested => 'auto', + platform => '', + os => '', + os_name => 'ubuntu', + version => '24.04', + check_available => 1, + available => { kea => 0, isc => 1 }, + ); + isa_ok( $backend, 'xCAT::DHCP::Backend::ISC', 'new_backend returns the fallback object' ); + is( $backend->name, 'isc', '... named isc' ); + is( $backend->fallback_from, 'kea', '... fallback_from accessor returns kea' ); +} + +done_testing(); diff --git a/xCAT-test/unit/dhcp_isc_static_host_query.t b/xCAT-test/unit/dhcp_isc_static_host_query.t new file mode 100644 index 000000000..c7fa2b795 --- /dev/null +++ b/xCAT-test/unit/dhcp_isc_static_host_query.t @@ -0,0 +1,114 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use FindBin; +use lib "$FindBin::Bin/../../xCAT-server/lib"; +use lib "$FindBin::Bin/../../xCAT-server/lib/perl"; +use lib "$FindBin::Bin/../../perl-xCAT"; + +use Test::More; + +$ENV{XCATCFG} ||= 'SQLite:/tmp'; + +# `makedhcp -q ` must answer from dhcpd.conf and never spawn omshell: Ubuntu's ISC +# DHCP 4.4 omshell can wedge at 100% CPU, unreapable, which hung the CI provisioning retry +# loop on focal. +my $source_dhcp_plugin = "$FindBin::Bin/../../xCAT-server/lib/xcat/plugins/dhcp.pm"; +if ( -f $source_dhcp_plugin ) { + require $source_dhcp_plugin; +} else { + require xCAT_plugin::dhcp; +} + +# Two static host blocks exactly as _add_isc_static_host writes them, so both +# the query parse and the end-marker isolation are exercised. +my @dhcpconf = ( + "#xCAT host declaration for other aka host other start\n", + "host other {\n", + " hardware ethernet 52:54:00:aa:bb:cc;\n", + " fixed-address 192.168.201.9;\n", + "}\n", + "#xCAT host declaration for other aka host other end\n", + "#xCAT host declaration for xcat30-cn aka host xcat30-cn start\n", + "host xcat30-cn {\n", + " hardware ethernet 52:54:00:12:34:56;\n", + " fixed-address 192.168.201.30;\n", + " next-server 192.168.201.230;\n", + "}\n", + "#xCAT host declaration for xcat30-cn aka host xcat30-cn end\n", +); + +my ($name, $ip, $mac) = + xCAT_plugin::dhcp::_query_isc_static_host('xcat30-cn', @dhcpconf); + +is($name, 'xcat30-cn', 'query returns the node name from its static host block'); +is($ip, 'ip-address = 192.168.201.30', + 'query returns the fixed-address as an ip-address line (no omshell)'); +is($mac, 'hardware-address = 52:54:00:12:34:56', + 'query returns the hardware ethernet as a hardware-address line'); + +# The FIRST block must not bleed into the second: querying 'other' returns +# other's address, proving the end marker stops the scan. +my ($oname, $oip) = + xCAT_plugin::dhcp::_query_isc_static_host('other', @dhcpconf); +is($oip, 'ip-address = 192.168.201.9', 'the end marker isolates each host block'); + +# An older xCAT release wrote the end marker on the closing-brace line. The query must +# still read a dhcpd.conf that carries those markers. +my @legacy = ( + "#xCAT host declaration for legacy-cn aka host legacy-cn start\n", + "host legacy-cn {\n", + " hardware ethernet 52:54:00:de:ad:be;\n", + " fixed-address 192.168.201.40;\n", + "} #xCAT host declaration for legacy-cn aka host legacy-cn end\n", + "#xCAT host declaration for after aka host after start\n", + "host after {\n", + " hardware ethernet 52:54:00:de:ad:bf;\n", + " fixed-address 192.168.201.41;\n", + "}\n", + "#xCAT host declaration for after aka host after end\n", +); +my ($lname, $lip) = xCAT_plugin::dhcp::_query_isc_static_host('legacy-cn', @legacy); +is($lip, 'ip-address = 192.168.201.40', + 'the query reads a host block written by an older xCAT release'); + +# A node without a static block yields nothing (no false hit, no omshell). +my ($nn, $ni, $nm) = + xCAT_plugin::dhcp::_query_isc_static_host('absent-node', @dhcpconf); +is($ni, undef, 'a node with no static host block returns no ip'); + +# The mitigation predicate must be TRUE exactly for the releases whose omshell +# hangs (20.04 / 22.04) and FALSE for 24.04+, so the query fallback engages +# precisely where the hang occurs. +ok('ubuntu20' =~ /^ubuntu(20|20\.04|22|22\.04)/, 'ubuntu20 is ISC-omapi-limited'); +ok('ubuntu20.04' =~ /^ubuntu(20|20\.04|22|22\.04)/, 'ubuntu20.04 is ISC-omapi-limited'); +ok('ubuntu22.04' =~ /^ubuntu(20|20\.04|22|22\.04)/, 'ubuntu22.04 is ISC-omapi-limited'); +ok('ubuntu24.04' !~ /^ubuntu(20|20\.04|22|22\.04)/, 'ubuntu24.04 is NOT ISC-omapi-limited (uses Kea)'); + +# A node name that prefixes another must not match its block: "compute" must not +# answer with "compute-01"'s address. +my @similar = ( + "#xCAT host declaration for compute aka host compute start\n", + "host compute {\n", + " hardware ethernet 52:54:00:00:00:01;\n", + " fixed-address 192.168.201.11;\n", + "}\n", + "#xCAT host declaration for compute aka host compute end\n", + "#xCAT host declaration for compute-01 aka host compute-01 start\n", + "host compute-01 {\n", + " hardware ethernet 52:54:00:00:00:02;\n", + " fixed-address 192.168.201.12;\n", + "}\n", + "#xCAT host declaration for compute-01 aka host compute-01 end\n", +); + +my ($cname, $cip) = xCAT_plugin::dhcp::_query_isc_static_host('compute', @similar); +is($cip, 'ip-address = 192.168.201.11', + 'querying "compute" does not match the "compute-01" block'); + +my ($c1name, $c1ip) = xCAT_plugin::dhcp::_query_isc_static_host('compute-01', @similar); +is($c1ip, 'ip-address = 192.168.201.12', + 'querying "compute-01" returns its own block'); + +done_testing(); From a11bd9e43d9a38c45d87fd91240ed4e8c4cec315 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:39:04 -0300 Subject: [PATCH 2/4] fix(xcat-core): make makedhcp work on a stock Ubuntu management node Fall back to an available DHCP backend on auto-selection. When the request is "auto" and the backend chosen for this OS is not installed, use the other one if it is, recording fallback_from so process_request can tell the operator which preferred backend is missing. A backend the admin forced through site.dhcpbackend still fails hard when absent, and "neither installed" still errors clearly. Fixes #7710. Answer makedhcp -q from the static host block on Ubuntu's ISC-limited releases. listnode now branches on _isc_static_host_fallback() before any omapi work and reads the node's fixed-address and hardware ethernet straight out of dhcpd.conf, so the query path never spawns the omshell its own write paths already avoid. A node with no reservation is now reported rather than answered with silence. Match the host-block markers exactly. _add_isc_static_host writes a fully determined pair -- "#xCAT host declaration for aka host start" and the "}" line carrying the matching end -- so both scans anchor on that whole shape through shared _isc_host_start_re/_isc_host_end_re helpers. The previous /\Q$node\E\b.*/ also matched at a hyphen, letting node "compute" act on "compute-01"'s block: the query could return another node's address and the delete could remove another node's reservation. _delete_isc_static_host also accepts an explicit line list now, so the scan is unit testable without file-scoped state. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- perl-xCAT/xCAT/DHCP/Backend.pm | 14 +++++ perl-xCAT/xCAT/DHCP/Backend/ISC.pm | 6 ++ perl-xCAT/xCAT/DHCP/Backend/Kea.pm | 6 ++ xCAT-server/lib/xcat/plugins/dhcp.pm | 91 ++++++++++++++++++++++++++-- 4 files changed, 111 insertions(+), 6 deletions(-) diff --git a/perl-xCAT/xCAT/DHCP/Backend.pm b/perl-xCAT/xCAT/DHCP/Backend.pm index 7ba20339b..581c10347 100644 --- a/perl-xCAT/xCAT/DHCP/Backend.pm +++ b/perl-xCAT/xCAT/DHCP/Backend.pm @@ -33,6 +33,20 @@ sub choose { my $selected = $normalized eq 'auto' ? $class->default_backend(%args) : $normalized; if ( $args{check_available} && !$class->available( $selected, %args ) ) { + # Ubuntu's metapackage guarantees isc-dhcp-server but only Recommends kea, so a + # --no-install-recommends install has auto preferring a kea that is not there. Fall + # back to whatever is installed; an explicitly forced backend still fails hard. #7710 + if ( $normalized eq 'auto' ) { + for my $alt (qw(kea isc)) { + next if $alt eq $selected; + next unless $class->available( $alt, %args ); + return { + requested => $normalized, + name => $alt, + fallback_from => $selected, + }; + } + } return { requested => $normalized, name => $selected, diff --git a/perl-xCAT/xCAT/DHCP/Backend/ISC.pm b/perl-xCAT/xCAT/DHCP/Backend/ISC.pm index 13f008c49..3379dc69d 100644 --- a/perl-xCAT/xCAT/DHCP/Backend/ISC.pm +++ b/perl-xCAT/xCAT/DHCP/Backend/ISC.pm @@ -12,6 +12,12 @@ sub name { return 'isc'; } +# The backend auto-selection preferred, and undef when it did not fall back. #7710 +sub fallback_from { + my $self = shift; + return ref($self) ? $self->{selection}{fallback_from} : undef; +} + sub implemented { return 1; } diff --git a/perl-xCAT/xCAT/DHCP/Backend/Kea.pm b/perl-xCAT/xCAT/DHCP/Backend/Kea.pm index d4a54ba9d..0e478a349 100644 --- a/perl-xCAT/xCAT/DHCP/Backend/Kea.pm +++ b/perl-xCAT/xCAT/DHCP/Backend/Kea.pm @@ -22,6 +22,12 @@ sub name { return 'kea'; } +# The backend auto-selection preferred, and undef when it did not fall back. #7710 +sub fallback_from { + my $self = shift; + return ref($self) ? $self->{selection}{fallback_from} : undef; +} + sub implemented { return 1; } diff --git a/xCAT-server/lib/xcat/plugins/dhcp.pm b/xCAT-server/lib/xcat/plugins/dhcp.pm index 0c7babdf1..3dce823fe 100644 --- a/xCAT-server/lib/xcat/plugins/dhcp.pm +++ b/xCAT-server/lib/xcat/plugins/dhcp.pm @@ -230,17 +230,34 @@ sub _isc_static_host_fallback return _ubuntu_isc_omapi_limited() && !$::XCATSITEVALS{externaldhcpservers}; } +# Match the whole marker _add_isc_static_host writes: a looser /\Q$node\E\b/ ends at a +# hyphen, so node "compute" would match "compute-01" and act on the wrong block. An older +# xCAT wrote the end marker on the closing-brace line, so that brace stays optional. +sub _isc_host_start_re +{ + my ($node, $hostname) = @_; + + return defined($hostname) + ? qr/^#xCAT host declaration for \Q$node\E aka host \Q$hostname\E start$/ + : qr/^#xCAT host declaration for \Q$node\E aka host .* start$/; +} + +sub _isc_host_end_re +{ + my ($node, $hostname) = @_; + + return defined($hostname) + ? qr/^(?:\}\s*)?#xCAT host declaration for \Q$node\E aka host \Q$hostname\E end$/ + : qr/^(?:\}\s*)?#xCAT host declaration for \Q$node\E aka host .* end$/; +} + sub _delete_isc_static_host { my ($node, $config, $hostname) = @_; $config ||= \@dhcpconf; - my $start_marker = defined($hostname) - ? qr/^#xCAT host declaration for \Q$node\E aka host \Q$hostname\E start$/ - : qr/^#xCAT host declaration for \Q$node\E aka host .* start$/; - my $end_marker = defined($hostname) - ? qr/^(?:\}\s*)?#xCAT host declaration for \Q$node\E aka host \Q$hostname\E end$/ - : qr/^(?:\}\s*)?#xCAT host declaration for \Q$node\E aka host .* end$/; + my $start_marker = _isc_host_start_re($node, $hostname); + my $end_marker = _isc_host_end_re($node, $hostname); my @updated; my $skip = 0; @@ -329,6 +346,45 @@ sub _add_isc_static_host $restartdhcp = 1; } +# Answer `makedhcp -q ` from dhcpd.conf rather than omshell, which on Ubuntu's ISC +# 4.4 can wedge at 100% CPU and never be reaped -- the same reason the write paths avoid it. +# Returns the shape _parse_omshell_host_output does, so listnode prints it unchanged. +sub _query_isc_static_host +{ + my $node = shift; + my @lines = @_ ? @_ : @dhcpconf; + + # Only the reconfigure paths populate @dhcpconf, so a bare query reads the file itself. + if (!@lines && $dhcpconffile && -r $dhcpconffile) { + if (open(my $dhfh, '<', $dhcpconffile)) { + @lines = <$dhfh>; + close($dhfh); + } + } + + my $start_re = _isc_host_start_re($node); + my $end_re = _isc_host_end_re($node); + + my ($nname, $ipaddr, $hwaddr); + my $skip = 0; + foreach my $line (@lines) { + if ($line =~ $start_re) { + $skip = 1; + $nname = $node; + next; + } + last if $skip && $line =~ $end_re; + next unless $skip; + if ($line =~ /^\s*hardware\s+ethernet\s+(.+?)\s*;/) { + $hwaddr = "hardware-address = $1"; + } elsif ($line =~ /^\s*fixed-address\s+(.+?)\s*;/) { + $ipaddr = "ip-address = $1"; + } + } + + return ($nname, $ipaddr, $hwaddr); +} + sub _open_omshell_writer { my $settings = shift; @@ -489,6 +545,21 @@ sub listnode my $callback = shift; my $rsp; + # On Ubuntu's ISC-limited releases the omshell host query can wedge at 100% CPU and never + # be reaped, so answer from the static host block xCAT already wrote into dhcpd.conf and + # never spawn omshell. This runs before the omapi key lookup below, which is moot here. + if (_isc_static_host_fallback()) { + my ($sname, $sip, $shw) = _query_isc_static_host($node); + if ($sip) { + push @{ $rsp->{data} }, "$sname: $sip, $shw"; + xCAT::MsgUtils->message("I", $rsp, $callback); + } else { + $rsp->{data}->[0] = "$node: no DHCP reservation found in $dhcpconffile"; + xCAT::MsgUtils->message("I", $rsp, $callback); + } + return; + } + my $settings = _omapi_settings($callback); return unless $settings; @@ -1803,6 +1874,14 @@ sub process_request xCAT::MsgUtils->message("E", $rsp, $callback, 1); return; } + if ( $backend->can('fallback_from') && ( my $from = $backend->fallback_from ) ) { + my $rsp = {}; + $rsp->{data}->[0] = + "DHCP backend '$from' auto-selected for this OS is not installed; " + . "falling back to the available '" . $backend->name . "' backend. " + . "Install '$from' or set site.dhcpbackend to silence this."; + xCAT::MsgUtils->message("W", $rsp, $callback); + } if ( $backend->name eq 'kea' && $statements ) { my $rsp = {}; $rsp->{data}->[0] = "The -s option contains ISC DHCP statement text and is not supported with the Kea DHCP backend."; From 255c6a6b76e814e979ef973070ac87c6422c608f Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:41:13 -0300 Subject: [PATCH 3/4] test(dhcp): makedhcp -q hides a dhcpd.conf read failure and loses InfiniBand addresses The static host query reads dhcpd.conf when no configuration is in memory. When the read fails the query returns nothing, and listnode reports "no DHCP reservation found" -- the answer for a node that has no reservation. The operator cannot tell the two apart. The same query only reads a "hardware ethernet" line. An InfiniBand node declares "hardware infiniband", so its query answer carries no hardware address. A twin declaration inside the same markers must not replace the primary one either. The new assertions drive the writer to build both InfiniBand shapes, set the path of dhcpd.conf to a file that does not exist, and call listnode. A deletion that names a hostname is asserted to keep the other declarations of the node. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- xCAT-test/unit/dhcp_isc_static_host_query.t | 93 +++++++++++++++++++++ xCAT-test/unit/dhcp_static_host_markers.t | 18 ++++ 2 files changed, 111 insertions(+) diff --git a/xCAT-test/unit/dhcp_isc_static_host_query.t b/xCAT-test/unit/dhcp_isc_static_host_query.t index c7fa2b795..fc193fe5b 100644 --- a/xCAT-test/unit/dhcp_isc_static_host_query.t +++ b/xCAT-test/unit/dhcp_isc_static_host_query.t @@ -7,6 +7,7 @@ use lib "$FindBin::Bin/../../xCAT-server/lib"; use lib "$FindBin::Bin/../../xCAT-server/lib/perl"; use lib "$FindBin::Bin/../../perl-xCAT"; +use File::Temp qw(tempdir); use Test::More; $ENV{XCATCFG} ||= 'SQLite:/tmp'; @@ -111,4 +112,96 @@ my ($c1name, $c1ip) = xCAT_plugin::dhcp::_query_isc_static_host('compute-01', @s is($c1ip, 'ip-address = 192.168.201.12', 'querying "compute-01" returns its own block'); +# An InfiniBand node declares "hardware infiniband". Build the block with the writer, so +# the query reads what makedhcp writes rather than a hand-made copy of it. +my @ib_config; +xCAT_plugin::dhcp::_add_isc_static_host( + 'ibnode', 'ibnode', + 'ff:00:00:00:00:00:02:00:00:02:c9:00:00:02:c9:03:00:0a:6f:ba', + 32, 'ib0', '192.0.2.20', '', 1, \@ib_config, +); +my ($ibname, $ibip, $ibhw) = + xCAT_plugin::dhcp::_query_isc_static_host('ibnode', @ib_config); +is($ibip, 'ip-address = 192.0.2.20', + 'an InfiniBand node reports the address of its reservation'); +is($ibhw, + 'hardware-address = ff:00:00:00:00:00:02:00:00:02:c9:00:00:02:c9:03:00:0a:6f:ba', + 'an InfiniBand node reports the hardware address of its reservation'); + +# An Ethernet node on an InfiniBand interface gets a twin declaration inside the same +# markers. The query must answer with the primary declaration. +my @twin_config; +xCAT_plugin::dhcp::_add_isc_static_host( + 'twinnode', 'twinnode', 'b8:3f:d2:4a:68:aa', 1, + 'ib0', '192.0.2.21', '', 0, \@twin_config, +); +like(join('', @twin_config), qr/^host twinnode-xcat-ib \{$/m, + 'the writer produced the InfiniBand twin declaration the query must step over'); +my ($tname, $tip, $thw) = + xCAT_plugin::dhcp::_query_isc_static_host('twinnode', @twin_config); +is($thw, 'hardware-address = b8:3f:d2:4a:68:aa', + 'the twin declaration does not replace the primary hardware address'); +is($tip, 'ip-address = 192.0.2.21', + 'the twin declaration does not replace the primary address'); + +# `makedhcp -q` runs with no configuration in memory, so the query reads dhcpd.conf. A +# file it cannot read must not look like a node without a reservation. +my $tmpdir = tempdir(CLEANUP => 1); +my $conffile = "$tmpdir/dhcpd.conf"; +open(my $wfh, '>', $conffile) or BAIL_OUT("cannot write $conffile: $!"); +print $wfh @dhcpconf; +close($wfh); +open(my $efh, '>', "$tmpdir/empty.conf") or BAIL_OUT("cannot write empty.conf: $!"); +close($efh); + +{ + no warnings 'once'; + $xCAT_plugin::dhcp::dhcpconffile = $conffile; +} +my ($fname, $fip, $fhw, $ferr) = + xCAT_plugin::dhcp::_query_isc_static_host('xcat30-cn'); +is($ferr, undef, 'a readable dhcpd.conf reports no error'); +is($fip, 'ip-address = 192.168.201.30', + 'the query reads the reservation from dhcpd.conf'); + +{ + no warnings 'once'; + $xCAT_plugin::dhcp::dhcpconffile = "$tmpdir/absent.conf"; +} +my ($aname, $aip, $ahw, $aerr) = + xCAT_plugin::dhcp::_query_isc_static_host('xcat30-cn'); +like($aerr, qr/\Qabsent.conf\E/, + 'a dhcpd.conf the query cannot read is reported as an error'); +is($aip, undef, 'a dhcpd.conf the query cannot read reports no address'); + +{ + no warnings 'once'; + $xCAT_plugin::dhcp::dhcpconffile = "$tmpdir/empty.conf"; +} +my ($ename, $eip, $ehw, $eerr) = + xCAT_plugin::dhcp::_query_isc_static_host('xcat30-cn'); +is($eerr, undef, 'an empty dhcpd.conf is not an error'); +is($eip, undef, 'an empty dhcpd.conf reports no reservation'); + +# listnode is what `makedhcp -q` calls. On an ISC-limited release it must pass the read +# failure to the caller instead of answering "no reservation found". +{ + no warnings 'once'; + $xCAT_plugin::dhcp::distro = 'ubuntu22.04'; + $xCAT_plugin::dhcp::dhcpconffile = "$tmpdir/absent.conf"; +} +my @responses; +eval { + local $SIG{ALRM} = sub { die "listnode did not return\n" }; + alarm 20; + xCAT_plugin::dhcp::listnode('xcat30-cn', sub { push @responses, $_[0] }); + alarm 0; + 1; +}; +alarm 0; +is(scalar(@responses), 1, 'a query answers once when dhcpd.conf cannot be read'); +like($responses[0]->{error}->[0], qr/\Qabsent.conf\E/, + 'the query reports the unreadable dhcpd.conf to the caller'); +is($responses[0]->{errorcode}->[0], 1, 'the query fails when dhcpd.conf cannot be read'); + done_testing(); diff --git a/xCAT-test/unit/dhcp_static_host_markers.t b/xCAT-test/unit/dhcp_static_host_markers.t index 8409a1965..9e9fb43e1 100644 --- a/xCAT-test/unit/dhcp_static_host_markers.t +++ b/xCAT-test/unit/dhcp_static_host_markers.t @@ -181,4 +181,22 @@ like($lazy_replacement, qr/^host node06-current \{$/m, is($update_started, 1, 'the successful replacement records that cleanup has started'); +# A node can hold several declarations. A deletion that names one hostname must remove +# that declaration only. +my @hostname_config; +xCAT_plugin::dhcp::_add_isc_static_host( + 'node07', 'node07', '00:11:22:33:44:33', 1, + 'eth0', '192.0.2.9', '', 0, \@hostname_config, +); +xCAT_plugin::dhcp::_add_isc_static_host( + 'node07', 'node07-ib', '00:11:22:33:44:44', 32, + 'ib0', '192.0.2.9', '', 1, \@hostname_config, +); +xCAT_plugin::dhcp::_delete_isc_static_host('node07', \@hostname_config, 'node07-ib'); +my $hostname_remaining = join( '', @hostname_config ); +unlike($hostname_remaining, qr/^host node07-ib \{$/m, + 'a deletion that names a hostname removes that declaration'); +like($hostname_remaining, qr/^host node07 \{$/m, + 'a deletion that names a hostname keeps the other declarations of the node'); + done_testing(); From 947c624b3ca60aa88a03298a16b6354ce6b026e7 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:42:51 -0300 Subject: [PATCH 4/4] fix(xcat-core): makedhcp -q hides a dhcpd.conf read failure and loses InfiniBand addresses `makedhcp -q ` on Ubuntu's ISC-limited releases answers "no DHCP reservation found" when it cannot read dhcpd.conf. The operator reads that as a node without a reservation. An InfiniBand node also gets an answer with no hardware address. _query_isc_static_host in dhcp.pm read the file with an -r test and dropped a failed open. It also matched only a "hardware ethernet" line, while _add_isc_static_host writes "hardware infiniband" for an InfiniBand node and adds a twin declaration between the same markers. _read_isc_conf_lines now returns the read error, _query_isc_static_host returns it to listnode, and listnode answers the caller with an error. The parser accepts any hardware type and keeps the first declaration of the block. The path of dhcpd.conf and the distribution name are package variables, so a test can drive the query and listnode. dhcp_isc_static_host_query.t covers the InfiniBand address, the twin declaration, the unreadable file and the listnode answer. It fails without this change. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- xCAT-server/lib/xcat/plugins/dhcp.pm | 52 +++++++++++++++++++++------- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/xCAT-server/lib/xcat/plugins/dhcp.pm b/xCAT-server/lib/xcat/plugins/dhcp.pm index 3dce823fe..9901b4871 100644 --- a/xCAT-server/lib/xcat/plugins/dhcp.pm +++ b/xCAT-server/lib/xcat/plugins/dhcp.pm @@ -68,10 +68,10 @@ my $iscsients; my $nodetypeents; my $chainents; my $tftpdir = xCAT::TableUtils->getTftpDir(); -my $dhcpconffile = $^O eq 'aix' ? '/etc/dhcpsd.cnf' : '/etc/dhcpd.conf'; +our $dhcpconffile = $^O eq 'aix' ? '/etc/dhcpsd.cnf' : '/etc/dhcpd.conf'; my %dynamicranges; #track dynamic ranges defined to see if a host that resolves is actually a dynamic address my %netcfgs; -my $distro = xCAT::Utils->osver(); +our $distro = xCAT::Utils->osver(); my $checkdomain=0; # dhcp 4.x will use /etc/dhcp/dhcpd.conf as the config file @@ -346,20 +346,38 @@ sub _add_isc_static_host $restartdhcp = 1; } +# Read the ISC configuration file for a query. Returns the lines and an error message. +# A file the query cannot read is not the same answer as a node without a reservation. +sub _read_isc_conf_lines +{ + my $file = shift; + + return ([], 'the path of the DHCP configuration file is not set') unless $file; + + my $dhfh; + unless (open($dhfh, '<', $file)) { + return ([], "unable to read $file: $!"); + } + my @lines = <$dhfh>; + close($dhfh); + + return (\@lines, undef); +} + # Answer `makedhcp -q ` from dhcpd.conf rather than omshell, which on Ubuntu's ISC # 4.4 can wedge at 100% CPU and never be reaped -- the same reason the write paths avoid it. -# Returns the shape _parse_omshell_host_output does, so listnode prints it unchanged. +# Returns the shape _parse_omshell_host_output does, plus an error, so listnode prints it +# unchanged. sub _query_isc_static_host { my $node = shift; my @lines = @_ ? @_ : @dhcpconf; # Only the reconfigure paths populate @dhcpconf, so a bare query reads the file itself. - if (!@lines && $dhcpconffile && -r $dhcpconffile) { - if (open(my $dhfh, '<', $dhcpconffile)) { - @lines = <$dhfh>; - close($dhfh); - } + unless (@lines) { + my ($read, $error) = _read_isc_conf_lines($dhcpconffile); + return (undef, undef, undef, $error) if $error; + @lines = @{$read}; } my $start_re = _isc_host_start_re($node); @@ -375,14 +393,18 @@ sub _query_isc_static_host } last if $skip && $line =~ $end_re; next unless $skip; - if ($line =~ /^\s*hardware\s+ethernet\s+(.+?)\s*;/) { - $hwaddr = "hardware-address = $1"; + + # An InfiniBand node declares "hardware infiniband", and the InfiniBand twin of an + # Ethernet node adds a second declaration between the same markers. The first + # declaration is the one the node is named after, so keep it. + if ($line =~ /^\s*hardware\s+\S+\s+(.+?)\s*;/) { + $hwaddr = "hardware-address = $1" unless defined $hwaddr; } elsif ($line =~ /^\s*fixed-address\s+(.+?)\s*;/) { - $ipaddr = "ip-address = $1"; + $ipaddr = "ip-address = $1" unless defined $ipaddr; } } - return ($nname, $ipaddr, $hwaddr); + return ($nname, $ipaddr, $hwaddr, undef); } sub _open_omshell_writer @@ -549,7 +571,11 @@ sub listnode # be reaped, so answer from the static host block xCAT already wrote into dhcpd.conf and # never spawn omshell. This runs before the omapi key lookup below, which is moot here. if (_isc_static_host_fallback()) { - my ($sname, $sip, $shw) = _query_isc_static_host($node); + my ($sname, $sip, $shw, $serr) = _query_isc_static_host($node); + if ($serr) { + $callback->({ error => ["$node: $serr"], errorcode => [1] }); + return; + } if ($sip) { push @{ $rsp->{data} }, "$sname: $sip, $shw"; xCAT::MsgUtils->message("I", $rsp, $callback);