xCAT/xcat-core
2
0
Fork 0
mirror of https://github.com/xcat2/xcat-core.git synced 2026-10-06 17:46:55 +00:00
Code Issues Releases Wiki Activity

fix(dhcp): give both backends one answer for where a node is sent

Browse Source 
siaddr comes from noderes, in an order neither backend had right. ISC
read xcatmaster only for petitboot and onie, so every other node in a
hierarchical cluster was sent to the management node rather than to its
service node -- and even for those two the address went into the URL
without going into siaddr, so one reply named two different machines.
Kea read xcatmaster for every node, but when nothing named a server it
used my_ip_facing rather than the subnet's value, which is a different
answer whenever networks.tftpserver names a third machine.

Both now read next_server_for_node: the node's tftpserver, then its
xcatmaster, then -- only for the methods that build a URL and so need an
address in hand -- the interface facing the node. A node that named none
of them inherits the subnet's value, which ISC states in the subnet and
Kea states by leaving next-server out of the reservation.

An xcatmaster that does not resolve is now an error on both rather than
silence on one: it is a misconfiguration, and sending the node somewhere
else instead hides it.

Appendix A rows 26 and 27. Unlike the rest, these two are not in
xcat-internal#175 -- the wire cases found them.
This commit is contained in:
Daniel Hilst
2026-09-10 14:01:22 -03:00
parent 980b7de169
commit 26e8e277ef
3 changed files with 154 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files
xCAT-server/lib/xcat/plugins/dhcp.pm
+67 -75
View File
@@ -1003,66 +1003,17 @@ sub addnode
if ($nrhash)
{
$nrent = $nrhash->{$node}->[0];
if ($nrent and $nrent->{tftpserver} and $nrent->{tftpserver} ne '<xcatmaster>')
{
#check the value of inet_ntoa(inet_aton("")),if the hostname cannot be resolved,
#the value of inet_ntoa() will be "undef", which will cause fatal error
my $tmp_name = inet_aton($nrent->{tftpserver});
unless ($tmp_name) {
#tell the reason to the user
$callback->(
{ error => ["Unable to resolve the tftpserver for node"], errorcode => [1] }
);
return;
}
$tftpserver = inet_ntoa($tmp_name);
$nxtsrv = $tftpserver;
$lstatements = _omapi_next_server_statement($tftpserver) . $statements;
}
else
{
$guess_next_server = 1;
}
if ($nrent->{netboot} and ($nrent->{netboot} eq 'petitboot' or $nrent->{netboot} eq 'onie' )) {
if ($guess_next_server) {
my $node_server = undef;
if ($nrent->{xcatmaster}) {
$node_server = $nrent->{xcatmaster};
}
unless ($node_server) {
my @nxtsrvd = xCAT::NetworkUtils->my_ip_facing($node);
unless ($nxtsrvd[0]) { $nxtsrv = $nxtsrvd[1]; }
elsif ($nxtsrvd[0] == 1) { $callback->({ error => [ $nxtsrvd[1] ] }); }
else {
$callback->({ error => ["Unable to determine the tftpserver for $node, verify \"xcatmaster\" is set correctly"], errorcode => [1] });
return;
}
} else {
my $tmp_server = inet_aton($node_server);
unless ($tmp_server) {
$callback->({ error => ["Unable to resolve the tftpserver for $node, verify \"xcatmaster\" is set correctly"], errorcode => [1] });
return;
}
$nxtsrv = inet_ntoa($tmp_server);
}
unless ($nxtsrv) {
$callback->({ error => ["Unable to determine the tftpserver for $node, verify \"xcatmaster\" is set correctly"], errorcode => [1] });
return;
}
$guess_next_server = 0;
}
}
#else {
# $nrent = $nrtab->getNodeAttribs($node,['servicenode']);
# if ($nrent and $nrent->{servicenode}) {
# $statements = 'next-server = \"'.inet_ntoa(inet_aton($nrent->{servicenode})).'\";'.$statements;
# }
#}
}
else
{
# Which server this node is sent to. Both backends read the same answer
# out of next_server_for_node; an undefined tftpserver is the one case
# nothing here can name, and the subnet's own value carries it.
( $nxtsrv, $tftpserver ) = next_server_for_node( $node, $nrent );
return unless defined $nxtsrv;
if ( defined $tftpserver ) {
$lstatements = _omapi_next_server_statement($tftpserver) . $statements;
}
else {
$guess_next_server = 1;
}
unless ($machash)
@@ -3659,7 +3610,7 @@ sub kea_node_reservations
return [];
}
my ( $nxtsrv, $tftpserver ) = kea_next_server_for_node($node, $nrent);
my ( $nxtsrv, $tftpserver ) = next_server_for_node($node, $nrent);
my @reservations;
my @macs = split(/\|/, $macent->{mac});
foreach my $mace (@macs) {
@@ -3851,7 +3802,7 @@ sub kea_node_client_classes_for_nodes
my $macent = $macents && $macents->{$node} ? $macents->{$node}->[0] : undef;
next unless $macent && $macent->{mac};
my ( $nxtsrv ) = kea_next_server_for_node($node, $nrent);
my ( $nxtsrv ) = next_server_for_node($node, $nrent);
my $ient = $ients && $ients->{$node} ? $ients->{$node}->[0] : undef;
my $iname = ( $ient and $ient->{server} and $ient->{target} ) ? $ient->{iname} : undef;
@@ -4081,35 +4032,76 @@ sub kea_query_node
}
}
sub kea_next_server_for_node
#: A netboot method that builds a URL needs the server's address in hand, so
#: for those the search cannot end at "whatever the subnet says" -- there is
#: nothing to interpolate a subnet value into.
sub _needs_absolute_next_server
{
my ($nrent) = @_;
my $netboot = $nrent ? $nrent->{netboot} : undef;
return 0 unless $netboot;
return ( $netboot eq 'petitboot' or $netboot eq 'onie' ) ? 1 : 0;
}
#: Which server a node is sent to, in the order noderes states it: the node's
#: own tftpserver, then its xcatmaster, then -- only for the methods above --
#: the interface of this machine that faces the node. A node that named none of
#: them inherits the subnet's value, which is what '${next-server}' stands for
#: and why the second return value is undefined there: there is no per-node
#: address to write down.
#:
#: Returns ( next-server, tftpserver ) or the empty list, having already told
#: the caller's callback why.
#:
#: Both backends read this. They had a copy each and the copies had drifted:
#: ISC honoured xcatmaster only for petitboot and onie, so every other node in
#: a hierarchical cluster was sent to the management node instead of to its
#: service node; and Kea fell back to my_ip_facing for every node, so a subnet
#: whose tftpserver named some third machine was overruled on one backend and
#: obeyed on the other.
sub next_server_for_node
{
my ( $node, $nrent ) = @_;
if ($nrent and $nrent->{tftpserver} and $nrent->{tftpserver} ne '<xcatmaster>') {
my $tmp_name = inet_aton($nrent->{tftpserver});
#check the value of inet_ntoa(inet_aton("")),if the hostname cannot be resolved,
#the value of inet_ntoa() will be "undef", which will cause fatal error
if ( $nrent and $nrent->{tftpserver} and $nrent->{tftpserver} ne '<xcatmaster>' ) {
my $tmp_name = inet_aton( $nrent->{tftpserver} );
unless ($tmp_name) {
#tell the reason to the user
$callback->({ error => ["Unable to resolve the tftpserver for node"], errorcode => [1] });
return;
}
my $server = inet_ntoa($tmp_name);
return ($server, $server);
return ( $server, $server );
}
my $node_server = $nrent && $nrent->{xcatmaster} ? $nrent->{xcatmaster} : undef;
if ($node_server) {
my $tmp_server = inet_aton($node_server);
if ($tmp_server) {
my $server = inet_ntoa($tmp_server);
return ($server, $server);
if ( $nrent and $nrent->{xcatmaster} ) {
my $tmp_server = inet_aton( $nrent->{xcatmaster} );
unless ($tmp_server) {
$callback->({ error => ["Unable to resolve the tftpserver for $node, verify \"xcatmaster\" is set correctly"], errorcode => [1] });
return;
}
my $server = inet_ntoa($tmp_server);
return ( $server, $server );
}
my @nxtsrvd = xCAT::NetworkUtils->my_ip_facing($node);
unless ($nxtsrvd[0]) {
return ($nxtsrvd[1], $nxtsrvd[1]);
if ( _needs_absolute_next_server($nrent) ) {
my @facing = xCAT::NetworkUtils->my_ip_facing($node);
unless ( $facing[0] ) {
return ( $facing[1], $facing[1] );
}
my $why =
( $facing[0] == 1 )
? $facing[1]
: "Unable to determine the tftpserver for $node, verify \"xcatmaster\" is set correctly";
$callback->({ error => [$why], errorcode => [1] });
return;
}
return ('${next-server}', undef);
return ( '${next-server}', undef );
}
sub kea_boot_for_node
xCAT-test/dhcptest/spec.md
+8 -3
View File
@@ -401,7 +401,7 @@ Scenario: The node is told its own name
A service node serves the racks behind it. Which server a node is sent to is
what makes a hierarchical cluster work, and it is visible in one field.
Source: `dhcp.pm:1000-1045`, `kea_next_server_for_node`, `dhcp.pm:3441`
Source: `next_server_for_node`, which both backends read, and `dhcp.pm:3441`
```gherkin
@S-36
@@ -421,7 +421,9 @@ Scenario: next-server otherwise comes from the subnet
Given a node with neither tftpserver nor xcatmaster
When it discovers
Then siaddr is the tftpserver of the subnet it discovered on
# dhcp.pm:1125 -- '${next-server}' defers to the network-level value.
# '${next-server}' is what a node that named no server of its own is
# given: ISC leaves the subnet statement to answer it, and Kea says
# nothing in the reservation, which comes to the same thing.
@S-39
Scenario: A node's URLs point at the same server as its next-server
@@ -794,7 +796,8 @@ Stated so that a green run is not read as more than it is:
Every row is a place where ISC dhcpd and Kea answered the same frame
differently, or where only one of them answered it at all. They were found by
reading this document against `dhcp.pm` and `BootPolicy.pm` and are enumerated
in VersatusHPC/xcat-internal#175.
in VersatusHPC/xcat-internal#175. Rows 26 and 27 are not in that issue: they
were found by the wire cases, which is what the wire cases are for.
The decision column is now the specification: the scenarios above state it
unconditionally, and the wire cases assert it against both backends. "Already
@@ -828,6 +831,8 @@ the drift was in the spec text, not in xCAT.
| 23 | Adoption without a daemon restart: an ISC/OMAPI property | No restart, both | Restarting mid-discovery drops every other machine being discovered | S-58 | already asserted on both by `run-adoption` |
| 24 | `authoritative`: an ISC directive, nothing cited for Kea | DHCPNAK rather than silence, both | A node that moved rack must be told to start over | S-53 | Kea: `authoritative` on |
| 25 | The whole common-option block sourced only from the ISC generator | Parity, asserted not assumed | An installer that loses its resolver, route, clock or MTU fails late and obscurely | S-44 to S-52 | neither, so far -- now asserted on the wire on both |
| 26 | `noderes.xcatmaster`: read for every node (Kea) vs only for `petitboot` and `onie` (ISC) | Read for every node, both | An operator sets it per node and deliberately; a hierarchical cluster's compute nodes were being sent to the management node on ISC | S-37 | ISC: honour it whatever the netboot method, and put it in siaddr as well as in the URLs built from it |
| 27 | No server named at all: subnet value (ISC) vs `my_ip_facing` (Kea) | The subnet's value, both | The two agree only while the subnet's tftpserver is this machine; `networks.tftpserver` exists precisely to say otherwise | S-38 | Kea: leave `next-server` out of the reservation and let the subnet answer |
## Appendix B: spec review
xCAT-test/unit/dhcp_kea_plugin_intent.t
+79 -11
View File
@@ -549,8 +549,14 @@ foreach my $case (@sysconfig_policy_cases) {
# get a Kea host reservation exactly like a regular compute node. The Kea
# reservation builder loops over every requested node without filtering on
# service-node membership, so kea_build_node_reservations must emit an
# ip/mac/hostname reservation whose next-server is resolved (via
# my_ip_facing) to the management server that serves the node's subnet.
# ip/mac/hostname reservation for it.
#
# This node names no server of its own -- its tftpserver is the
# <xcatmaster> placeholder and it has no xcatmaster -- so the address it is
# sent to is the subnet's, which a reservation states by saying nothing.
# Kea used to fall back to my_ip_facing here, which is a different answer
# from the one ISC gives the same node whenever networks.tftpserver names
# some third machine.
package DHCPKeaResTable;
sub new { my ( $class, $rows ) = @_; return bless { rows => $rows }, $class; }
sub getNodesAttribs {
@@ -608,7 +614,69 @@ foreach my $case (@sysconfig_policy_cases) {
is( $r->{'ip-address'}, '192.168.201.21', 'service node reservation carries the node IP' );
is( $r->{'hw-address'}, '42:d7:c0:a8:c9:15', 'service node reservation carries the node MAC' );
is( $r->{hostname}, 'svc01', 'service node reservation carries the hostname' );
is( $r->{'next-server'}, '192.168.201.20', 'service node reservation next-server resolves to the serving management IP' );
ok( !exists $r->{'next-server'},
'a node that names no server of its own leaves next-server to the subnet' );
}
{
# Where a node is sent, in the order noderes states it. Both backends read
# this one answer; they used to have a copy each and the copies disagreed
# about every row but the first.
no warnings 'redefine';
local *xCAT::NetworkUtils::my_ip_facing = sub { return ( 0, '10.0.0.1' ); };
my @errors;
local $xCAT_plugin::dhcp::callback = sub {
my $resp = shift;
push @errors, @{ $resp->{error} || [] };
};
my @cases = (
[ { tftpserver => '192.0.2.10', xcatmaster => '192.0.2.20' },
[ '192.0.2.10', '192.0.2.10' ],
'the node\'s own tftpserver outranks its xcatmaster',
],
[ { tftpserver => '<xcatmaster>', xcatmaster => '192.0.2.20' },
[ '192.0.2.20', '192.0.2.20' ],
'the <xcatmaster> placeholder defers to the xcatmaster attribute',
],
[ { netboot => 'xnba', xcatmaster => '192.0.2.20' },
[ '192.0.2.20', '192.0.2.20' ],
'xcatmaster is honoured for every netboot method, not only petitboot and onie',
],
[ { netboot => 'xnba' },
[ '${next-server}', undef ],
'a node naming neither inherits the subnet\'s value',
],
[ {},
[ '${next-server}', undef ],
'and so does a node with no noderes entry to speak of',
],
[ { netboot => 'petitboot' },
[ '10.0.0.1', '10.0.0.1' ],
'petitboot needs an address to build its URL with, so it falls back to the facing interface',
],
[ { netboot => 'onie' },
[ '10.0.0.1', '10.0.0.1' ],
'and so does onie',
],
);
foreach my $case (@cases) {
my ( $nrent, $want, $why ) = @{$case};
is_deeply( [ xCAT_plugin::dhcp::next_server_for_node( 'n1', $nrent ) ], $want, $why );
}
is_deeply( [ xCAT_plugin::dhcp::next_server_for_node( 'n1', undef ) ],
[ '${next-server}', undef ], 'a node with no noderes row at all inherits the subnet too' );
is( scalar(@errors), 0, 'none of those are an error the operator has to read about' );
# An xcatmaster nobody can resolve is a misconfiguration, and silently
# sending the node somewhere else hides it.
@errors = ();
my @unresolvable = xCAT_plugin::dhcp::next_server_for_node( 'n1', { xcatmaster => 'no.such.host.invalid' } );
is( scalar(@unresolvable), 0, 'an unresolvable xcatmaster yields no address' );
like( ( $errors[0] || '' ), qr/xcatmaster/, 'and says which attribute to look at' );
}
my @normalized_mac_cases = (
@@ -681,7 +749,7 @@ foreach my $case (@invalid_mac_cases) {
return '192.0.2.25';
};
local *xCAT_plugin::dhcp::ipIsDynamic = sub { return 0; };
local *xCAT_plugin::dhcp::kea_next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
local *xCAT_plugin::dhcp::next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
local *xCAT_plugin::dhcp::kea_boot_for_node = sub { return {}; };
my $backend = bless {}, 'DHCPKeaMacBackend';
@@ -787,7 +855,7 @@ foreach my $case (@invalid_mac_cases) {
return $host eq 'valid01' ? '192.0.2.30' : undef;
};
local *xCAT_plugin::dhcp::ipIsDynamic = sub { return 0; };
local *xCAT_plugin::dhcp::kea_next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
local *xCAT_plugin::dhcp::next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
local *xCAT_plugin::dhcp::kea_boot_for_node = sub { return {}; };
local *xCAT::MsgUtils::message = sub { return; };
local *xCAT::MsgUtils::trace = sub { return; };
@@ -856,7 +924,7 @@ foreach my $case (@invalid_mac_cases) {
my ( $class, $name ) = @_;
return $xnba_tables{$name};
};
local *xCAT_plugin::dhcp::kea_next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
local *xCAT_plugin::dhcp::next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
my $classes = xCAT_plugin::dhcp::kea_node_client_classes_for_nodes(['xnba01'])->{classes};
my ($bios_class) = grep { $_->{name} =~ /-bios\z/ } @$classes;
@@ -904,7 +972,7 @@ foreach my $case (@invalid_mac_cases) {
local *xCAT::NetworkUtils::getipaddr = $noip_getipaddr;
local *xCAT_plugin::dhcp::getipaddr = $noip_getipaddr;
local *xCAT_plugin::dhcp::ipIsDynamic = sub { return 0; };
local *xCAT_plugin::dhcp::kea_next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
local *xCAT_plugin::dhcp::next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
local *xCAT_plugin::dhcp::kea_boot_for_node = sub { return {}; };
my @errors;
@@ -963,7 +1031,7 @@ foreach my $case (@invalid_mac_cases) {
my ($ip) = @_;
return $ip eq '192.0.2.150' || $ip eq '2001:db8::150';
};
local *xCAT_plugin::dhcp::kea_next_server_for_node = sub { return; };
local *xCAT_plugin::dhcp::next_server_for_node = sub { return; };
local *xCAT_plugin::dhcp::kea_boot_for_node = sub { return {}; };
local *xCAT::MsgUtils::message = sub { return; };
local *xCAT::MsgUtils::trace = sub { return; };
@@ -1255,7 +1323,7 @@ foreach my $case (@invalid_mac_cases) {
my ( $class, $name ) = @_;
return $tables{$name};
};
local *xCAT_plugin::dhcp::kea_next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
local *xCAT_plugin::dhcp::next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
my $classes = xCAT_plugin::dhcp::kea_node_client_classes_for_nodes( [ 'smp01', 'san01' ] )->{classes};
my %by_name = map { $_->{name} => $_ } @$classes;
@@ -1321,7 +1389,7 @@ foreach my $case (@invalid_mac_cases) {
my ( $class, $name ) = @_;
return $tables{$name};
};
local *xCAT_plugin::dhcp::kea_next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
local *xCAT_plugin::dhcp::next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
my $config = { Dhcp4 => { 'client-classes' => [] } };
ok( xCAT_plugin::dhcp::kea_sync_node_client_classes( $config, [ 'cn01', 'cn02' ] ),
@@ -1432,7 +1500,7 @@ foreach my $case (@invalid_mac_cases) {
my ( $class, $name ) = @_;
return $tables{$name};
};
local *xCAT_plugin::dhcp::kea_next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
local *xCAT_plugin::dhcp::next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
local *xCAT_plugin::dhcp::proxydhcp = sub { return 1; };
my $classes = xCAT_plugin::dhcp::kea_node_client_classes_for_nodes( [ 'booted', 'win01' ] )->{classes};