diff --git a/xCAT-server/lib/xcat/plugins/nodestat.pm b/xCAT-server/lib/xcat/plugins/nodestat.pm index 65f476022..e9fa159d2 100644 --- a/xCAT-server/lib/xcat/plugins/nodestat.pm +++ b/xCAT-server/lib/xcat/plugins/nodestat.pm @@ -116,6 +116,13 @@ sub getstat { =cut #------------------------------------------------------- +sub option_spec { + return ( + 'm|usemon|use|us', 'q|quiet', 'u|updatedb', 'p|powerstat', + 'f|usefping|useping', 'h|help', 'v|version', + ); +} + sub preprocess_request { my $req = shift; @@ -135,26 +142,23 @@ sub preprocess_request } # parse the options - $::UPDATE = 0; - $::QUIET = 0; - $::MON = 0; - $::POWER = 0; - #Getopt::Long::Configure("posix_default"); #Getopt::Long::Configure("no_gnu_compat"); Getopt::Long::Configure("bundling"); $Getopt::Long::ignorecase = 0; - if (!GetOptions( - 'm|usemon' => \$::MON, - 'q|quiet' => \$::QUIET, #this is a internal flag used by monitoring - 'u|updatedb' => \$::UPDATE, - 'p|powerstat' => \$::POWER, - 'h|help' => \$::HELP, - 'v|version' => \$::VERSION)) + my %opt; + if (!GetOptions(\%opt, option_spec())) { &usage($cb,1); return (1); } + $::MON = $opt{m} ? 1 : 0; + $::QUIET = $opt{q} ? 1 : 0; #this is a internal flag used by monitoring + $::UPDATE = $opt{u} ? 1 : 0; + $::POWER = $opt{p} ? 1 : 0; + $::USEFPING = $opt{f} ? 1 : 0; + $::HELP = $opt{h}; + $::VERSION = $opt{v}; if ($::HELP) { &usage($cb); return (0); @@ -922,9 +926,9 @@ sub process_request { my $usefping; if (ref $request->{arg}) { @ARGV = @{ $request->{arg} }; - GetOptions( - 'f|useping' => \$usefping - ); + my %opt; + GetOptions(\%opt, option_spec()); + $usefping = $opt{f}; } diff --git a/xCAT-test/unit/nodestat_usefping_option.t b/xCAT-test/unit/nodestat_usefping_option.t new file mode 100644 index 000000000..c460d692b --- /dev/null +++ b/xCAT-test/unit/nodestat_usefping_option.t @@ -0,0 +1,79 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use FindBin; +use File::Spec; +use Test::More; +use Getopt::Long qw(GetOptionsFromArray); + +my $plugin = File::Spec->catfile( $FindBin::Bin, '..', '..', + 'xCAT-server', 'lib', 'xcat', 'plugins', 'nodestat.pm' ); +plan skip_all => 'nodestat.pm not found' unless -r $plugin; + +open( my $fh, '<', $plugin ) or die "Unable to read $plugin: $!"; +my $source = do { local $/; <$fh> }; +close($fh); + +# nodestat parses its arguments twice, once in the preprocessor and once in the +# handler. Both have to read the same specification, or an option that one +# accepts is dropped or misread by the other. +my $calls = () = $source =~ /GetOptions\s*\(\s*\\%opt,\s*option_spec\(\)\s*\)/g; +is( $calls, 2, 'both places parse through the one specification' ); + +my ($routine) = $source =~ /(sub option_spec \{.*?\n\}\n)/s; +BAIL_OUT('could not extract option_spec from nodestat.pm') unless $routine; +eval "package NodestatSpec; $routine 1;" or BAIL_OUT("could not evaluate: $@"); + +# The daemon leaves Getopt::Long in pass_through, because xCAT::Usage sets it +# and the setting lasts for the life of the process. +sub parse { + my (@argv) = @_; + Getopt::Long::ConfigDefaults(); + Getopt::Long::Configure( 'pass_through', 'bundling' ); + $Getopt::Long::ignorecase = 0; + my %opt; + do { local $SIG{__WARN__} = sub { }; GetOptionsFromArray( \@argv, \%opt, NodestatSpec::option_spec() ) }; + return \%opt; +} + +# The option that the manual page and the usage message give. +foreach my $given (qw(-f --usefping)) { + ok( parse($given)->{f}, "$given selects fping" ); +} + +# The spelling that the code has carried since 2.14.2. A site can have it in a +# script, so it keeps working. +ok( parse('--useping')->{f}, '--useping still selects fping' ); + +# usemon owns these abbreviations. They were unambiguous before the fping +# option gained a long name beginning with the same letters, and an +# administrator who monitors with them must not silently lose monitoring. +foreach my $given (qw(--use --us --usemon -m)) { + my $opt = parse($given); + ok( $opt->{m}, "$given still selects usemon" ); + ok( !$opt->{f}, "$given does not select fping" ); +} + +# Short options bundle, so both orders have to give both settings. +foreach my $given (qw(-mf -fm)) { + my $opt = parse($given); + ok( $opt->{m} && $opt->{f}, "$given selects usemon and fping" ); +} + +# The other options keep their own letters. +is_deeply( + [ map { parse($_) } qw(-u -p -q) ], + [ { u => 1 }, { p => 1 }, { q => 1 } ], + 'the remaining options are unchanged' +); + +# The usage text is what an administrator types. +my ($usage) = $source =~ /(nodestat \[noderange\][^"]*)/; +ok( defined $usage, 'the usage message was found' ); +my ($long) = grep { /^f\|/ } NodestatSpec::option_spec(); +($long) = $long =~ /^f\|([^|]+)/; +ok( defined $usage && $usage =~ /--\Q$long\E/, + 'the usage message names the option that the code accepts' ); + +done_testing();