Merge pull request #7745 from VersatusHPC/fix/rspconfig-per-bmc-args

fix(rspconfig): accept one network value per BMC

docs/source/guides/admin-guides/references/man1/rspconfig.1.rst

@@ -40,6 +40,8 @@ BMC specific:
 
 \ **rspconfig**\  \ *noderange*\  {\ **ip | netmask | gateway | backupgateway | garp | vlan**\ }
 
+\ **rspconfig**\  \ *noderange*\  {\ **ip | netmask | gateway**\ }=\ *value*\ [,\ *value*\ ...]
+
 \ **rspconfig**\  \ *noderange*\  \ **garp**\ =\ *time*\ 
 
 
@@ -313,6 +315,8 @@ DESCRIPTION
 
 For options \ **autopower | iocap | decfg | memdecfg | procdecfg | time | date | spdump | sysdump | network**\ , user need to use \ *chdef -t site enableASMI=yes*\  to enable ASMI first.
 
+A node whose \ **ipmi.bmc**\  attribute names more than one BMC is configured one BMC at a time, in the order the attribute lists them. Give one value per BMC, separated by commas, when a setting differs between them, for example \ *ip=10.1.1.1,10.1.1.2*\ . A single value is sent to every BMC of the node.
+
 
 *******
 OPTIONS

xCAT-client/pods/man1/rspconfig.1.pod

@@ -20,6 +20,8 @@ B<rspconfig> I<noderange> B<community>={B<public> | I<string>}
 
 B<rspconfig> I<noderange> {B<ip>|B<netmask>|B<gateway>|B<backupgateway>|B<garp>|B<vlan>}
 
+B<rspconfig> I<noderange> {B<ip>|B<netmask>|B<gateway>}=I<value>[,I<value>...]
+
 B<rspconfig> I<noderange> B<garp>=I<time>
 
 =head2 OpenBMC specific:
@@ -266,6 +268,11 @@ B<rspconfig> configures various settings in the nodes' service processors.
 
 For options B<autopower>|B<iocap>|B<decfg>|B<memdecfg>|B<procdecfg>|B<time>|B<date>|B<spdump>|B<sysdump>|B<network>, user need to use I<chdef -t site enableASMI=yes> to enable ASMI first.
 
+A node whose B<ipmi.bmc> attribute names more than one BMC is configured one
+BMC at a time, in the order the attribute lists them. Give one value per BMC,
+separated by commas, when a setting differs between them, for example
+I<ip=10.1.1.1,10.1.1.2>. A single value is sent to every BMC of the node.
+
 =head1 OPTIONS
 
 =over 4

xCAT-server/lib/perl/xCAT/BMCUtils.pm

@@ -0,0 +1,34 @@
+package xCAT::BMCUtils;
+
+use strict;
+use warnings;
+
+my %per_bmc_setting = map { $_ => 1 } qw(ip netmask gateway);
+
+sub rspconfig_bmc_setting {
+    my ( $subcommand, $argument, $bmcnum ) = @_;
+
+    return { argument => $argument }
+      unless defined $subcommand
+      and $per_bmc_setting{$subcommand}
+      and defined $argument
+      and $argument =~ /,/x;
+
+    if (not defined $bmcnum or $bmcnum !~ /^\d+$/x or $bmcnum < 1) {
+        $bmcnum = 1;
+    }
+    my @arguments = split /,/x, $argument, -1;
+    my $value = $arguments[ $bmcnum - 1 ];
+    if (not defined $value or $value eq q{}) {
+        return {
+            error => "The value $argument does not carry a setting for BMC $bmcnum, give one value per BMC",
+        };
+    }
+
+    return {
+        argument           => $value,
+        session_subcommand => "$subcommand=$value",
+    };
+}
+
+1;

xCAT-server/lib/xcat/plugins/ipmi.pm

@@ -19,6 +19,7 @@ use xCAT::GlobalDef;
 use xCAT_monitoring::monitorctrl;
 use xCAT::SPD qw/decode_spd/;
 use xCAT::IPMI;
+use xCAT::BMCUtils;
 use xCAT::PasswordUtils;
 use File::Basename;
 my %needbladeinv;
@@ -895,6 +896,20 @@ sub setnetinfo {
     if ($subcommand eq "thermprofile") {
         return idpxthermprofile($argument);
     }
+
+    my $bmcsetting = xCAT::BMCUtils::rspconfig_bmc_setting(
+        $subcommand, $argument, $sessdata->{bmcnum} );
+    if ($bmcsetting->{error}) {
+        $callback->(
+            { errorcode => [1], error => [ $bmcsetting->{error} ] } );
+        return;
+    }
+    if ($bmcsetting->{session_subcommand}) {
+        $argument = $bmcsetting->{argument};
+
+        # Follow-up callbacks read the subcommand from the session.
+        $sessdata->{subcommand} = $bmcsetting->{session_subcommand};
+    }
     if ($subcommand eq "alert" and ($argument =~ /^(on|en|enable|enabled)$/i)) {
         $netfun = 0x4;
         @cmd = (0x12, 0x9, 0x1, 0x18, 0x11, 0x00);

xCAT-test/unit/rspconfig_per_bmc_argument.t

@@ -0,0 +1,77 @@
+#!/usr/bin/env perl
+use strict;
+use warnings;
+
+use FindBin;
+use File::Spec;
+use Test::More;
+use lib File::Spec->catdir( $FindBin::Bin, '..', '..',
+    'xCAT-server', 'lib', 'perl' );
+use xCAT::BMCUtils;
+
+sub setting {
+    my ( $subcommand, $argument, $bmcnum ) = @_;
+    return xCAT::BMCUtils::rspconfig_bmc_setting(
+        $subcommand, $argument, $bmcnum );
+}
+
+sub pick {
+    my ( $argument, $bmcnum ) = @_;
+    return setting( 'ip', $argument, $bmcnum )->{argument};
+}
+
+my $per_bmc = {};
+foreach my $subcommand (qw(ip netmask gateway community snmpdest alert garp thermprofile)) {
+    my $selection = setting( $subcommand, 'first,second', 1 );
+    $per_bmc->{$subcommand} = exists $selection->{session_subcommand};
+}
+
+# A single value reaches every BMC, which is what a node with one BMC needs
+# and what every existing command line looks like.
+is( pick( '10.0.0.5', 1 ), '10.0.0.5', 'a single value serves the first BMC' );
+is( pick( '10.0.0.5', 2 ), '10.0.0.5', 'a single value serves a later BMC too' );
+
+# A list gives one setting per BMC, in session order.
+is( pick( '10.0.0.5,10.0.0.6', 1 ), '10.0.0.5', 'the first BMC takes the first value' );
+is( pick( '10.0.0.5,10.0.0.6', 2 ), '10.0.0.6', 'the second BMC takes the second value' );
+is( pick( '10.0.0.5,10.0.0.6,10.0.0.7', 3 ), '10.0.0.7', 'the third BMC takes the third value' );
+
+# A list too short reports nothing rather than an empty setting, so the caller
+# can tell the operator instead of writing a blank value into a BMC.
+is( pick( '10.0.0.5,10.0.0.6', 3 ), undef, 'a list shorter than the BMC number yields nothing' );
+
+# An empty element carries no setting, so it reads as missing rather than
+# reaching the address encoders, which reject an empty string outright.
+is( pick( '10.0.0.5,', 2 ), undef, 'an empty element yields nothing' );
+is( pick( ',10.0.0.6', 1 ), undef, 'a leading empty element yields nothing' );
+
+# Defensive input.
+is( pick( undef, 1 ), undef, 'an undefined argument stays undefined' );
+is( pick( '10.0.0.5,10.0.0.6' ), '10.0.0.5', 'a missing BMC number reads as the first' );
+is( pick( '10.0.0.5,10.0.0.6', 0 ), '10.0.0.5', 'a zero BMC number reads as the first' );
+
+# The netmask and gateway settings travel the same path.
+is( pick( '255.255.255.0,255.255.254.0', 2 ), '255.255.254.0', 'a netmask list follows the BMC number' );
+
+# Only the settings the man page advertises take a list. Every other
+# subcommand keeps its argument whole, because a comma can belong to the value,
+# as it does in a free form SNMP community string.
+ok( $per_bmc->{$_},  "$_ takes one value per BMC" )      for qw(ip netmask gateway);
+ok( !$per_bmc->{$_}, "$_ keeps its argument whole" )     for qw(community snmpdest alert garp thermprofile);
+
+my $selected = setting( 'ip', '10.0.0.5,10.0.0.6', 2 );
+is( $selected->{session_subcommand}, 'ip=10.0.0.6',
+    'the follow-up subcommand holds the value of its own BMC' );
+ok( !exists $selected->{error}, 'a complete list reports no mismatch' );
+
+my $short = setting( 'gateway', '10.0.0.1,10.0.0.2', 3 );
+like( $short->{error}, qr/does not carry a setting for BMC 3/,
+    'a short list reports which BMC has no setting' );
+
+my $thermal = setting( 'thermprofile', 'balanced,maximum', 2 );
+is( $thermal->{argument}, 'balanced,maximum',
+    'the thermal profile keeps its argument whole' );
+ok( !exists $thermal->{session_subcommand},
+    'the thermal profile does not rewrite the session' );
+
+done_testing();