Repository: ansible/ansible
Pull request: Add scope parameter to systemd
##### SUMMARY
<!--- Describe the change, including rationale and design decisions -->
This change changes the user: paramater into a state: paramater with the following choices:
- system
- user
- global

with system being the default. The system scope is the default scope. The user scope matches with the old user: paramater. The global scope applies to all users.

An example use case would be masking a unit. In the system scope it would create the mask via symlink from /etc/systemd/system/. In the user scope it would mask via symlink from ~/.config/systemd/user/. In the global scope it would mask via symlink from /etc/systemd/user/.

Previously a global: paramater was tested but since global is a keyword in python, it caused errors during testing. Since this patch removes the old user: paramater, it is a backwards incompatible change.

<!--- If you are fixing an existing issue, please include "Fixes #nnn" in your
commit message and your description; but you should still explain what
the change does.-->
Fixes #38828
##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Feature Pull Request

##### COMPONENT NAME
<!--- Name of the module, plugin, module or task -->
systemd
##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes below -->
```
ansible 2.5.0
  config file = /home/hi117/.ansible.cfg
  configured module search path = [u'/home/hi117/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.15 (default, May  1 2018, 20:16:04) [GCC 7.3.1 20180406]
```


##### ADDITIONAL INFORMATION
<!--- Include additional information to help people understand the change here.
For bugs that don't have a linked bug report, a step-by-step reproduction
of the problem is helpful. -->

<!--- Paste verbatim command output below, e.g. before and after your change -->
Output of make tests-nonet, snipped to only the relavent tests.
```
test/units/modules/system/test_systemd.py::ParseSystemctlShowTestCase::test_multiline_exec PASSED
test/units/modules/system/test_systemd.py::ParseSystemctlShowTestCase::test_simple PASSED
test/units/modules/system/test_systemd.py::ParseSystemctlShowTestCase::test_single_line_with_brace PASSED 
```


Problem statement:
Add global parameter to the systemd module
<!---
Verify first that your issue/request is not already reported on GitHub.
Also test if the latest release, and devel branch are affected too.
Always add information AFTER of these html comments.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
 - Feature Idea

##### COMPONENT NAME
<!---
Insert below this comment the name of the module, plugin, task or feature.
Do not include extra details here, e.g. "vyos_command" not "the network module vyos_command" or the full path
-->
systemd
##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes below -->
```
ansible 2.5.0
  config file = /home/hi117/.ansible.cfg
  configured module search path = [u'/home/hi117/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.14 (default, Jan  5 2018, 10:41:29) [GCC 7.2.1 20171224]
```

##### CONFIGURATION
<!---
If using Ansible 2.4 or above, paste the results of "ansible-config dump --only-changed"
Otherwise, mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).
-->
DEFAULT_HOST_LIST(/home/hi117/.ansible.cfg) = [u'/home/hi117/ansible/hosts']
DEFAULT_PRIVATE_KEY_FILE(/home/hi117/.ansible.cfg) = /home/hi117/ansible/ansibleKey
DEFAULT_REMOTE_USER(/home/hi117/.ansible.cfg) = ansible
DEFAULT_ROLES_PATH(/home/hi117/.ansible.cfg) = [u'/home/hi117/ansible/roles']

##### OS / ENVIRONMENT
<!---
Mention the OS you are running Ansible from, and the OS you are
managing, or say "N/A" for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.
-->
N/A
##### SUMMARY
<!--- Explain the problem briefly -->
The systemd module does not support the equivalent of the --global flag to systemctl, despite supporting the equivalent to the --user flag to systemctl.
##### STEPS TO REPRODUCE
<!---
For bugs, show exactly how to reproduce the problem, using a minimal test-case.
For new features, show how the feature would be used.
-->

<!--- Paste example playbooks or commands between quotes below -->
```yaml
- name: Mask a user unit for all users
  systemd: name=gpg-agent-ssh.socket masked=yes global=yes
```

<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- What did you expect to happen when running the steps above? -->
It would mask the user unit for all users, equivalent to the --global flag to systemctl.
##### ACTUAL RESULTS
<!--- What actually happened? If possible run with extra verbosity (-vvvv) -->
Not implemented
<!--- Paste verbatim command output between quotes below -->
```
N/A
```

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/modules/system/systemd.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/system/systemd.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
i would almost be in favor of deprecating `user` for `scope: None|user|global`
I started writing a pull request for this but using the name global seems to cause issues during testing since it is a reserved keyword. I'll reimplement using the suggested scope method instead to get around this.
Created pull request #40179 to implement the changes.

Commit message: Added version_added field for new option

Patch to review:
diff --git a/lib/ansible/modules/system/systemd.py b/lib/ansible/modules/system/systemd.py
index 6217752cbe0f15..96d2e6c1f1b4be 100644
--- a/lib/ansible/modules/system/systemd.py
+++ b/lib/ansible/modules/system/systemd.py
@@ -48,12 +48,13 @@
         type: bool
         default: 'no'
         aliases: [ daemon-reload ]
-    user:
+    scope:
         description:
-            - run systemctl talking to the service manager of the calling user, rather than the service manager
-              of the system.
-        type: bool
-        default: 'no'
+            - run systemctl within a given service manager scope, either as the default system scope (system),
+              the current user's scope (user), or the scope of all users (global).
+        choices: [ system, user, global ]
+        default: 'system'
+        version_added: "2.6"
     no_block:
         description:
             - Do not synchronously wait for the requested operation to finish.
@@ -298,7 +299,7 @@ def main():
             force=dict(type='bool'),
             masked=dict(type='bool'),
             daemon_reload=dict(type='bool', default=False, aliases=['daemon-reload']),
-            user=dict(type='bool', default=False),
+            scope=dict(type='str', default='system', choices=['system', 'user', 'global']),
             no_block=dict(type='bool', default=False),
         ),
         supports_check_mode=True,
@@ -306,8 +307,10 @@ def main():
     )
 
     systemctl = module.get_bin_path('systemctl', True)
-    if module.params['user']:
+    if module.params['scope'] == 'user':
         systemctl = systemctl + " --user"
+    if module.params['scope'] == 'global':
+        systemctl = systemctl + " --global"
     if module.params['no_block']:
         systemctl = systemctl + " --no-block"
     if module.params['force']:
@@ -401,8 +404,8 @@ def main():
             if rc == 0:
                 enabled = True
             elif rc == 1:
-                # if not a user service and both init script and unit file exist stdout should have enabled/disabled, otherwise use rc entries
-                if not module.params['user'] and \
+                # if not a user or global user service and both init script and unit file exist stdout should have enabled/disabled, otherwise use rc entries
+                if module.params['scope'] == 'system' and \
                         is_initd and \
                         (not out.strip().endswith('disabled') or sysv_is_enabled(unit)):
                     enabled = True


Code review comments:
diff_hunk: @@ -48,12 +48,13 @@
         type: bool
         default: 'no'
         aliases: [ daemon-reload ]
-    user:
+    scope:
path: lib/ansible/modules/system/systemd.py
text: @user1:
I'm fine with adding scope, but user should be deprecated, not removed and the interaction between both handled, with scope having precedence when set.

@author:
I didn't see in the documentation, but is there a standard way of makring something as deprecated?

@user1:
just add a note about deprecation in the description (and point to `scope`) and call` module.deprecate()` if it is set, our normal deprecation cycle is 4 versions, so if this goes in now (2.7) it should be removed in (2.11 or equivalent)