mirror of
https://github.com/xcat2/xcat-core.git
synced 2026-08-03 07:57:00 +00:00
Merge pull request #7708 from VersatusHPC/fix/kea-inconsistent-reservation-state
fix(dhcp): Kea reservation updates leave inconsistent state
This commit is contained in:
Daniel Hilst
GitHub
@@ -630,42 +630,6 @@ sub query_reservations {
|
||||
return \@found;
|
||||
}
|
||||
|
||||
sub preserve_reservations {
|
||||
my ( $self, $config, $intent ) = @_;
|
||||
|
||||
my $target_subnets = $intent->{subnets} || [];
|
||||
return $intent unless @$target_subnets;
|
||||
|
||||
my $target_config = $config->{Dhcp6}
|
||||
? { Dhcp6 => { subnet6 => $target_subnets } }
|
||||
: { Dhcp4 => { subnet4 => $target_subnets } };
|
||||
|
||||
foreach my $source_subnet ( _subnets_for_config($config) ) {
|
||||
foreach my $reservation ( @{ $source_subnet->{reservations} || [] } ) {
|
||||
my $ip = $reservation->{'ip-address'};
|
||||
$ip = $reservation->{'ip-addresses'}[0]
|
||||
if !$ip && ref( $reservation->{'ip-addresses'} ) eq 'ARRAY';
|
||||
|
||||
my $target_subnet;
|
||||
if ($ip) {
|
||||
my $subnet_id = $self->subnet_id_for_ip( $target_config, $ip );
|
||||
$target_subnet = _find_subnet_by_id( $target_config, $subnet_id ) if defined($subnet_id);
|
||||
}
|
||||
if ( !$target_subnet && $source_subnet->{subnet} ) {
|
||||
($target_subnet) = grep {
|
||||
( $_->{subnet} || '' ) eq $source_subnet->{subnet}
|
||||
} @$target_subnets;
|
||||
}
|
||||
next unless $target_subnet;
|
||||
|
||||
$target_subnet->{reservations} ||= [];
|
||||
push @{ $target_subnet->{reservations} }, { %$reservation };
|
||||
}
|
||||
}
|
||||
|
||||
return $intent;
|
||||
}
|
||||
|
||||
sub subnet_id_for_ip {
|
||||
my ( $self, $config, $ip ) = @_;
|
||||
|
||||
|
||||
@@ -2381,24 +2381,6 @@ sub kea_process_request
|
||||
}
|
||||
|
||||
if ($opt->{n}) {
|
||||
my $loaded4 = $backend->load_dhcp4_config();
|
||||
if ($loaded4->{error}) {
|
||||
$callback->({ error => [ $loaded4->{error} ], errorcode => [1] });
|
||||
flock($dhcplockfd, LOCK_UN);
|
||||
return;
|
||||
}
|
||||
$backend->preserve_reservations($loaded4, $intent4);
|
||||
|
||||
if ($using_dhcp6) {
|
||||
my $loaded6 = $backend->load_dhcp6_config();
|
||||
if ($loaded6->{error}) {
|
||||
$callback->({ error => [ $loaded6->{error} ], errorcode => [1] });
|
||||
flock($dhcplockfd, LOCK_UN);
|
||||
return;
|
||||
}
|
||||
$backend->preserve_reservations($loaded6, $intent6);
|
||||
}
|
||||
|
||||
my $result = $backend->write_dhcp4_config($intent4, backup_existing => 1);
|
||||
if ($result->{error}) {
|
||||
$callback->({ error => [ $result->{error} ], errorcode => [1] });
|
||||
@@ -2495,11 +2477,6 @@ sub kea_process_request
|
||||
$client_classes_changed = kea_remove_xnba_client_classes($loaded4, $nodes);
|
||||
} else {
|
||||
$reservations4 = kea_build_node_reservations($backend, $loaded4, $nodes);
|
||||
if (ref($reservations4) eq 'HASH' && $reservations4->{error}) {
|
||||
$callback->({ error => [ $reservations4->{error} ], errorcode => [1] });
|
||||
flock($dhcplockfd, LOCK_UN);
|
||||
return;
|
||||
}
|
||||
$backend->upsert_reservations($loaded4, $reservations4);
|
||||
$client_classes_changed = kea_sync_xnba_client_classes($loaded4, $nodes);
|
||||
if ($loaded6) {
|
||||
@@ -3120,7 +3097,6 @@ sub kea_build_node_reservations
|
||||
my @reservations;
|
||||
foreach my $node (@$nodes) {
|
||||
my $node_reservations = kea_node_reservations($backend, $config, $node);
|
||||
return $node_reservations if ref($node_reservations) eq 'HASH' && $node_reservations->{error};
|
||||
push @reservations, @$node_reservations;
|
||||
}
|
||||
|
||||
@@ -3182,7 +3158,10 @@ sub kea_node_reservations
|
||||
$mac = $normalized_mac;
|
||||
|
||||
my $ip = getipaddr($hname, OnlyV4 => 1);
|
||||
return { error => "Unable to resolve $hname for the Kea DHCP reservation for $node." } unless $ip;
|
||||
unless ($ip) {
|
||||
$callback->({ warning => ["The hostname $hname of node $node could not be resolved."] });
|
||||
next;
|
||||
}
|
||||
|
||||
if (ipIsDynamic($ip)) {
|
||||
$callback->({ error => ["Node $node has IP $ip which is inside the DHCP dynamic range. Move the node IP outside the dynamic range or adjust the range in the networks table."], errorcode => [1] });
|
||||
|
||||
@@ -215,6 +215,77 @@ ok(!xCAT_plugin::dhcp::dhcpd_sysconfig_uses_interface_key('opensuse-tumbleweed')
|
||||
);
|
||||
}
|
||||
|
||||
{
|
||||
package DHCPKeaRegenerateBackend;
|
||||
sub load_dhcp4_config {
|
||||
$_[0]->{loads}++;
|
||||
return { error => 'existing Kea configuration must not be loaded by makedhcp -n' };
|
||||
}
|
||||
sub write_dhcp4_config {
|
||||
my ( $self, $intent, %opts ) = @_;
|
||||
$self->{written_intent} = $intent;
|
||||
$self->{write_options} = \%opts;
|
||||
return {};
|
||||
}
|
||||
sub restart_services {
|
||||
my ( $self, %opts ) = @_;
|
||||
$self->{restart_options} = \%opts;
|
||||
return {};
|
||||
}
|
||||
|
||||
package main;
|
||||
|
||||
my $network_intent = {
|
||||
interfaces => ['eth0'],
|
||||
'client-classes' => [ { name => 'xcat-generic' } ],
|
||||
subnets => [ { id => 1, subnet => '192.0.2.0/24' } ],
|
||||
};
|
||||
|
||||
no warnings 'redefine';
|
||||
local *xCAT_plugin::dhcp::kea_build_dhcp4_intent = sub { return $network_intent; };
|
||||
local *xCAT_plugin::dhcp::kea_build_dhcp6_intent = sub { return { subnets => [] }; };
|
||||
local *xCAT_plugin::dhcp::kea_build_ddns_intent = sub { return; };
|
||||
local *xCAT_plugin::dhcp::kea_control_agent_enabled = sub { return 0; };
|
||||
local *xCAT::MsgUtils::message = sub { return; };
|
||||
local *xCAT::MsgUtils::trace = sub { return; };
|
||||
local $::XCATSITEVALS{externaldhcpservers};
|
||||
|
||||
my @errors;
|
||||
my $capture_response = sub {
|
||||
my $response = shift;
|
||||
push @errors, @{ $response->{error} || [] };
|
||||
};
|
||||
my $saved_umask = umask;
|
||||
my $saved_ignorecase = $Getopt::Long::ignorecase;
|
||||
{
|
||||
local @ARGV;
|
||||
xCAT_plugin::dhcp::process_request(
|
||||
{
|
||||
_xcatpreprocessed => [0],
|
||||
arg => [ '-q', '-a' ],
|
||||
},
|
||||
$capture_response
|
||||
);
|
||||
}
|
||||
umask $saved_umask;
|
||||
$Getopt::Long::ignorecase = $saved_ignorecase;
|
||||
Getopt::Long::Configure('pass_through');
|
||||
@errors = ();
|
||||
|
||||
my $backend = bless { loads => 0 }, 'DHCPKeaRegenerateBackend';
|
||||
xCAT_plugin::dhcp::kea_process_request( $backend, {}, { n => 1 }, { eth0 => 1 }, 0 );
|
||||
|
||||
is( $backend->{loads}, 0, 'makedhcp -n does not parse the previous Kea configuration' );
|
||||
is_deeply(
|
||||
$backend->{written_intent},
|
||||
$network_intent,
|
||||
'makedhcp -n writes only the newly generated network intent'
|
||||
);
|
||||
ok( $backend->{write_options}{backup_existing}, 'makedhcp -n backs up the replaced Kea configuration' );
|
||||
ok( $backend->{restart_options}{enable}, 'makedhcp -n enables and restarts Kea after replacement' );
|
||||
is_deeply( \@errors, [], 'makedhcp -n replacement completes without errors' );
|
||||
}
|
||||
|
||||
{
|
||||
no warnings 'redefine';
|
||||
local *xCAT::NetworkUtils::thishostisnot = sub { return 1; };
|
||||
@@ -495,58 +566,44 @@ foreach my $case (@invalid_mac_cases) {
|
||||
}
|
||||
|
||||
{
|
||||
my %unresolved_tables = (
|
||||
noderes => DHCPKeaResTable->new( { unresolved01 => {} } ),
|
||||
chain => DHCPKeaResTable->new( { unresolved01 => {} } ),
|
||||
nodetype => DHCPKeaResTable->new( { unresolved01 => {} } ),
|
||||
my %lookup_tables = (
|
||||
noderes => DHCPKeaResTable->new(
|
||||
{
|
||||
unresolved01 => {},
|
||||
valid01 => {},
|
||||
}
|
||||
),
|
||||
chain => DHCPKeaResTable->new( {} ),
|
||||
nodetype => DHCPKeaResTable->new( {} ),
|
||||
iscsi => DHCPKeaResTable->new( {} ),
|
||||
mac => DHCPKeaResTable->new( { unresolved01 => { mac => '00:11:22:33:44:55' } } ),
|
||||
mac => DHCPKeaResTable->new(
|
||||
{
|
||||
unresolved01 => { mac => '00:11:22:33:44:55' },
|
||||
valid01 => { mac => '00:11:22:33:44:66' },
|
||||
}
|
||||
),
|
||||
);
|
||||
|
||||
no warnings 'redefine';
|
||||
local *xCAT::Table::new = sub {
|
||||
my ( $class, $name ) = @_;
|
||||
return $unresolved_tables{$name};
|
||||
return $lookup_tables{$name};
|
||||
};
|
||||
local *xCAT_plugin::dhcp::getipaddr = sub { return; };
|
||||
local *xCAT_plugin::dhcp::getipaddr = sub {
|
||||
my ($host) = @_;
|
||||
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' ); };
|
||||
|
||||
my $backend = bless {}, 'DHCPKeaResBackend';
|
||||
my $result = xCAT_plugin::dhcp::kea_build_node_reservations( $backend, {}, ['unresolved01'] );
|
||||
|
||||
is( ref($result), 'HASH', 'unresolved Kea reservation returns an error result' );
|
||||
is(
|
||||
$result->{error},
|
||||
'Unable to resolve unresolved01 for the Kea DHCP reservation for unresolved01.',
|
||||
'unresolved Kea reservation identifies the requested node'
|
||||
);
|
||||
}
|
||||
|
||||
{
|
||||
package DHCPKeaWriteGuardBackend;
|
||||
sub load_dhcp4_config {
|
||||
return { Dhcp4 => { subnet4 => [ { id => 1, subnet => '192.0.2.0/24' } ] } };
|
||||
}
|
||||
sub upsert_reservations { $_[0]->{writes}++; return; }
|
||||
sub write_dhcp4_json { $_[0]->{writes}++; return {}; }
|
||||
|
||||
package main;
|
||||
|
||||
no warnings 'redefine';
|
||||
local *xCAT_plugin::dhcp::kea_build_dhcp4_intent = sub { return { subnets => [] }; };
|
||||
local *xCAT_plugin::dhcp::kea_build_dhcp6_intent = sub { return { subnets => [] }; };
|
||||
local *xCAT_plugin::dhcp::kea_build_ddns_intent = sub { return; };
|
||||
local *xCAT_plugin::dhcp::kea_expand_request_nodes = sub { return ['unresolved01']; };
|
||||
local *xCAT_plugin::dhcp::kea_build_node_reservations = sub {
|
||||
return { error => 'Unable to resolve unresolved01 for the Kea DHCP reservation for unresolved01.' };
|
||||
};
|
||||
local *xCAT_plugin::dhcp::kea_boot_for_node = sub { return {}; };
|
||||
local *xCAT::MsgUtils::message = sub { return; };
|
||||
local *xCAT::MsgUtils::trace = sub { return; };
|
||||
|
||||
my @errors;
|
||||
my $capture_error = sub {
|
||||
my ( @warnings, @errors );
|
||||
my $capture_response = sub {
|
||||
my $response = shift;
|
||||
push @errors, @{ $response->{error} || [] };
|
||||
push @warnings, @{ $response->{warning} || [] };
|
||||
push @errors, @{ $response->{error} || [] };
|
||||
};
|
||||
my $saved_umask = umask;
|
||||
my $saved_ignorecase = $Getopt::Long::ignorecase;
|
||||
@@ -557,23 +614,42 @@ foreach my $case (@invalid_mac_cases) {
|
||||
_xcatpreprocessed => [0],
|
||||
arg => [ '-q', '-a' ],
|
||||
},
|
||||
$capture_error
|
||||
$capture_response
|
||||
);
|
||||
}
|
||||
umask $saved_umask;
|
||||
$Getopt::Long::ignorecase = $saved_ignorecase;
|
||||
Getopt::Long::Configure('pass_through');
|
||||
@errors = ();
|
||||
@warnings = ();
|
||||
@errors = ();
|
||||
|
||||
my $backend = bless { writes => 0 }, 'DHCPKeaWriteGuardBackend';
|
||||
xCAT_plugin::dhcp::kea_process_request( $backend, { node => ['unresolved01'] }, {}, {}, 0 );
|
||||
{
|
||||
package DHCPKeaLookupBackend;
|
||||
sub subnet_id_for_ip { return 1; }
|
||||
}
|
||||
my $backend = bless {}, 'DHCPKeaLookupBackend';
|
||||
my $reservations = xCAT_plugin::dhcp::kea_build_node_reservations(
|
||||
$backend,
|
||||
{},
|
||||
[ 'unresolved01', 'valid01' ]
|
||||
);
|
||||
|
||||
if ( ref($reservations) eq 'HASH' && $reservations->{error} ) {
|
||||
push @errors, $reservations->{error};
|
||||
$reservations = [];
|
||||
}
|
||||
|
||||
is_deeply(
|
||||
\@errors,
|
||||
['Unable to resolve unresolved01 for the Kea DHCP reservation for unresolved01.'],
|
||||
'unresolved Kea reservation is reported to the caller'
|
||||
[ map { $_->{hostname} } @$reservations ],
|
||||
['valid01'],
|
||||
'an unresolved hostname does not block later valid Kea reservations'
|
||||
);
|
||||
is( $backend->{writes}, 0, 'unresolved Kea reservation leaves the configuration unchanged' );
|
||||
is_deeply(
|
||||
\@warnings,
|
||||
['The hostname unresolved01 of node unresolved01 could not be resolved.'],
|
||||
'an unresolved Kea hostname reports the ISC-compatible warning'
|
||||
);
|
||||
is_deeply( \@errors, [], 'an unresolved Kea hostname does not abort the request' );
|
||||
}
|
||||
|
||||
{
|
||||
|
||||
@@ -329,96 +329,6 @@ my $deleted = $backend->delete_reservations( $reservation_config, { 'hw-address'
|
||||
is( scalar @$deleted, 1, 'reservation delete returns deleted reservation' );
|
||||
is( scalar @{ $reservation_config->{Dhcp4}{subnet4}[0]{reservations} }, 0, 'reservation is removed from config' );
|
||||
|
||||
my $existing_dhcp4 = {
|
||||
Dhcp4 => {
|
||||
subnet4 => [
|
||||
{
|
||||
id => 1,
|
||||
subnet => '10.20.0.0/24',
|
||||
reservations => [
|
||||
{
|
||||
'hw-address' => '00:11:22:33:44:66',
|
||||
'ip-address' => '10.20.0.12',
|
||||
hostname => 'node20',
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
id => 2,
|
||||
subnet => '10.30.0.0/24',
|
||||
reservations => [
|
||||
{
|
||||
'hw-address' => '00:11:22:33:44:77',
|
||||
'ip-address' => '10.30.0.12',
|
||||
hostname => 'removed-node',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
my $regenerated_dhcp4 = {
|
||||
subnets => [
|
||||
{
|
||||
id => 9,
|
||||
subnet => '10.20.0.0/24',
|
||||
},
|
||||
],
|
||||
};
|
||||
$backend->preserve_reservations( $existing_dhcp4, $regenerated_dhcp4 );
|
||||
is_deeply(
|
||||
$regenerated_dhcp4->{subnets}[0]{reservations},
|
||||
$existing_dhcp4->{Dhcp4}{subnet4}[0]{reservations},
|
||||
'regeneration preserves IPv4 reservations in the matching subnet'
|
||||
);
|
||||
is(
|
||||
scalar @{ $regenerated_dhcp4->{subnets}[0]{reservations} },
|
||||
1,
|
||||
'regeneration drops reservations for removed IPv4 subnets'
|
||||
);
|
||||
is(
|
||||
$regenerated_dhcp4->{subnets}[0]{id},
|
||||
9,
|
||||
'regeneration does not copy the old IPv4 subnet id'
|
||||
);
|
||||
|
||||
my $existing_dhcp6 = {
|
||||
Dhcp6 => {
|
||||
subnet6 => [
|
||||
{
|
||||
id => 10001,
|
||||
subnet => '2001:db8:20::/64',
|
||||
reservations => [
|
||||
{
|
||||
duid => '00:04:00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff',
|
||||
'ip-addresses' => ['2001:db8:20::12'],
|
||||
hostname => 'node20-v6',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
my $regenerated_dhcp6 = {
|
||||
subnets => [
|
||||
{
|
||||
id => 10009,
|
||||
subnet => '2001:db8:20::/64',
|
||||
},
|
||||
],
|
||||
};
|
||||
$backend->preserve_reservations( $existing_dhcp6, $regenerated_dhcp6 );
|
||||
is_deeply(
|
||||
$regenerated_dhcp6->{subnets}[0]{reservations},
|
||||
$existing_dhcp6->{Dhcp6}{subnet6}[0]{reservations},
|
||||
'regeneration preserves IPv6 reservations in the matching subnet'
|
||||
);
|
||||
is(
|
||||
$regenerated_dhcp6->{subnets}[0]{id},
|
||||
10009,
|
||||
'regeneration does not copy the old IPv6 subnet id'
|
||||
);
|
||||
|
||||
my $hookdir = tempdir(CLEANUP => 1);
|
||||
my $hook = "$hookdir/libdhcp_host_cmds.so";
|
||||
open(my $hookfh, '>', $hook) or die "Unable to create fake hook: $!";
|
||||
|
||||
Reference in New Issue
Block a user