Skip to content
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.

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

systemd: fix global scope #55936

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
4 participants
@tchernomax
Copy link
Contributor

commented Apr 30, 2019

SUMMARY

before this commit using "global" scope was impossible as sudo systemctl --global show test.service doesn't work (as it doesn't make sense)

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

systemd

Maxime de Roucy
systemd: fix global scope
before this commit using "global" scope was impossible as
`sudo systemctl --global show test.service` doesn't work (as it doesn't
make sense)

@tchernomax tchernomax force-pushed the tchernomax:systemd-global branch from 570e406 to b32a15f May 1, 2019

@tchernomax

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2019

ping @bcoca I fixed the checks

@ansibot ansibot added core_review and removed needs_revision labels May 1, 2019

@bcoca

This comment has been minimized.

Copy link
Member

commented May 1, 2019

I'm not sure this change is correct, nor what it is trying to solve, @tchernomax can you give examples?

needs_info

@ansibot ansibot added needs_info and removed core_review labels May 1, 2019

@tchernomax

This comment has been minimized.

Copy link
Contributor Author

commented May 5, 2019

@bcoca

what I am trying to solve

Without this fix:

max@mde-oxalide % ansible localhost -m systemd -a 'name=dbus-broker.service enabled=true scope=global' -b
localhost | FAILED! => {
    "changed": false,
    "cmd": "/usr/bin/systemctl --global",
    "msg": "Failed to connect to bus: No such file or directory",
    "rc": 1,
    "stderr": "Failed to connect to bus: No such file or directory\n",
    "stderr_lines": [
        "Failed to connect to bus: No such file or directory"
    ],
    "stdout": "",
    "stdout_lines": []
}

with this fix:

max@mde-oxalide % ansible localhost -m systemd -a 'name=dbus-broker.service enabled=true scope=global' -b
localhost | SUCCESS => {
    "changed": false,
    "enabled": true,
    "name": "dbus-broker.service",
    "status": {}
}

why it fail without this fix

    if unit:
        …

        # check service data, cannot error out on rc as it changes across versions, assume not found
        (rc, out, err) = module.run_command("%s show '%s'" % (systemctl, unit))

        if request_was_ignored(out) or request_was_ignored(err):
            # fallback list-unit-files as show does not work on some systems (chroot)
            # not used as primary as it skips some services (like those using init.d) and requires .service/etc notation
            (rc, out, err) = module.run_command("%s list-unit-files '%s'" % (systemctl, unit))
            if rc == 0:
                is_systemd = True

        elif rc == 0:
            # load return of systemctl show into dictionary for easy access and return
            if out:
                result['status'] = parse_systemctl_show(to_native(out).split('\n'))

                is_systemd = 'LoadState' in result['status'] and result['status']['LoadState'] != 'not-found'

                is_masked = 'LoadState' in result['status'] and result['status']['LoadState'] == 'masked'

                # Check for loading error
                if is_systemd and not is_masked and 'LoadError' in result['status']:
                    module.fail_json(msg="Error loading unit file '%s': %s" % (unit, result['status']['LoadError']))
        else:
            # Check for systemctl command
            module.run_command(systemctl, check_rc=True)
        …

The module try to run systemctl --global show …service, which is not possible.
systemctl show … intend to show the status/properties of a unit.
By default systemctl show X == systemctl --system show X, it ask to the system systemd manager the status of it's unit X (system unit, not user unit).
systemctl --user show X ask the current user systemd manager the status of it's unit X (user unit, not system unit).

The --global systemctl parameter is only valid for the enable and disable subcommand ; man systemctl:

--global
When used with enable and disable, operate on the global user configuration directory, thus enabling or disabling a unit file globally for all future logins of all users.

So systemctl --global show … fail.

what I did

rewrite the faulty portion of code to:

  • if possible we use systemctl show
  • else we use systemctl list-unit-files
  • if it doesn't work either check if systemctl exist

what may break (what to check)

AFAIK we can't use systemctl show only when --global and when the module is run in chroot, as this comment suggest:

fallback list-unit-files as show does not work on some systems (chroot)

If there is other condition when we can't use systemctl show my fix should be changed

@ansibot ansibot added core_review and removed needs_info labels May 5, 2019

@bcoca

This comment has been minimized.

Copy link
Member

commented May 6, 2019

I see the issue, but I don't think this is the correct solution, handling the usage of --<scope> correctly seems the better fix

Quick 'top of my head' (needs testing):

index 10c9e42f25..58d358e8a5 100644
--- a/lib/ansible/modules/system/systemd.py
+++ b/lib/ansible/modules/system/systemd.py
@@ -354,8 +354,8 @@ def main():
             module.params['scope'] = 'system'

     # if scope is 'system' or None, we can ignore as there is no extra switch.
-    # The other choices match the corresponding switch
-    if module.params['scope'] not in (None, 'system'):
+    # The other choices match the corresponding switch, but --global only applies to enable/disable
+    if module.params['scope'] not in (None, 'system', 'global'):
         systemctl += " --%s" % module.params['scope']

     if module.params['no_block']:
@@ -451,7 +451,8 @@ def main():

             # do we need to enable the service?
             enabled = False
-            (rc, out, err) = module.run_command("%s is-enabled '%s'" % (systemctl, unit))
+            switch = '--global' if module.params['scope'] == 'global' else ''
+            (rc, out, err) = module.run_command("%s %s is-enabled '%s'" % (systemctl, switch, unit))

             # check systemctl result or if it is a init script
             if rc == 0:```
@tchernomax

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

@bcoca Can you explain (like in pseudo code) what you mean by:

handling the usage of --<scope> correctly

@bcoca

This comment has been minimized.

Copy link
Member

commented May 6, 2019

@tchernomax refresh ticket, i updated my post

@ansibot ansibot added the stale_ci label May 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.