ncm-metaconfig: switch to no-shell and support "ok to fail" commands

ncm-metaconfig/src/main/pan/components/metaconfig/schema.pan

@@ -130,16 +130,22 @@ type ${project.artifactId}_config =  {
     'contents' : ${project.artifactId}_extension
     @{Predefined conversions from EDG::WP4::CCM::TextRender}
     'convert' ? ${project.artifactId}_textrender_convert
-    @{Actions (i.e. named registered commands) to run on pre, validation and/or post step.
-      These are independent of daemons and are executed at time of processing each service.}
+    @{Actions (i.e. names found in /software/components/metadata/commands) to run when processing the service.
+      Refer to the ${project.artifactId}_actions type definition for the available hooks
+      for when a command may be run.}
     'actions' ? ${project.artifactId}_actions
 } = dict();
 
+@{Command must start with absolute path to executable.
+  If the executable is preceded with a '-', it means that the a command failure
+  treated as success w.r.t. reporting and continuation.}
+type ${project.artifactId}_command = string with match(SELF, '^-?/');
+
 type ${project.artifactId}_component = {
     include structure_component
     'services' : ${project.artifactId}_config{} with valid_absolute_file_paths(SELF)
     @{Command registry for allowed actions, keys should be used as action value}
-    'commands' ? string{}
+    'commands' ? ${project.artifactId}_command{}
 } with {
     foreach (esc_fn; srv; SELF['services']) {
         if (exists(srv['actions'])) {

ncm-metaconfig/src/main/perl/metaconfig.pm

@@ -153,6 +153,7 @@ use LC::Exception;
 use EDG::WP4::CCM::TextRender 18.6.1;
 use CAF::Service;
 use EDG::WP4::CCM::Path qw(unescape);
+use Text::ParseWords qw(old_shellwords);
 use Readonly;
 
 # Has to correspond to what is allowed in the schema
@@ -236,11 +237,23 @@ sub run_shell_command
 
     my $command = $commands->{$type};
     if ($command) {
-        $self->debug(1, "Going to run $type command '$command'");
+        my $error_on_fail = 1;
+        if ($command =~ m/^-/) {
+            $command =~ s/^-//;
+            $error_on_fail = 0;
+        }
+        my $cmd_ref = [old_shellwords($command)];
+        if (!$cmd_ref) {
+            $self->error("Failed to split '$command'");
+            return;
+        }
+
+        $self->verbose("Going to run $type command '$command' as ['",
+                       join("','", @$cmd_ref), "']",
+                       $error_on_fail ? "" : " and no error reporting on fail");
 
         my ($err, $out);
         my %opts = (
-            shell => 1,
             log => $self,
             stdout => \$out,
             stderr => \$err,
@@ -252,16 +265,17 @@ sub run_shell_command
             $opts{keeps_state} = 1;
         };
 
-        CAF::Process->new([$command], %opts)->execute();
+        CAF::Process->new($cmd_ref, %opts)->execute();
 
         my $ec = $?;
 
-        my $report = $ec ? 'error' : 'verbose';
+        my $report = $ec && $error_on_fail ? 'error' : 'verbose';
         $self->$report("run $type command '$command' ",
-                         ($ec ? 'failed' : 'ok'),
-                         ,": stdout '$out'\n stderr '$err'",
-                         ($input ? "\n stdin '$input'" : ""));
-        return $ec ? undef : 1;
+                       ($ec ? 'failed' : 'ok'),
+                       ($error_on_fail ? '' : ' (no error on fail set)'),
+                       ,": stdout '$out'\n stderr '$err'",
+                       ($input ? "\n stdin '$input'" : ""));
+        return $ec && $error_on_fail ? undef : 1;
     } else {
         $self->debug(5, "No $type command to run");
         return 1;

ncm-metaconfig/src/test/perl/commands.t

@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 use Test::More;
-use Test::Quattor qw(commands);
+use Test::Quattor qw(commands commands_fail_pre);
 use NCM::Component::metaconfig;
 use Test::MockModule;
 
@@ -23,6 +23,7 @@ sub clean
 
 my $cmp = NCM::Component::metaconfig->new('metaconfig');
 my $cfg = get_config_for_profile('commands');
+my $cfg_fail = get_config_for_profile('commands_fail_pre');
 
 clean();
 
@@ -34,39 +35,48 @@ ok($fh, "A file was actually created");
 isa_ok($fh, "CAF::FileWriter");
 
 # if default sysv init service changes, also modify the aii_command negative test
-ok(command_history_ok(['cmd pre', 'cmd test', 'cmd changed', 'cmd post', 'service foo restart']),
+ok(command_history_ok(['/cmd pre', '/cmd test', '/cmd changed', '/cmd post', 'service foo restart']),
    "commands run and serivce foo restarted");
 
 clean();
 
 # changed failed, post does not run, daemons does run
-set_command_status('cmd changed', 1);
+set_command_status('/cmd changed', 1);
 $cmp->Configure($cfg);
-ok(command_history_ok(['cmd pre', 'cmd test', 'cmd changed', 'service foo restart'], ['cmd post']),
+ok(command_history_ok(['/cmd pre', '/cmd test', '/cmd changed', 'service foo restart'], ['/cmd post']),
    "commands except post run and serivce foo restarted");
 my $fcnt = get_file_contents("/foo/bar");
 ok($orig ne "$fcnt", "orig content is not same as current $fcnt");
 
 clean();
 
 # test failed, changed, post and daemons do not run, content unmodified
-set_command_status('cmd test', 1);
+set_command_status('/cmd test', 1);
 $cmp->Configure($cfg);
-ok(command_history_ok(['cmd pre', 'cmd test'], ['cmd changed', 'service foo restart', 'cmd post']),
+ok(command_history_ok(['/cmd pre', '/cmd test'], ['/cmd changed', 'service foo restart', '/cmd post']),
    "commands run except changed, post and no serivce foo restarted");
 $fcnt = get_file_contents("/foo/bar");
 # unmodified
 is($orig, "$fcnt", "orig content is same as current $fcnt on test failed");
 
 clean();
 # pre failed, test, changed, post and daemons do not run, content unmodified
-set_command_status('cmd pre', 1);
+set_command_status('/cmd pre', 1);
 $cmp->Configure($cfg);
-ok(command_history_ok(['cmd pre'], ['cmd test', 'cmd changed', 'service foo restart', 'cmd post']),
+ok(command_history_ok(['/cmd pre'], ['/cmd test', '/cmd changed', 'service foo restart', '/cmd post']),
    "commands run except test, changed, post and no serivce foo restarted");
 $fcnt = get_file_contents("/foo/bar");
 # unmodified
 is($orig, "$fcnt", "orig content is same as current $fcnt on pre failed");
 
+clean();
+# Rerun with cmd_pre that can fail. Test still fails as usual
+$cmp->Configure($cfg_fail);
+ok(command_history_ok(['/cmd pre', '/cmd test'], ['/cmd changed', 'service foo restart', '/cmd post']),
+   "commands pre fails, but is ok so still runs test, and no changed, post and no serivce foo restarted");
+$fcnt = get_file_contents("/foo/bar");
+# unmodified
+is($orig, "$fcnt", "orig content is same as current $fcnt on pre failed but ok to fail and test fails");
+
 
 done_testing();

ncm-metaconfig/src/test/resources/simple_commands.pan

@@ -3,10 +3,10 @@ unique template simple_commands;
 include 'simple';
 
 prefix "/software/components/metaconfig/commands";
-"cmd_pre" = "cmd pre";
-"cmd_test" = "cmd test";
-"cmd_changed" = "cmd changed";
-"cmd_post" = "cmd post";
+"cmd_pre" = "/cmd pre";
+"cmd_test" = "/cmd test";
+"cmd_changed" = "/cmd changed";
+"cmd_post" = "/cmd post";
 
 prefix "/software/components/metaconfig/services/{/foo/bar}/actions";
 "pre" = "cmd_pre";