Service.is_valid always returns invalid because of unrelated issues

[coofercat]

On a system with some systemd issues, the outcome of host.service('myservice').is_valid is always False, even if my service is actually good.

On further investigation the original pull request to add this #265 added a check to make sure STDOUT and STDERR were emtpy, as the call to systemd-analyze verify %s always returns 0. However, in come cases, STDERR can contain unrelated information which trips up the comparison.

As an example, I have a deliberate typo in my service unit. The output of systemd-analyze verify myservice.service is as follows (all on STDERR):

snap-core18-2632.mount: Unit is bound to inactive unit dev-loop1.device. Stopping, too.
snap-snapd-18357.mount: Unit is bound to inactive unit dev-loop5.device. Stopping, too.
snap-snapd-17883.mount: Unit is bound to inactive unit dev-loop4.device. Stopping, too.
snap-core20-1828.mount: Unit is bound to inactive unit dev-loop7.device. Stopping, too.
/lib/systemd/system/myservice.service:17: Missing '=', ignoring line.

If I correct the typo, then the last line of this output goes away, but the others remain (actually, on my system the others seem to change every time I run the verify, but I always get at least a couple of lines out of it).

I'm not sure what's up with my system that it should do this (or why systemd would behave this way) but I'm wondering what the best solution to the testinfra problem is. I'm thinking a sort of "grep" of the output for myservice.service would suffice? Something like:

stderr = "".join(filter(lambda x: name in x, cmd.stderr.split()))

Or in place in the function in testinfra/modules/service.py (also taking the opportunity to add some output to explain the reason for the failure):

   @property
    def is_valid(self):
        # systemd-analyze requires a full path.
        if self.name.endswith(".service"):
            name = self.name
        else:
            name = self.name + ".service"
        cmd = self.run("systemd-analyze verify %s", name)
        # A bad unit file still returns a rc of 0, so check the
        # stdout for anything.  Nothing means no warns/errors.
        # Docs at https://www.freedesktop.org/software/systemd/man/systemd
        # -analyze.html#Examples%20for%20verify
        stderr = "".join(filter(lambda x: name in x, cmd.stderr.split()))
        assert (cmd.stdout, stderr) == ("", ""), "service %s is not valid (see 'systemd-analyze verify %s' for details)" % (name, name)
        return True

Happy to hear other suggestions or ideas etc :-)

[CarstenGrohmann]

Starting with Systemd version 250 system-analyze verify has a new option --recursive-errors to control the return code. Additionally it looks like system-analyze verify always returns an error "when syntax warnings arise during verification of the specified units or any of their dependencies.".

See: systemd/systemd@3cc3dc7

[CarstenGrohmann]

@coofercat: Please test this change. You can activate in in your conftest.py with

def new_is_valid(self):
    # systemd-analyze requires a full unit name.
    name = self.name if self._has_systemd_suffix() else f"{self.name}.service"
    cmd = self.run("systemd-analyze verify -- %s", name)
    # A bad unit file still returns a rc of 0, so check the
    # stdout for anything.  Nothing means no warns/errors.
    # Docs at https://www.freedesktop.org/software/systemd/man/systemd
    # -analyze.html#Examples%20for%20verify

    # Ignore non-relevant messages from the output of "systemd-analyze
    # verify":
    #   "Unit is bound to inactive unit"
    #   "ssh.service: Command 'man sshd(8)' failed with code"
    #      --man=no: suppress the man page existence check
    #                implemented in Systemd 235 (2017-10-06)
    #   "Suspicious symlink /etc/systemd/system/[...] treating as alias."
    #    probably a bug in systemd https://github.com/systemd/systemd/issues/30166
    stderr_lines = [
        i
        for i in cmd.stderr.splitlines()
        if "Unit is bound to inactive unit" not in i
        and ": Command 'man" not in i
        and "Suspicious symlink /" not in i
    ]

    stderr = "".join(stderr_lines)
    return (cmd.stdout, stderr) == ("", "")
  

from testinfra.modules.service import SystemdService

SystemdService.is_valid = property(new_is_valid)