Merge lp:~louis/charms/trusty/rsyslog-forwarder-ha/add_forward_hosts into lp:charms/trusty/rsyslog-forwarder-ha

[Louis Bouchard]

Add 'forward_hosts' parameter to provide a list of rsyslog listeners that are not juju-deployed.
Test coverage also included.

[Jorge Niedbalski (niedbalski)]

Hello Louis,

Thank you for submitting this fix.

I deployed this charm on the following environment:

[Environment]
UPGRADE-AVAILABLE
1.25.6

[Services]
NAME STATUS EXPOSED CHARM
external active false cs:xenial/ubuntu-8
rsyslog-forwarder-ha false local:trusty/rsyslog-forwarder-ha-0
ubuntu active false cs:trusty/ubuntu-8

[Units]
ID WORKLOAD-STATE AGENT-STATE VERSION MACHINE PORTS PUBLIC-ADDRESS MESSAGE
external/0 active idle 1.25.5.1 4 10.5.0.41 ready
ubuntu/3 active idle 1.25.5.1 5 10.5.0.42 ready
  rsyslog-forwarder-ha/0 unknown idle 1.25.5.1 10.5.0.42

[Machines]
ID STATE VERSION DNS INS-ID SERIES HARDWARE
0 started 1.25.5.1 10.5.0.37 d459cd19-5ea2-4d96-b187-13fc8e4117af xenial arch=amd64 cpu-cores=1 mem=1536M root-disk=10240M availability-zone=nova
4 started 1.25.5.1 10.5.0.41 82aac885-f27b-450f-a221-a6c65c68104a xenial arch=amd64 cpu-cores=1 mem=1536M root-disk=10240M availability-zone=nova
5 started 1.25.5.1 10.5.0.42 813f9b99-bdeb-4a6f-8838-546720998853 trusty arch=amd64 cpu-cores=1 mem=1536M root-disk=10240M availability-zone=nova

Then I related ubuntu with rsyslog-forwarder-ha, and then I did set the forward_hosts option:

ubuntu@niedbalski-bastion:~$ juju set rsyslog-forwarder-ha forward_hosts=external=10.5.0.41

The resulting configuration looks like:

$ActionQueueType LinkedList # use asynchronous processing
$ActionQueueFileName external # set file name, also enables disk mode
$ActionResumeRetryCount -1 # infinite retries on insert failure
$ActionQueueSaveOnShutdown on # save in-memory data if rsyslog shuts down

# Forward everything

*.* @10.5.0.41:514

The logs are being forwarder to the remote rsyslog server, I did run the lint/tests
also. All seems good.

LGTM.

review: Approve

=== modified file 'config.yaml'
--- config.yaml	2016-04-26 17:00:59 +0000
+++ config.yaml	2016-10-24 14:10:45 +0000
@@ -31,4 +31,10 @@
     type: string
     default: "/var/log/dpkg.log /var/log/apt/history.log"
     description: "A space separated list of files to follow with rsyslog imfile and forward to the aggregator. If set to empty string no files will be watched with imfile."
-
+  forward_hosts:
+    type: string
+    default: ""
+    description: >
+        A comma-separated list of key=value representing hostname and 
+        ip address of servers to forward all logs to as in
+        hostname1=host_ip1, hostname2=host_ip2, hostname3=host_ip3
 
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2016-08-25 16:06:35 +0000
+++ hooks/hooks.py	2016-10-24 14:10:45 +0000
@@ -52,10 +52,10 @@
 ]
 
 
-IMFILE_FILE = '/etc/rsyslog.d/40-rsyslog-imfile.conf'
+IMFILE_FILE = '/etc/rsyslog.d/75-rsyslog-imfile.conf'
 LOGS_TEMPLATE = 'keep_local.template'
 LOGS_SYSTEM_FILE = '/etc/rsyslog.d/50-default.conf'
-REPLICATION_FILE = '/etc/rsyslog.d/45-rsyslog-replication.conf'
+REPLICATION_FILE = '/etc/rsyslog.d/80-rsyslog-replication.conf'
 
 
 hooks = Hooks()
@@ -123,7 +123,14 @@
 
 
 def update_replication():
+    server_list = config_get('forward_hosts')
     servers = session.query(Server).all()
+    if server_list:
+        for server_pair in server_list.split(','):
+            server = Server()
+            [server.remote_unit,
+             server.private_address] = server_pair.split('=')
+            servers.append(server)
 
     if not len(servers):
         juju_log("Ready for add rsyslog relations to this forwarder")
 
=== modified file 'tests/unit/test_basic.py'
--- tests/unit/test_basic.py	2016-05-19 16:36:12 +0000
+++ tests/unit/test_basic.py	2016-10-24 14:10:45 +0000
@@ -160,7 +160,7 @@
                 return [{}, {}]
 
         self.session.query.return_value = DummyServer()
-        self.config_get.return_value = 'failover'
+        self.config_get.side_effect = ['', 'failover']
 
         hooks.update_replication()
         failover.assert_called_once()
@@ -178,7 +178,86 @@
                 return [{}, {}]
 
         self.session.query.return_value = DummyServer()
-        self.config_get.return_value = 'fanout'
+        self.config_get.side_effect = ['', 'fanout']
+
+        hooks.update_replication()
+        fanout.assert_called_once()
+
+        self.service_restart.assert_called_once_with("rsyslog")
+
+    @mock.patch("hooks.hooks.update_failover_replication")
+    @mock.patch("hooks.hooks.update_fanout_replication")
+    def test_update_replication_fwd_hosts_failover(self, fanout, failover):
+        """check if update_replication works with provided forward_hosts
+           in failover mode"""
+
+        self.config_get.side_effect = ['svr1=ip1, srv2=ip2', 'failover']
+
+        hooks.update_replication()
+        failover.assert_called_once()
+
+        self.service_restart.assert_called_once_with("rsyslog")
+
+    @mock.patch("hooks.hooks.update_failover_replication")
+    @mock.patch("hooks.hooks.update_fanout_replication")
+    def test_update_replication_fwd_one_host_fanout(self, fanout, failover):
+        """check if update_replication switch to fanout if only one provided 
+           forward_hosts in failover mode"""
+
+        self.config_get.side_effect = ['svr1=ip1', 'fanout']
+
+        hooks.update_replication()
+
+        self.juju_log.assert_called_once()
+        failover.assert_called_once()
+        self.sys.exit.assert_called_once()
+        self.service_restart.assert_called_once_with("rsyslog")
+
+    @mock.patch("hooks.hooks.update_failover_replication")
+    @mock.patch("hooks.hooks.update_fanout_replication")
+    def test_update_replication_fwd(self, fanout, failover):
+        """check if update_replication works with provided forward_hosts
+           in fanout mode"""
+
+        self.config_get.side_effect = ['svr1=ip1, srv2=ip2', 'fanout']
+
+        hooks.update_replication()
+        fanout.assert_called_once()
+
+        self.service_restart.assert_called_once_with("rsyslog")
+
+    @mock.patch("hooks.hooks.update_failover_replication")
+    @mock.patch("hooks.hooks.update_fanout_replication")
+    def test_update_replication_fwd_rel_failover(self, fanout, failover):
+        """check if update_replication works with provided forward_hosts
+           and a relation in failover mode"""
+
+        class DummyServer(object):
+            @classmethod
+            def all(self, *args, **kwargs):
+                return [{}]
+
+        self.config_get.side_effect = ['svr1=ip1, srv2=ip2', 'failover']
+        self.session.query.return_value = DummyServer()
+
+        hooks.update_replication()
+        failover.assert_called_once()
+
+        self.service_restart.assert_called_once_with("rsyslog")
+
+    @mock.patch("hooks.hooks.update_failover_replication")
+    @mock.patch("hooks.hooks.update_fanout_replication")
+    def test_update_replication_fwd_rel_fanout(self, fanout, failover):
+        """check if update_replication works with provided forward_hosts
+           and a relation in fanout mode"""
+
+        class DummyServer(object):
+            @classmethod
+            def all(self, *args, **kwargs):
+                return [{}, {}]
+
+        self.config_get.side_effect = ['svr1=ip1, srv2=ip2', 'fanout']
+        self.session.query.return_value = DummyServer()
 
         hooks.update_replication()
         fanout.assert_called_once()