Skip to content

Service.is_valid always returns invalid because of unrelated issues #689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
coofercat opened this issue Mar 16, 2023 · 2 comments · May be fixed by #805
Open

Service.is_valid always returns invalid because of unrelated issues #689

coofercat opened this issue Mar 16, 2023 · 2 comments · May be fixed by #805

Comments

@coofercat
Copy link

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
Copy link
Contributor

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 CarstenGrohmann linked a pull request Apr 15, 2025 that will close this issue
Open
@CarstenGrohmann
Copy link
Contributor

@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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ignore useless messages from "systemd-analyze verify" CarstenGrohmann/pytest-testinfra
2 participants
@coofercat @CarstenGrohmann