diff --git a/xCAT-server/lib/xcat/plugins/dhcp.pm b/xCAT-server/lib/xcat/plugins/dhcp.pm index 4f6febaad..77ac981ea 100644 --- a/xCAT-server/lib/xcat/plugins/dhcp.pm +++ b/xCAT-server/lib/xcat/plugins/dhcp.pm @@ -2262,15 +2262,15 @@ sub process_request foreach $syspath ("/etc/sysconfig", "/etc/default") { my $generatedpath = "$syspath/$dhcpver"; - my $dhcpd_key = "DHCPDARGS"; + my @dhcpd_keys = ("DHCPDARGS"); # For SLES11+ and RHEL7+ Operating system releases, the # dhcpd/dhcpd6 configuration is stored in the same file if (dhcpd_sysconfig_uses_interface_key($os)) { - $dhcpd_key = "DHCPD_INTERFACE"; + @dhcpd_keys = ("DHCPD_INTERFACE"); if ($usingipv6 and $dhcpver eq "dhcpd6") { - $dhcpd_key = "DHCPD6_INTERFACE"; + @dhcpd_keys = ("DHCPD6_INTERFACE"); $generatedpath = "$syspath/dhcpd"; } } @@ -2293,7 +2293,8 @@ sub process_request delete($missingfiles{"dhcpd6"}); delete($missingfiles{"dhcp3-server"}); - $dhcpd_key = "INTERFACES"; + @dhcpd_keys = debian_sysconfig_interface_keys( + isc_dhcp_installed_version()); } delete($missingfiles{$dhcpver}); @@ -2304,7 +2305,7 @@ sub process_request close DHCPD_FD; } $syscfg_dhcpd = _sysconfig_interfaces_content( - $syscfg_dhcpd, $dhcpd_key, [ keys %activenics ]); + $syscfg_dhcpd, \@dhcpd_keys, [ keys %activenics ]); # write out the new file with the interfaces defined open DBG_FD, '>', "$generatedpath"; @@ -3162,12 +3163,97 @@ sub dhcpd_sysconfig_uses_interface_key return 0; } +# The isc-dhcp-server revision at which the daemon stopped being launched with +# $INTERFACES and started being launched with $INTERFACESv4 / $INTERFACESv6. +# +# Note this is not an upstream ISC boundary: 20.04 and 22.04 both ship upstream +# 4.4.1 and differ only in the Debian revision, so the comparison has to be made +# against the whole package version. +our $ISC_DHCP_SPLIT_INTERFACES_VERSION = "4.4.1-2.3"; + +# Compare two isc-dhcp-server package versions. Not a general dpkg comparator: +# it compares the runs of digits in the upstream version and then in the Debian +# revision, which is enough to order every version isc-dhcp-server has shipped +# with, and is deliberately kept free of any dependency on dpkg being callable. +sub _isc_dhcp_version_cmp +{ + my ($left, $right) = @_; + + my @sides; + foreach my $version ($left, $right) { + $version =~ s/^\d+://; # epoch plays no part here + my ($upstream, $revision) = ($version, ""); + if ($version =~ /^(.*)-([^-]*)$/) { + ($upstream, $revision) = ($1, $2); + } + push @sides, [ [ $upstream =~ /(\d+)/g ], [ $revision =~ /(\d+)/g ] ]; + } + + foreach my $part (0, 1) { + my @l = @{ $sides[0][$part] }; + my @r = @{ $sides[1][$part] }; + while (@l or @r) { + my $lv = @l ? shift(@l) : 0; + my $rv = @r ? shift(@r) : 0; + return $lv <=> $rv if ($lv != $rv); + } + } + + return 0; +} + +# The version of isc-dhcp-server this machine has installed, or undef when that +# cannot be established. +sub isc_dhcp_installed_version +{ + my $version = `dpkg-query -W -f='\${Version}' isc-dhcp-server 2>/dev/null`; + return unless (defined($version)); + $version =~ s/\s+//g; + return unless (length($version) && $version =~ /\d/); + return $version; +} + +# Which variables /etc/default/isc-dhcp-server has to carry so that dhcpd is +# actually started on the interfaces xCAT is serving. The systemd unit expands +# exactly one of them onto the command line, and which one changed with the +# package: +# +# 14.04 4.2.4-7ubuntu12 sysvinit only $INTERFACES +# 16.04 4.3.3-5ubuntu12 unit $INTERFACES +# 18.04 4.3.5-3ubuntu7 unit $INTERFACES +# 20.04 4.4.1-2.1ubuntu5 unit $INTERFACES +# 22.04 4.4.1-2.3ubuntu2 unit $INTERFACESv4 (v6 unit: v6) +# 24.04 4.4.3-P1-4ubuntu2 unit $INTERFACESv4 (v6 unit: v6) +# 26.04 4.4.3-P1-4ubuntu2 unit $INTERFACESv4 (v6 unit: v6) +# +# The sysvinit script does copy INTERFACES into INTERFACESv4, but nothing on a +# systemd host runs it, so that bridge cannot be relied on. +sub debian_sysconfig_interface_keys +{ + my $version = shift; + $version = isc_dhcp_installed_version() unless (defined($version)); + + # With no version to go on, write both spellings. An unset variable expands + # to nothing and leaves dhcpd binding every interface on the machine, which + # is a far worse outcome than one variable no daemon reads. + unless (defined($version) && length($version) && $version =~ /\d/) { + return ("INTERFACESv4", "INTERFACESv6", "INTERFACES"); + } + + if (_isc_dhcp_version_cmp($version, $ISC_DHCP_SPLIT_INTERFACES_VERSION) >= 0) { + return ("INTERFACESv4", "INTERFACESv6"); + } + + return ("INTERFACES"); +} + # Rewrite the daemon's sysconfig/default file so it names the interfaces xCAT # is serving. Returns the new file contents; the caller writes them out. sub _sysconfig_interfaces_content { - my ($content, $key, $nics) = @_; + my ($content, $keys, $nics) = @_; $content = "" unless defined($content); + $keys = [$keys] unless (ref($keys) eq 'ARRAY'); my $iflist = ""; foreach my $nic (@{$nics}) { @@ -3175,21 +3261,34 @@ sub _sysconfig_interfaces_content $iflist .= " $nic"; } $iflist =~ s/^ //; - my $ifarg = "$key=\"$iflist\"\n"; - my $out = ""; - my $found = 0; - foreach my $line (split /^/, $content) { - if ($line =~ m/^$key/) { - $found = 1; - $out .= $ifarg; - } else { - $out .= $line; + foreach my $key (@{$keys}) { + my $ifarg = "$key=\"$iflist\"\n"; + my $out = ""; + my $found = 0; + + foreach my $line (split /^/, $content) { + + # Anchor on the assignment: INTERFACES is a prefix of INTERFACESv4 + # and INTERFACESv6, and an unanchored match overwrites those lines + # instead of the one it was asked for. + if ($line =~ m/^\s*\Q$key\E\s*=/) { + + # An earlier xCAT release could leave more than one assignment + # behind. Keep the first, drop the rest, so the file ends up + # with exactly one line per variable. + next if ($found); + $found = 1; + $out .= $ifarg; + } else { + $out .= $line; + } } + $out .= $ifarg unless ($found); + $content = $out; } - $out .= $ifarg unless ($found); - return $out; + return $content; } sub kea_ddns_enabled diff --git a/xCAT-test/unit/dhcp_debian_interfaces.t b/xCAT-test/unit/dhcp_debian_interfaces.t index 16ffc2652..97ce4ba86 100644 --- a/xCAT-test/unit/dhcp_debian_interfaces.t +++ b/xCAT-test/unit/dhcp_debian_interfaces.t @@ -76,45 +76,130 @@ sub launched_with { return defined($out) ? $out : ''; } -# makedhcp is serving one provisioning NIC, named in site.dhcpinterfaces. -my $written = xCAT_plugin::dhcp::_sysconfig_interfaces_content( - $stock, 'INTERFACES', ['eth1']); +# Every release in support, the package it ships, and the variable its +# isc-dhcp-server unit expands onto dhcpd's command line. This table is the +# contract: the writer has to satisfy every row. +my @RELEASES = ( + [ '16.04', '4.3.3-5ubuntu12', 'INTERFACES' ], + [ '18.04', '4.3.5-3ubuntu7', 'INTERFACES' ], + [ '20.04', '4.4.1-2.1ubuntu5', 'INTERFACES' ], + [ '22.04', '4.4.1-2.3ubuntu2', 'INTERFACESv4' ], + [ '24.04', '4.4.3-P1-4ubuntu2', 'INTERFACESv4' ], + [ '26.04', '4.4.3-P1-4ubuntu2', 'INTERFACESv4' ], +); -is( launched_with($written, 'INTERFACESv4'), 'eth1', - 'dhcpd is launched restricted to the interface xCAT is serving' ); +sub written_for { + my ($version, $nics, $content) = @_; + my @keys = xCAT_plugin::dhcp::debian_sysconfig_interface_keys($version); + return xCAT_plugin::dhcp::_sysconfig_interfaces_content( + defined($content) ? $content : $stock, [@keys], $nics); +} -isnt( launched_with($written, 'INTERFACESv4'), '', - 'dhcpd is not left to bind every interface on the machine' ); +# makedhcp is serving one provisioning NIC, named in site.dhcpinterfaces. On +# every release, that NIC has to be what dhcpd is launched with. +foreach my $release (@RELEASES) { + my ($ubuntu, $version, $variable) = @{$release}; -# The stock file has no INTERFACES line, so a writer that targets the wrong -# variable does not merely fail to take effect -- its prefix match claims the -# INTERFACESv4 and INTERFACESv6 lines and overwrites both, removing the only -# variables the units read. -like( $written, qr/^\s*INTERFACESv4\s*=/m, - 'the INTERFACESv4 line the unit reads is still present' ); -like( $written, qr/^\s*INTERFACESv6\s*=/m, - 'the INTERFACESv6 line the v6 unit reads is still present' ); + my $written = written_for($version, ['eth1']); -# Serving several interfaces must reach the daemon as several interfaces. -my $multi = xCAT_plugin::dhcp::_sysconfig_interfaces_content( - $stock, 'INTERFACES', ['eth1', 'eth2']); -my @served = sort split /\s+/, launched_with($multi, 'INTERFACESv4'); -is_deeply( \@served, ['eth1', 'eth2'], - 'every served interface reaches dhcpd' ); + is( launched_with($written, $variable), 'eth1', + "$ubuntu ($version): dhcpd is launched restricted to the interface xCAT serves" ); -# A remote (service node) interface is not something this daemon can bind. -my $remote = xCAT_plugin::dhcp::_sysconfig_interfaces_content( - $stock, 'INTERFACES', ['eth1', '!remote!eth9']); -unlike( launched_with($remote, 'INTERFACESv4'), qr/remote/, - 'a !remote! interface is not passed to the local daemon' ); + isnt( launched_with($written, $variable), '', + "$ubuntu ($version): dhcpd is not left to bind every interface on the machine" ); -# An admin or the package's debconf prompt may already have set the variable. -# xCAT's list must win, or site.dhcpinterfaces is decoration. -my $preset = $stock; -$preset =~ s/INTERFACESv4=""/INTERFACESv4="eth0"/; -my $overridden = xCAT_plugin::dhcp::_sysconfig_interfaces_content( - $preset, 'INTERFACES', ['eth1']); -is( launched_with($overridden, 'INTERFACESv4'), 'eth1', - 'a value left by debconf is replaced by the interfaces xCAT serves' ); + # Serving several interfaces must reach the daemon as several interfaces. + my $multi = written_for($version, ['eth1', 'eth2']); + my @served = sort split /\s+/, launched_with($multi, $variable); + is_deeply( \@served, ['eth1', 'eth2'], + "$ubuntu ($version): every served interface reaches dhcpd" ); + + # A remote (service node) interface is not something this daemon can bind. + my $remote = written_for($version, ['eth1', '!remote!eth9']); + unlike( launched_with($remote, $variable), qr/remote/, + "$ubuntu ($version): a !remote! interface is not passed to the local daemon" ); + + # The v6 unit takes $INTERFACES before 22.04 and $INTERFACESv6 after it. + # Leaving its variable empty is how dhcpd6, once the admin enables it, ends + # up bound to everything. + my $v6 = $variable eq 'INTERFACES' ? 'INTERFACES' : 'INTERFACESv6'; + is( launched_with($written, $v6), 'eth1', + "$ubuntu ($version): the v6 unit is restricted to the same interfaces" ); + + # A value the package's debconf prompt or an admin left behind has to lose + # to site.dhcpinterfaces, or the setting is decoration. + my $preset = $stock; + $preset =~ s/^\Q$variable\E=.*$/$variable="eth0"/m + or $preset .= qq{$variable="eth0"\n}; + my $overridden = written_for($version, ['eth1'], $preset); + is( launched_with($overridden, $variable), 'eth1', + "$ubuntu ($version): a value left by debconf is replaced" ); + + # Whichever variable is not the one this release reads must still be left + # alone, not claimed by a prefix match. + foreach my $other (grep { $_ ne $variable } qw(INTERFACESv4 INTERFACESv6)) { + next if ($written =~ m/^\s*\Q$other\E\s*=/m); + fail("$ubuntu ($version): the $other line the package ships was removed"); + } +} + +# With no package version to go on -- dpkg-query unavailable, or a machine that +# is not the one being configured -- err towards writing every spelling. An +# unset variable is what leaves dhcpd bound to everything. +foreach my $unknown (undef, '', 'none') { + my @keys = xCAT_plugin::dhcp::debian_sysconfig_interface_keys($unknown); + my $written = xCAT_plugin::dhcp::_sysconfig_interfaces_content( + $stock, [@keys], ['eth1']); + foreach my $variable (qw(INTERFACES INTERFACESv4 INTERFACESv6)) { + is( launched_with($written, $variable), 'eth1', + 'an unknown package version still restricts dhcpd, via ' . $variable ); + } +} + +# Ordering of the package versions themselves. 20.04 and 22.04 both ship +# upstream 4.4.1 and are told apart only by the Debian revision, so a +# comparison that stops at the upstream version puts them on the wrong side. +is( xCAT_plugin::dhcp::_isc_dhcp_version_cmp('4.4.1-2.1ubuntu5', '4.4.1-2.3ubuntu2'), -1, + '20.04 sorts below 22.04 despite sharing upstream 4.4.1' ); +is( xCAT_plugin::dhcp::_isc_dhcp_version_cmp('4.4.3-P1-4ubuntu2', '4.4.1-2.3ubuntu2'), 1, + '24.04 sorts above 22.04' ); +is( xCAT_plugin::dhcp::_isc_dhcp_version_cmp('4.3.5-3ubuntu7', '4.4.1-2.3'), -1, + '18.04 sorts below the split' ); +is( xCAT_plugin::dhcp::_isc_dhcp_version_cmp('4.4.1-2.3ubuntu2', '4.4.1-2.3ubuntu2'), 0, + 'a version equals itself' ); + +# Debian's own packages have no systemd unit; the sysvinit script reads +# INTERFACESv4 and only falls back to INTERFACES when v4 is empty, so the +# post-split keys are right for them too. +foreach my $debian ('4.4.1-2.3+deb11u2', '4.4.3-P1-2', '4.4.3-P1-8') { + my $written = written_for($debian, ['eth1']); + is( launched_with($written, 'INTERFACESv4'), 'eth1', + "Debian $debian: dhcpd is launched restricted to the interface xCAT serves" ); +} + +# A management node upgraded from an xCAT that wrote the wrong key is left with +# the damage already on disk: no INTERFACESv4 at all and two INTERFACES lines +# where the package's two variables used to be. Running makedhcp again has to +# repair that file, not add to it. +my $damaged = <<'EOF'; +# Defaults for isc-dhcp-server (sourced by /etc/init.d/isc-dhcp-server) +INTERFACES="eth1" +INTERFACES="eth1" +EOF +my $repaired = written_for('4.4.3-P1-4ubuntu2', ['eth2'], $damaged); + +is( launched_with($repaired, 'INTERFACESv4'), 'eth2', + 'a file left behind by an older xCAT is repaired' ); + +foreach my $key (qw(INTERFACESv4 INTERFACESv6)) { + my $count = () = ($repaired =~ m/^\s*\Q$key\E\s*=/mg); + is( $count, 1, "$key is assigned exactly once" ); +} + +# The EL and SLES paths pass a single key and must keep working unchanged. +my $el = xCAT_plugin::dhcp::_sysconfig_interfaces_content( + qq{# Command line options here\nDHCPDARGS=\n}, 'DHCPDARGS', ['eth1', 'eth2']); +is( launched_with($el, 'DHCPDARGS'), 'eth1 eth2', + 'the single-key sysconfig path is unchanged' ); done_testing();