ncm-metaconfig: support daemons with actions

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

@@ -9,11 +9,14 @@ include { 'quattor/schema' };
 
 type ${project.artifactId}_extension = extensible {};
 
+type caf_service_action = string with match(SELF, '^(restart|reload|stop_sleep_start)$');
+
 type ${project.artifactId}_config =  {
      'mode' : long = 0644
      'owner' : string = 'root'
      'group' : string = 'root'
      'daemon' ? string[]
+     'daemons' ? caf_service_action{}
      'module' : string
      'backup' ? string
      'preamble' ? string

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

@@ -18,17 +18,83 @@ use Readonly;
 
 Readonly::Scalar my $PATH => '/software/components/${project.artifactId}';
 
+# Has to correspond to what is allowed in the schema
+Readonly::Hash my %ALLOWED_ACTIONS => { restart => 1, reload => 1, stop_sleep_start => 1 };
+
 our $EC=LC::Exception::Context->new->will_store_all;
 
 our $NoActionSupported = 1;
 
+# Keep track of all actions to be taken in a hash
+# key = action; value = array ref to with all actions
+my %actions = ();
+
+# Convenience method to retrieve actions (mainly for unittests)
+# Returns a reference to the actions hash
+sub get_actions
+{
+    my $self = shift;
+    return \%actions;
+}
+
+# Convenience method to reset actions (mainly for unittests)
+sub reset_actions
+{
+    my $self = shift;
+    %actions = ();
+}
+
+# Add action for daemon
+sub add_action
+{
+    my ($self, $daemon, $action) = @_;
+
+    $actions{$action} = () if (! $actions{$action});
+    if (! grep {$_ eq $daemon} @{$actions{$action}}) {
+        $self->debug(1, "Adding daemon $daemon action with action $action");
+        push(@{$actions{$action}}, $daemon);
+    }
+}
+
 # Given metaconfigservice C<$srv> and C<CAF::FileWriter> instance C<$fh>, 
-# check if a daemon needs to be restarted. 
-sub needs_restarting
+# prepare the actions to be taken for the service service
+sub prepare_action
 {
-    my ($self, $fh, $srv) = @_;
+    my ($self, $srv) = @_;
 
-    return $fh->close() && $srv->{daemon} && scalar(@{$srv->{daemon}});
+    $self->verbose('File changed, looking for daemons and actions');
+    if ($srv->{daemons}) {
+        while (my ($daemon, $action) = each %{$srv->{daemons}}) {
+            $self->add_action($daemon, $action);
+        }
+    }
+
+    if ($srv->{daemon}) {
+        $self->verbose("Deprecated daemon(s) restart via daemon field.");
+        foreach my $daemon (@{$srv->{daemon}}) {
+            if ($srv->{daemons}->{$daemon}) {
+                $self->verbose('Daemon $daemon also defined in daemons field. Adding restart action anyway.');
+            }
+            $self->add_action($daemon, 'restart');
+        }
+    }
+}
+
+# Restart any daemons whose configurations we have changed.
+sub process_actions
+{
+    my $self = shift;
+    while (my ($action, $ds) = each %actions) {
+        my $msg = "action $action for daemons ".join(',', @$ds);
+        my $srv = CAF::Service->new($ds, log => $self);
+        if(exists($ALLOWED_ACTIONS{$action})) {
+            $self->verbose("Taking $msg");                
+            $srv->$action();
+        } else {
+            $self->error("No CAF::Service allowed action $action; no $msg");                
+        }
+    }
+
 }
 
 # Generate $file, configuring $srv using CAF::TextRender.
@@ -61,11 +127,8 @@ sub handle_service
         return;
     }
 
-    if ($self->needs_restarting($fh, $srv)) {
-        foreach my $d (@{$srv->{daemon}}) {
-            $self->{daemons}->{$d} = 1;
-        }
-    }
+    $self->prepare_action($srv) if ($fh->close());
+
     return 1;
 }
 
@@ -76,15 +139,14 @@ sub Configure
 
     my $t = $config->getElement($PATH)->getTree();
 
+    $self->reset_actions();
+
     while (my ($f, $c) = each(%{$t->{services}})) {
         $self->handle_service(unescape($f), $c);
     }
 
-    # Restart any daemons whose configurations we have changed.
-    if ($self->{daemons}) {
-        my $srv = CAF::Service->new([keys(%{$self->{daemons}})], log => $self);
-        $srv->restart();
-    }
+    $self->process_actions();
+
     return 1;
 }
 

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

@@ -5,8 +5,8 @@
 
 =head1 NAME
 
-ncm-${project.artifactId}: Configure services whose config format is
-very widespread, such as YAML or JSON.
+ncm-${project.artifactId}: Configure services whose config format can be 
+rendered via C<CAF::TextRender>.
 
 =head1 DESCRIPTION
 
@@ -42,9 +42,22 @@ Extension for the file's backup.
 Module to render the configuration file. See L<CONFIGURATION MODULES>
 below.
 
-=item * daemon ? string
+=item * daemons ? caf_service_action{}
 
-Daemon to restart if the file changes.
+An nlist with foreach daemon the C<CAF::Service> action to take 
+if the file changes.
+
+Even if multiple C<services> are associated to the same daemon, each action 
+for the daemon will be taken at most once.
+
+If multiple actions are to be taken for the same daemon, all actions
+will be taken (no attempt to optimize is made).
+
+=item * daemon ? string[]
+
+[Deprecated in favour of daemons]
+
+List of daemons to restart if the file changes.
 
 Even if multiple C<services> are associated to the same daemon, the
 daemon will be restarted at most once.
@@ -71,7 +84,7 @@ validation.
 
 =head1 CONFIGURATION MODULES
 
-The following formats my be rendered:
+The following formats can be rendered via C<CAF::TextRender>:
 
 =over 4
 

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

@@ -0,0 +1,104 @@
+#!/usr/bin/perl
+# -*- mode: cperl -*-
+use strict;
+use warnings;
+use Test::More;
+use Test::Quattor qw(actions_daemons actions_nodaemons);
+use Test::MockModule;
+use NCM::Component::metaconfig;
+use CAF::Object;
+use CAF::FileWriter;
+
+$CAF::Object::NoAction = 1;
+
+my $mock = Test::MockModule->new('CAF::Service');
+our ($restart, $reload);
+$mock->mock('restart', sub {
+    my $self = shift;
+    $restart += scalar @{$self->{services}}; 
+});
+$mock->mock('reload', sub {
+    my $self = shift;
+    $reload += scalar @{$self->{services}}; 
+});
+
+my $pretend_changed;
+
+no warnings 'redefine';
+*CAF::FileWriter::close = sub {
+    return $pretend_changed;
+};
+use warnings 'redefine';
+
+=pod
+
+=head1 DESCRIPTION
+
+Test how the need for restarting a service is handled
+
+=cut
+
+
+my $cmp = NCM::Component::metaconfig->new('metaconfig');
+
+$cmp->add_action("daemon1", "action1");
+$cmp->add_action("daemon2", "action1");
+$cmp->add_action("daemon1", "action2");
+
+is_deeply($cmp->get_actions, { "action1" => ["daemon1", "daemon2"], "action2" => ["daemon1"]}, 
+          "Actions added");
+
+$cmp->reset_actions;
+$cmp->prepare_action({'daemon' => ['d1', 'd2']});
+is_deeply($cmp->get_actions, {"restart" => ["d1", "d2"]}, 
+            "Daemon restart actions added");
+
+$cmp->reset_actions;
+$cmp->prepare_action({'daemon' => ['d1', 'd2'], 
+                      'daemons' => {'d1' => 'reload', 
+                                    'd2' => 'restart', 
+                                    'd3' => 'doesnotexist'
+                      }});
+# d2 only once in restart
+# d1 in reload and restart                      
+is_deeply($cmp->get_actions, { "restart" => ["d2", "d1"],  # restart from daemons is processed first
+                              'reload' => ['d1'], 
+                              'doesnotexist' => ['d3']}, 
+            "Daemon restart and daemons actions added");
+
+$cmp->process_actions();
+is($restart, 2, '2 restarts triggered');
+is($reload, 1, '1 reload triggered');
+is($cmp->{ERROR}, 1, '1 error logged due to unsupported action');
+
+my $cfg_d = get_config_for_profile('actions_daemons');
+my $cfg_nd = get_config_for_profile('actions_nodaemons');
+
+$restart = $reload = 0;
+$cmp->reset_actions;
+is($cmp->Configure($cfg_d), 1, 'Configure actions_daemons returned 1');
+is($restart, 0, '0 restarts triggered (daemons configured, no file changes)');
+is($reload, 0, '0 reload triggered (daemons configured, no file changes)');
+
+$restart = $reload = 0;
+$cmp->reset_actions;
+is($cmp->Configure($cfg_nd), 1, 'Configure actions_nodaemons returned 1');
+is($restart, 0, '0 restarts triggered (no daemons configured, no file changes)');
+is($reload, 0, '0 reload triggered (no daemons configured, no file changes)');
+
+# all files are changed files
+$pretend_changed=1;
+
+$restart = $reload = 0;
+$cmp->reset_actions;
+is($cmp->Configure($cfg_d), 1, 'Configure actions_daemons returned 1');
+is($restart, 1, '1 restarts triggered (daemons configured, file changes)');
+is($reload, 1, '1 reload triggered (daemons configured, file changes)');
+
+$restart = $reload = 0;
+$cmp->reset_actions;
+is($cmp->Configure($cfg_nd), 1, 'Configure actions_nodaemons returned 1');
+is($restart, 0, '0 restarts triggered (no daemons configured, file changes)');
+is($reload, 0, '0 reload triggered (no daemons configured, file changes)');
+
+done_testing();

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

@@ -8,15 +8,10 @@ use NCM::Component::metaconfig;
 use Test::MockModule;
 use CAF::Object;
 
-eval { use JSON::XS; };
-
-plan skip_all => "Testing module not found in the system" if $@;
+use JSON::XS;
 
 $CAF::Object::NoAction = 1;
 
-my $s_mock = Test::MockModule->new('CAF::Service');
-$s_mock->mock("os_flavour", "linux_sysv");
-
 my $mock = Test::MockModule->new('NCM::Component::metaconfig');
 
 =pod
@@ -36,15 +31,5 @@ my $fh = get_file("/foo/bar");
 ok($fh, "A file was actually created");
 isa_ok($fh, "CAF::FileWriter");
 
-my $c = get_command("service foo restart");
-ok(!$c, "Daemon was not restarted when there are no changes");
-
-# Pretend there are changes
-
-$mock->mock('needs_restarting', 1);
-
-$cmp->Configure($cfg);
-$c = get_command("service foo restart");
-ok($c, "Daemon was restarted when there were changes");
 
 done_testing();

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

@@ -38,28 +38,13 @@ my $cfg = {
 	   module => "json",
 	  };
 
-my $restart = 1;
-
-no warnings 'redefine';
-*NCM::Component::metaconfig::needs_restarting = sub {
-    return $restart;
-};
-use warnings 'redefine';
-
 $cmp->handle_service("/foo/bar", $cfg);
 
 my $fh = get_file("/foo/bar");
 
 isa_ok($fh, "CAF::FileWriter", "Correct class");
 my $js = JSON::Any->Load("$fh");
 is($js->{foo}, 1, "JSON file correctly created and reread");
-is($cmp->{daemons}->{'httpd'}, 1, "File marks httpd for restarting");
-
-$restart = 0;
-
-$cfg->{daemon} = 'foo';
-
-$cmp->handle_service("/foo/bar", $cfg);
-ok(!exists($cmp->{daemon}->{foo}), "The daemon won't be restarted");
+is($js->{baz}->{a}->[2], 2, "JSON file correctly created and reread (pt 2)");
 
 done_testing();