From 7572adaf22178e9478d7b320c2710f62645c82ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Sat, 8 Aug 2026 01:08:04 -0300 Subject: [PATCH 1/3] refactor(destiny): drop the unreachable legacy nodeset path setdestiny reports the deprecated install, netboot and statelite states and returns, and the legacy path that set nodetype.provmethod from the target sits directly after that return. It has not run since the deprecation landed in 57aa41798, and it still reads like live code when following setdestiny. Remove it. The deprecated states are still rejected exactly as before, and the osimage path is untouched. --- xCAT-server/lib/xcat/plugins/destiny.pm | 34 ------------------------- 1 file changed, 34 deletions(-) diff --git a/xCAT-server/lib/xcat/plugins/destiny.pm b/xCAT-server/lib/xcat/plugins/destiny.pm index 2832d7710..76158ab1a 100644 --- a/xCAT-server/lib/xcat/plugins/destiny.pm +++ b/xCAT-server/lib/xcat/plugins/destiny.pm @@ -287,40 +287,6 @@ sub setdestiny { if ($state ne 'osimage') { $callback->({ error => "The options \"install\", \"netboot\", and \"statelite\" have been deprecated, use \"osimage=\" instead.", errorcode => [1], errorabort => [1] }); return; - - my $updateattribs; - if ($target) { - my $archentries = $nodetypetable->getNodesAttribs($req->{node}, ['supportedarchs']); - if ($target =~ /^([^-]*)-([^-]*)-(.*)/) { - $updateattribs->{os} = $1; - $updateattribs->{arch} = $2; - $updateattribs->{profile} = $3; - my $nodearch = $2; - foreach (@{ $req->{node} }) { - if ($archentries->{$_}->[0]->{supportedarchs} and $archentries->{$_}->[0]->{supportedarchs} !~ /(^|,)$nodearch(\z|,)/) { - xCAT::MsgUtils->report_node_error($callback, $_, - "Requested architecture " . $nodearch . " is not one of the architectures supported by $_ (per nodetype.supportedarchs, it supports " . $archentries->{$_}->[0]->{supportedarchs} . ")" - ); - $failurenodes{$_} = 1; - next; - } - } #end foreach - } else { - $updateattribs->{profile} = $target; - } - } #end if($target) - - $updateattribs->{provmethod} = $state; - my @tmpnodelist = (); - foreach (@{ $req->{node} }) { - if ($failurenodes{$_}) { - delete $state_hash{$_}; - next; - } - push @tmpnodelist, $_; - } - $nodetypetable->setNodesAttribs(\@tmpnodelist, $updateattribs); - } else { #state is osimage if ($target) { if (@{ $req->{node} } == 0) { return; } From 2a3af4c34a2df2c3e4c0712d55360b21e796e5de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Sat, 8 Aug 2026 01:08:52 -0300 Subject: [PATCH 2/3] fix(packimage): report the missing image name instead of a rejected option packimage rejects -o, -p and -a before anything else runs, which leaves three things below it that cannot be reached. The branch handling a missing image name asks for -o and reports "Please specify a os version with the -o flag", so passing the option is rejected and omitting it is reported as the option being absent. The check for those options alongside an image name re-tests a condition already rejected, and the block that built an image from them can no longer be entered. Report the missing image name instead, and drop the parts that cannot run. Passing -o, -p or -a is rejected exactly as before, and packing a named image is unchanged. --- xCAT-server/lib/xcat/plugins/packimage.pm | 32 ++--------------------- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/xCAT-server/lib/xcat/plugins/packimage.pm b/xCAT-server/lib/xcat/plugins/packimage.pm index 5c10ea80e..00b36c866 100644 --- a/xCAT-server/lib/xcat/plugins/packimage.pm +++ b/xCAT-server/lib/xcat/plugins/packimage.pm @@ -132,10 +132,6 @@ sub process_request { if (@ARGV > 0) { $imagename = $ARGV[0]; - if ($arch or $osver or $profile) { - $callback->({ error => ["-o, -p and -a options are not allowed when a image name is specified."], errorcode => [1] }); - return 1; - } # load the module in memory eval { require("$::XCATROOT/lib/perl/xCAT/Table.pm") }; @@ -186,21 +182,8 @@ sub process_request { $exlistloc = $ref1->{'exlist'}; $destdir = $ref1->{'rootimgdir'}; } else { - $provmethod = "netboot"; - unless ($osver) { - $callback->({ error => ["Please specify a os version with the -o flag"], errorcode => [1] }); - return 1; - } - unless ($arch) { - $arch = `uname -m`; - chomp($arch); - $arch = "x86" if ($arch =~ /i.86$/); - } - - unless ($profile) { - $callback->({ error => ["Please specify a profile name with -p flag"], errorcode => [1] }); - return 1; - } + $callback->({ error => ["An image name is required, use 'packimage '."], errorcode => [1] }); + return 1; } unless ($destdir) { @@ -233,17 +216,6 @@ sub process_request { return 1; } my $oldpath = cwd(); - unless ($imagename) { - $exlistloc = xCAT::SvrUtils->get_exlist_file_name("$installroot/custom/netboot/$distname", $profile, $osver, $arch); - unless ($exlistloc) { $exlistloc = xCAT::SvrUtils->get_exlist_file_name("$::XCATROOT/share/xcat/netboot/$distname", $profile, $osver, $arch); } - - #save the settings into DB, it will not update if the image already exist - my @ret = xCAT::SvrUtils->update_tables_with_diskless_image($osver, $arch, $profile, "netboot"); - unless ($ret[0] eq 0) { - $callback->({ error => [ "Error when updating the osimage tables: " . $ret[1] ], errorcode => [1] }); - return 1; - } - } #before generating rootimg.$suffix, copy $installroot/postscripts into the image at /xcatpost if (-e "$rootimg_dir/xcatpost") { From 354213e53d3aa596995d0f020ff02843e4f7de05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Sat, 8 Aug 2026 01:08:52 -0300 Subject: [PATCH 3/3] test(packimage): guard against the deprecated paths coming back Scan destiny.pm and packimage.pm for statements sitting after an unconditional return, which is how the deprecated provisioning paths stayed in the tree after they stopped running. Also assert the behavior that has to survive the removal: the deprecated nodeset states are still rejected, packimage still rejects -o, -p and -a, and a missing image name is now reported as such rather than as a missing option that would be rejected anyway. --- .../deprecated_provisioning_reachability.t | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 xCAT-test/unit/deprecated_provisioning_reachability.t diff --git a/xCAT-test/unit/deprecated_provisioning_reachability.t b/xCAT-test/unit/deprecated_provisioning_reachability.t new file mode 100644 index 000000000..b1df7911e --- /dev/null +++ b/xCAT-test/unit/deprecated_provisioning_reachability.t @@ -0,0 +1,84 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use FindBin; +use File::Spec; +use Test::More; + +my $repo_root = File::Spec->catdir( $FindBin::Bin, '..', '..' ); + +sub slurp { + my ($rel) = @_; + my $path = File::Spec->catfile( $repo_root, $rel ); + return unless -r $path; + open( my $fh, '<', $path ) or die "Unable to read $path: $!"; + my $c = do { local $/; <$fh> }; + close($fh); + return $c; +} + +# Report statements that sit after an unconditional return inside the same +# block. Those are unreachable, which is how the deprecated provisioning paths +# survived in the tree for years after they stopped running. +sub unreachable_after_return { + my ($source) = @_; + my @lines = split( /\n/, $source, -1 ); + my @found; + for my $i ( 0 .. $#lines ) { + my ($indent) = $lines[$i] =~ /^(\s*)(?:return\s*;|return\s+\d+\s*;)\s*$/; + next unless defined $indent; + my $depth = length($indent); + for my $j ( $i + 1 .. $#lines ) { + my $next = $lines[$j]; + next if $next =~ /^\s*$/ || $next =~ /^\s*#/; + my ($ni) = $next =~ /^(\s*)/; + last if length($ni) < $depth; + last if $next =~ /^\s*[}\]\)]/; + push @found, ( $j + 1 ) . ": $next"; + last; + } + } + return @found; +} + +my $destiny = slurp('xCAT-server/lib/xcat/plugins/destiny.pm'); +my $packimage = slurp('xCAT-server/lib/xcat/plugins/packimage.pm'); + +plan skip_all => 'destiny.pm or packimage.pm not found' + unless defined($destiny) && defined($packimage); + +my @destiny_dead = unreachable_after_return($destiny); +is_deeply( \@destiny_dead, [], 'destiny.pm has no statements after an unconditional return' ); + +my @packimage_dead = unreachable_after_return($packimage); +is_deeply( \@packimage_dead, [], 'packimage.pm has no statements after an unconditional return' ); + +# The deprecated states must still be rejected. Removing the dead path below the +# rejection must not remove the rejection itself. +like( + $destiny, + qr/have been deprecated, use \\"osimage=/, + 'the deprecated nodeset states are still rejected' +); + +# packimage rejects -o, -p and -a up front, so nothing after that point can ask +# for them again. The old no-imagename branch demanded -o, which could never be +# supplied, and reported that as the error. +unlike( + $packimage, + qr/Please specify a os version with the -o flag/, + 'packimage no longer asks for an option it rejects earlier' +); +like( + $packimage, + qr/-o, -p and -a options are obsoleted/, + 'packimage still rejects the deprecated options' +); +like( + $packimage, + qr/An image name is required/, + 'packimage reports the missing image name instead' +); + +done_testing();