Merge pull request #328 from stdweird/metaconfig_reload

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,23 +18,81 @@ 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;
 
-# Given metaconfigservice C<$srv> and C<CAF::FileWriter> instance C<$fh>, 
-# check if a daemon needs to be restarted. 
-sub needs_restarting
+# Given metaconfigservice C<$srv> for C<$file> and hash-reference C<$actions>, 
+# prepare the actions to be taken for this service/file.
+# Does not return anything.
+sub prepare_action
 {
-    my ($self, $fh, $srv) = @_;
+    my ($self, $srv, $file, $actions) = @_;
+
+    # Not using a hash here to detect and support 
+    # any overlap with legacy daemon-restart config
+    my @daemon_action;
+
+    my $msg = "for file $file";
+
+    if ($srv->{daemons}) {
+        while (my ($daemon, $action) = each %{$srv->{daemons}}) {
+            push(@daemon_action, $daemon, $action);
+        }
+    }
 
-    return $fh->close() && $srv->{daemon} && scalar(@{$srv->{daemon}});
+    if ($srv->{daemon}) {
+        $self->verbose("Deprecated daemon(s) restart via daemon field $msg.");
+        foreach my $daemon (@{$srv->{daemon}}) {
+            if ($srv->{daemons}->{$daemon}) {
+                $self->verbose('Daemon $daemon also defined in daemons field $msg. Adding restart action anyway.');
+            }
+            push(@daemon_action, $daemon, 'restart');
+        }
+    }
+
+    my @acts;    
+    while(my ($daemon,$action) = splice(@daemon_action,0,2)) {
+        if(exists($ALLOWED_ACTIONS{$action})) {
+            $actions->{$action} ||= {};
+            $actions->{$action}->{$daemon} = 1;
+            push(@acts, "$daemon:$action");
+        } else {
+            $self->error("Not a CAF::Service allowed action ",
+                         "$action for daemon $daemon $msg ",
+                         "in profile (component/schema mismatch?).");                
+        }
+    }
+
+    if (@acts) {
+        $self->verbose("Scheduled daemon/action ".join(', ',@acts)." $msg.");
+    } else {
+        $self->verbose("No daemon/action scheduled $msg.");
+    }
+}
+
+# Take the action for all daemons as defined in hash-reference C<$actions>.
+# Does not return anything.
+sub process_actions
+{
+    my ($self, $actions) = @_;
+    while (my ($action, $ds) = each(%$actions)) {
+        my $srv = CAF::Service->new([keys(%$ds)], log => $self);
+        # CAF::Service does all the logging we need
+        $srv->$action();
+    }
 }
 
-# Generate $file, configuring $srv using CAF::TextRender.
+# Generate C<$file>, configuring C<$srv> using CAF::TextRender.
+# Also tracks the actions that need to be taken via the 
+# C<$actions> hash-reference.
+# Returns undef in case of rendering failure, 1 otherwise.
 sub handle_service
 {
-    my ($self, $file, $srv) = @_;
+    my ($self, $file, $srv, $actions) = @_;
 
     my $trd = CAF::TextRender->new($srv->{module},
                                    $srv->{contents},
@@ -61,11 +119,8 @@ sub handle_service
         return;
     }
 
-    if ($self->needs_restarting($fh, $srv)) {
-        foreach my $d (@{$srv->{daemon}}) {
-            $self->{daemons}->{$d} = 1;
-        }
-    }
+    $self->prepare_action($srv, $file, $actions) if ($fh->close());
+
     return 1;
 }
 
@@ -76,15 +131,14 @@ sub Configure
 
     my $t = $config->getElement($PATH)->getTree();
 
+    my $actions = {};
+
     while (my ($f, $c) = each(%{$t->{services}})) {
-        $self->handle_service(unescape($f), $c);
+        $self->handle_service(unescape($f), $c, $actions);
     }
 
-    # 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($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,121 @@
+#!/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 $actions = {};
+my $cmp = NCM::Component::metaconfig->new('metaconfig');
+
+=pod
+
+=head2 Test prepare_action
+
+Test the update of the actions reference for single service
+
+=cut
+
+$actions = {};
+$cmp->prepare_action({'daemon' => ['d1', 'd2']}, "myfile", $actions);
+is_deeply($actions, {"restart" => {"d1" => 1, "d2" => 1}}, 
+            "Daemon restart actions added");
+
+$actions = {};
+$cmp->prepare_action({'daemon' => ['d1', 'd2'], 
+                      'daemons' => {'d1' => 'reload', 
+                                    'd2' => 'restart', 
+                                    'd3' => 'doesnotexist'
+                                    }
+                      }, "myfile", $actions);
+# d2 only once in restart
+# d1 in reload and restart
+# doesnotexist is not allowed
+is_deeply($actions, { "restart" => { "d2" => 1, "d1" => 1}, 
+                      'reload' => {'d1' => 1 }
+                    }, "Daemon restart and daemons actions added");
+
+is($cmp->{ERROR}, 1, '1 error logged due to unsupported action');
+
+=pod
+
+=head2 Test process_action
+
+Test taking actions based on the actions reference
+
+=cut
+
+$cmp->process_actions($actions);
+is($restart, 2, '2 restarts triggered');
+is($reload, 1, '1 reload triggered');
+
+=pod
+
+=head2 Test actions taken via Configure
+
+Test taking actions by whole Configure method
+
+=cut
+
+
+my $cfg_d = get_config_for_profile('actions_daemons');
+my $cfg_nd = get_config_for_profile('actions_nodaemons');
+
+$restart = $reload = 0;
+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;
+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;
+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;
+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();