Skip to content

Commit

Permalink
osutils: Do not hide errors of exec call in run() and run_diag()
Browse files Browse the repository at this point in the history
This change does actually 3 changes.
* It fix the issue poo#81828 [1], as it bypass the race error of
  `ReadWriteProcess(code => sub {})` (see: [2])
  We now use `ReadWriteProcess(execute => '/foo')` as the `code=>sub{}`
  argument is intended to call perl code and not simply exec an
  external command. For instance if `exec` fail, perl throw an
  exception and this exception message is used in `_exit($@)` call,
  which doesn't looks like a sane exit-code. Using `execute=>''` gives us
  the exception directly and we can handle it properly!
* It show the output of the executed process.
* run_diag() show a stacktrace if someone try to run a command which
  does not exists.

[1] https://progress.opensuse.org/issues/81828
[2] openSUSE/Mojo-IOLoop-ReadWriteProcess#18
  • Loading branch information
cfconrad committed Nov 22, 2021
1 parent 040c109 commit 260b34e
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 22 deletions.
2 changes: 0 additions & 2 deletions backend/qemu.pm
Original file line number Diff line number Diff line change
Expand Up @@ -716,8 +716,6 @@ sub start_qemu ($self) {
mkpath($basedir);

# do not use autodie here, it can fail on tmpfs, xfs, ...
# potential endless loop on chattr call,
# see https://progress.opensuse.org/issues/81828
run_diag('/usr/bin/chattr', '+C', $basedir);

bmwqemu::diag('Configuring storage controllers and block devices');
Expand Down
37 changes: 20 additions & 17 deletions osutils.pm
Original file line number Diff line number Diff line change
Expand Up @@ -78,28 +78,31 @@ sub quote {
}

sub run {
bmwqemu::diag "running " . join(' ', @_);
my @args = @_;
my $out;
my $buffer;
open my $handle, '>', \$buffer;
my $p = process(sub { local *STDERR = $handle; exec(@args) });
$p->channels(0)->quirkiness(1)->internal_pipes(0)->separate_err(0)->start;
$p->on(stop => sub {
while (defined(my $line = $p->getline)) {
$out .= $line;
}
bmwqemu::diag $buffer if defined $buffer && length($buffer) > 0;
});
$p->wait_stop;
my @cmd = @_;

bmwqemu::diag "running `@cmd`";
my $p = process(execute => shift @cmd, args => [@cmd]);
$p->quirkiness(1)->separate_err(0)->start()->wait_stop();

my $stdout = join('', $p->read_stream->getlines());
chomp $stdout;

close($p->$_ ? $p->$_ : ()) for qw(read_stream write_stream error_stream);

chomp($out) if $out;
return $p->exit_status, $out;
return $p->exit_status, $stdout;
}

# Do not check for anything - just execute and print
sub run_diag { my $o = (run(@_))[1]; bmwqemu::diag($o) if $o; $o }
sub run_diag {
my ($exit_status, $output);
eval {
local $SIG{__DIE__} = undef;
($exit_status, $output) = run(@_);
bmwqemu::diag("Command `@_` terminated with $exit_status" . (length($output) ? "\n$output" : ''));
};
bmwqemu::diag("Fatal error in command `@_`: $@") if ($@);
return $output;
}

# Open a process to run external program and check its return status
sub runcmd {
Expand Down
36 changes: 33 additions & 3 deletions t/13-osutils.t
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use lib "$Bin/../external/os-autoinst-common/lib";
use OpenQA::Test::TimeLimit '5';
use Test::MockModule;
use Test::Warnings qw(:all :report_warnings);
use Test::Output qw(stderr_like);
use Test::Output qw(stderr_like stderr_unlike);

subtest qv => sub {
use osutils 'qv';
Expand Down Expand Up @@ -150,15 +150,45 @@ subtest runcmd => sub {

my @cmd = ('qemu-img', 'create', '-f', 'qcow2', 'image.qcow2', '1G');
my $ret;
stderr_like { $ret = runcmd(@cmd) } qr/running qemu-img/, 'debug runcmd progress output';
stderr_like { $ret = runcmd(@cmd) } qr/running `qemu-img/, 'debug runcmd progress output';
is $ret, 0, "qemu-image creation and get its return code";
stderr_like { $ret = runcmd('rm', 'image.qcow2') } qr/running rm/, 'debug runcmd output with rm';
stderr_like { $ret = runcmd('rm', 'image.qcow2') } qr/running `rm/, 'debug runcmd output with rm';
is $ret, 0, "delete image and get its return code";
local $@;
stderr_like { eval { runcmd('ls', 'image.qcow2') } } qr/No such file or directory/, 'no image found as expected';
like $@, qr/runcmd 'ls image.qcow2' failed with exit code \d+/, "command failed and calls die";
};

subtest run_diag => sub {
use osutils 'run_diag';

stderr_like {
is(run_diag(qw(echo foo)), 'foo', 'Return stdout')
} qr/terminated with 0/, 'Exit code appear in log';

stderr_like {
is(run_diag('echo foo 1>&2'), 'foo', 'Return stderr')
} qr/running `echo/, 'Command appear in log';

stderr_unlike {
is(run_diag('false'), '', 'Empty string, if command does not produce output')
} qr/^\s*$/m, 'No empty line, if command does not produce output';



stderr_like {
run_diag('echo "foo$$bar"');
} qr/foo\d+bar/, 'Output appear in the log';

stderr_like {
run_diag('echo "foo$$bar" 1>&2');
} qr/foo\d+bar/, 'STDERR output appear in the log';

stderr_like {
is(run_diag('/I_do_not_exists'), undef, 'Return undef on execution error and do not die')
} qr/No such file or directory/, 'Error message appear in log';
};

subtest attempt => sub {
use osutils 'attempt';
my $module = Test::MockModule->new('osutils');
Expand Down

0 comments on commit 260b34e

Please sign in to comment.