Add a check for spurious messages in ssh_command output

The aim of this check is to early detect potential communication
problems through the configured ssh_command.

For example, if the login script on the PostgreSQL server outputs
any messages, this additional output could confuse Barman and
therefore trigger random backup failures.

Signed-off-by: Marco Nenciarini <marco.nenciarini@2ndquadrant.it>
Signed-off-by: Gabriele Bartolini <gabriele.bartolini@2ndQuadrant.it>
Signed-off-by: Leonardo Cecchi <leonardo.cecchi@2ndquadrant.it>

barman/backup_executor.py

@@ -601,16 +601,27 @@ def check(self, check_strategy):
 
         hint = "PostgreSQL server"
         cmd = None
+        minimal_ssh_output = None
         try:
             cmd = UnixRemoteCommand(self.ssh_command,
                                     self.ssh_options,
                                     path=self.server.path)
+            minimal_ssh_output = ''.join(cmd.get_last_output())
         except FsOperationFailed as e:
                 hint = str(e).strip()
 
         # Output the result
         check_strategy.result(self.config.name, 'ssh', cmd is not None, hint)
 
+        # Check if the communication channel is "clean"
+        if minimal_ssh_output:
+            check_strategy.result(
+                self.config.name,
+                'ssh output clean',
+                False,
+                "the configured ssh_command must not add anything "
+                "to the remote command output")
+
         # If SSH works but PostgreSQL is not responding
         if (cmd is not None and
                 self.server.get_remote_status()['server_txt_version']

barman/fs.py

@@ -46,6 +46,14 @@ def __init__(self, path=None):
         # initialize a shell
         self.cmd = Command(cmd='sh -c', shell=True, path=path)
 
+    def get_last_output(self):
+        """
+        Return the output and the error strings from the last executed command
+
+        :rtype: tuple[str,str]
+        """
+        return _str(self.cmd.out), _str(self.cmd.err)
+
     def create_dir_if_not_exists(self, dir_path):
         """
             This method check for the existence of a directory.

tests/test_executor.py

@@ -112,13 +112,33 @@ def test_check(self, command_mock, capsys):
 
         # Test 1: ssh ok
         check_strategy = CheckOutputStrategy()
+        command_mock.return_value.get_last_output.return_value = ('', '')
         backup_manager.executor.check(check_strategy)
         out, err = capsys.readouterr()
         assert err == ''
         assert 'ssh: OK' in out
 
-        # Test 2: ssh ok and PostgreSQL is not responding
+        # Test 2: ssh success, with unclean output (out)
         command_mock.reset_mock()
+        command_mock.return_value.get_last_output.return_value = (
+            'This is unclean', '')
+        backup_manager.executor.check(check_strategy)
+        out, err = capsys.readouterr()
+        assert err == ''
+        assert 'ssh output clean: FAILED' in out
+
+        # Test 2bis: ssh success, with unclean output (err)
+        command_mock.reset_mock()
+        command_mock.return_value.get_last_output.return_value = (
+            '', 'This is unclean')
+        backup_manager.executor.check(check_strategy)
+        out, err = capsys.readouterr()
+        assert err == ''
+        assert 'ssh output clean: FAILED' in out
+
+        # Test 3: ssh ok and PostgreSQL is not responding
+        command_mock.reset_mock()
+        command_mock.return_value.get_last_output.return_value = ('', '')
         check_strategy = CheckOutputStrategy()
         backup_manager.server.get_remote_status.return_value = {
             'server_txt_version': None
@@ -131,7 +151,7 @@ def test_check(self, command_mock, capsys):
         assert "Check that the PostgreSQL server is up and no " \
                "'backup_label' file is in PGDATA."
 
-        # Test 3: ssh failed
+        # Test 4: ssh failed
         command_mock.reset_mock()
         command_mock.side_effect = FsOperationFailed
         backup_manager.executor.check(check_strategy)