Skip to content

Commit

Permalink
Implemet PIPESTATUS env var to handle error correctly.
Browse files Browse the repository at this point in the history
As tar is here piped into openssl, we only get the return code for
openssl and lose the one of tar. This patch fixes this issue using
bash PIPESTATUS variable.

The piped command now returns return code of tar if openssl succeeds.
Otherwise it returns return code of openssl. That means, both the
commands should succeed.

Change-Id: I6f9de52539f7284d9479805fc4ca478f2ffd9123
Closes-Bug: #1596587
  • Loading branch information
Partha Bera committed Dec 9, 2016
1 parent 9efe7e4 commit b08c785
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 9 deletions.
7 changes: 4 additions & 3 deletions freezer/engine/tar/tar.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ def backup_data(self, backup_path, manifest_path):

LOG.info("Execution command: \n{}".format(command))

tar_process = subprocess.Popen(command, stdout=subprocess.PIPE,
stderr=subprocess.PIPE, shell=True)
tar_process = subprocess.Popen(
command, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
shell=True, executable='/bin/bash')
read_pipe = tar_process.stdout
tar_chunk = read_pipe.read(self.max_segment_size)
while tar_chunk:
Expand Down Expand Up @@ -124,7 +125,7 @@ def restore_level(self, restore_path, read_pipe, backup, except_queue):

tar_process = subprocess.Popen(
command, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
stderr=subprocess.PIPE, shell=True)
stderr=subprocess.PIPE, shell=True, executable='/bin/bash')
# Start loop reading the pipe and pass the data to the tar
# std input. If EOFError exception is raised, the loop end
# the std err will be checked for errors.
Expand Down
6 changes: 4 additions & 2 deletions freezer/engine/tar/tar_builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ def build(self):
openssl_cmd = "{openssl_path} enc -aes-256-cfb -pass file:{file}"\
.format(openssl_path=self.openssl_path,
file=self.encrypt_pass_file)
tar_command = '{0} | {1}'.format(tar_command, openssl_cmd)
tar_command = '{0} | {1} && exit ${{PIPESTATUS[0]}}'\
.format(tar_command, openssl_cmd)

return tar_command

Expand Down Expand Up @@ -147,7 +148,8 @@ def build(self):
openssl_cmd = self.OPENSSL_DEC.format(
openssl_path=self.openssl_path,
file=self.encrypt_pass_file)
tar_command = '{0} | {1}'.format(openssl_cmd, tar_command)
tar_command = '{0} | {1} && exit ${{PIPESTATUS[0]}}'\
.format(openssl_cmd, tar_command)
return tar_command


Expand Down
11 changes: 7 additions & 4 deletions freezer/tests/unit/engines/tar/test_tar_builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def test_build_every_arg(self):
"--seek --ignore-failed-read --hard-dereference "
"--listed-incremental=listed-file.tar "
"--exclude=\"excluded_files\" . | openssl enc -aes-256-cfb -pass "
"file:encrypt_pass_file")
"file:encrypt_pass_file && exit ${PIPESTATUS[0]}")

def test_build_every_arg_windows(self):
self.builder = tar_builders.TarCommandBuilder(".", "gzip", True,
Expand All @@ -64,7 +64,8 @@ def test_build_every_arg_windows(self):
'gnutar -c -z --incremental --unlink-first --ignore-zeros '
'--hard-dereference --listed-incremental=listed-file.tar '
'--exclude="excluded_files" . '
'| openssl enc -aes-256-cfb -pass file:encrypt_pass_file')
'| openssl enc -aes-256-cfb -pass file:encrypt_pass_file '
'&& exit ${PIPESTATUS[0]}')


class TestTarCommandRestoreBuilder(unittest.TestCase):
Expand All @@ -90,7 +91,8 @@ def test_all_args(self):
self.builder.build(),
"openssl enc -d -aes-256-cfb -pass file:encrypt_pass_file | "
"gnutar -z --incremental --extract --unlink-first --ignore-zeros"
" --warning=none --directory restore_path")
" --warning=none --directory restore_path "
"&& exit ${PIPESTATUS[0]}")

def test_all_args_windows(self):
self.builder = tar_builders.TarCommandRestoreBuilder(
Expand All @@ -99,7 +101,8 @@ def test_all_args_windows(self):
self.assertEqual(
self.builder.build(),
'openssl enc -d -aes-256-cfb -pass file:encrypt_pass_file '
'| gnutar -x -z --incremental --unlink-first --ignore-zeros')
'| gnutar -x -z --incremental --unlink-first --ignore-zeros '
'&& exit ${PIPESTATUS[0]}')

def test_get_tar_flag_from_algo(self):
assert tar_builders.get_tar_flag_from_algo('gzip') == '-z'
Expand Down

0 comments on commit b08c785

Please sign in to comment.