If no Firecracker process with `self.firecracker_pid` is found, this can
have two reasons:
- Firecracker died unexpectedly. In this case, we want to log a test
  failure
- Firecracker exited expectedly (say, in a negative test for CLI
  arguments). In this case, we want to be explicit about that in the
  test (by setting `self._killed` to `True`, or similar).

Thus, just silently logging an error message in this scenario is
undesirable, as it hides genuine errors. Instead, log the firecracker
process log_data and re-raise the exception. Follow up commits will fix
up the second case above.

Signed-off-by: Patrick Roy <[email protected]>

There's library methods for killing processes in python, so let's just
use those instead of shelling out.

Also move the `os.kill` call into the try-except block, to get logging
in case Firecracker unexpected died before we could kill it.

The only change in behavior is that we no longer silently ignore the
case where we fail to kill Firecracker because it is already dead.

Signed-off-by: Patrick Roy <[email protected]>

The comment about "it being too late to assert" was outdated, as it only
applied back when `kill` was called inside of destructors, and before we
configured pytest to stop ignoring exceptions in destructors.

Nowadays, we call `kill` inside of test teardown, were it is very
possible indeed to raise exceptions. So do that.

Signed-off-by: Patrick Roy <[email protected]>

- utils.check_output already asserts `rc == 0`
- `output.find("firecracker") == -1` is an unidiomatic version of
  "firecracker" not in output.

Signed-off-by: Patrick Roy <[email protected]>

The open-coded subprocess.Popen sequence in Microvm.spawn was equivalent
to `utils.check_output(..., shell=False)`, so just use that.

Signed-off-by: Patrick Roy <[email protected]>

Call this method on microvms that have met an untimely demise during
test execution (for example, because we are specifically testing that
firecracker exits correctly after receiving a signal).

The function raises an exception if the Firecracker process is not
actually dead, to avoid leaking processes.

Signed-off-by: Patrick Roy <[email protected]>

This is so that we do not try to kill them again in `Microvm.kill`,
which would result in a test failure.

Also remove various variants of "wait for Firecracker process to go
away", as `mark_killed` has a `wait_for_termination` inside of it.

Signed-off-by: Patrick Roy <[email protected]>

Use a library function instead of calling out to a shell.

Signed-off-by: Patrick Roy <[email protected]>

In `test_signals.py::test_sigxfsz_handler`, we seem to need to wait on
average 6 seconds. Prior to unifying all the "wait for FC to die" tests,
this one just had an endless loop, and the retry parameters passed to
`wait_process_termination` which it is using now instead do not provide
sufficient waiting. So simply increase the waiting in
`wait_process_termination` to 10s.

Signed-off-by: Patrick Roy <[email protected]>

Document how we use `depends_on` to make sure that a failure of the
Style step does not prevent all the other integration tests from
running.

Signed-off-by: Patrick Roy <[email protected]>

All tests that formerly set this property now use `mark_killed`. The
only other location that set this value was `kill()` itself, to avoid
errors when kill() ends up being called twice. However, this is already
managed by the `Microvm._killed` attribute, so this guarding is not
necessary. Thus, remove the `expect_kill_by_signal`, as it is useless.

Signed-off-by: Patrick Roy <[email protected]>

A PID is an integer, so return it as one. Having it be a string means it
cannot be directly passed to things such as `os.kill`.

Signed-off-by: Patrick Roy <[email protected]>

Explicitly call out that the whole "grep for jailer_id" dance is a
regression test.

Signed-off-by: Patrick Roy <[email protected]>

Use os.kill in test_serial_io instead of using a shell to invoke `kill`.

Signed-off-by: Patrick Roy <[email protected]>