Merge 5118f2da78 into 2c52f969e1

firewalld: make offline do something

SUMMARY

ansible.posix.firewalld has an offline flag, but it currently does not do anything. What most people expect it to do is allow the task to proceed even when firewalld is offline, so it makes the most sense for it to override the immediate flag and prevent the module from throwing an error in that case.
Fixes #81.
ISSUE TYPE


Feature Pull Request

COMPONENT NAME

firewalld
ADDITIONAL INFORMATION

Reviewed-by: Adam Miller <admiller@redhat.com>

changelogs/fragments/484-firewalld-offline.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - firewalld - added offline flag implementation (https://github.com/ansible-collections/ansible.posix/pull/484)

changelogs/fragments/511_profile-callbacks-add-summary-only-parameter.yml

@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - "Add summary_only parameter to profile_roles and profile_tasks callbacks."

plugins/callback/profile_roles.py

@@ -14,6 +14,19 @@ DOCUMENTATION = '''
         - This callback module provides profiling for ansible roles.
     requirements:
       - whitelisting in configuration
+    options:
+      summary_only:
+        description:
+          - Only show summary, not individual task profiles.
+            Especially usefull in combination with C(DISPLAY_SKIPPED_HOSTS=false) and/or C(ANSIBLE_DISPLAY_OK_HOSTS=false).
+        type: bool
+        default: False
+        env:
+          - name: PROFILE_ROLES_SUMMARY_ONLY
+        ini:
+          - section: callback_profile_roles
+            key: summary_only
+        version_added: '1.5.5'
 '''
 
 import collections
@@ -76,13 +89,26 @@ class CallbackModule(CallbackBase):
         self.stats = collections.Counter()
         self.totals = collections.Counter()
         self.current = None
+
+        self.summary_only = None
+
         super(CallbackModule, self).__init__()
 
+    def set_options(self, task_keys=None, var_options=None, direct=None):
+
+        super(CallbackModule, self).set_options(task_keys=task_keys, var_options=var_options, direct=direct)
+
+        self.summary_only = self.get_option('summary_only')
+
+    def _display_tasktime(self):
+        if not self.summary_only:
+            self._display.display(tasktime())
+
     def _record_task(self, task):
         """
         Logs the start of each task
         """
-        self._display.display(tasktime())
+        self._display_tasktime()
         timestamp(self)
 
         if task._role:
@@ -99,10 +125,10 @@ class CallbackModule(CallbackBase):
         self._record_task(task)
 
     def playbook_on_setup(self):
-        self._display.display(tasktime())
+        self._display_tasktime()
 
     def playbook_on_stats(self, stats):
-        self._display.display(tasktime())
+        self._display_tasktime()
         self._display.display(filled("", fchar="="))
 
         timestamp(self)

plugins/callback/profile_tasks.py

@@ -40,6 +40,18 @@ DOCUMENTATION = '''
         ini:
           - section: callback_profile_tasks
             key: sort_order
+      summary_only:
+        description:
+          - Only show summary, not individual task profiles.
+            Especially usefull in combination with C(DISPLAY_SKIPPED_HOSTS=false) and/or C(ANSIBLE_DISPLAY_OK_HOSTS=false).
+        type: bool
+        default: False
+        env:
+          - name: PROFILE_TASKS_SUMMARY_ONLY
+        ini:
+          - section: callback_profile_tasks
+            key: summary_only
+        version_added: '1.5.5'
 '''
 
 EXAMPLES = '''
@@ -120,6 +132,7 @@ class CallbackModule(CallbackBase):
         self.current = None
 
         self.sort_order = None
+        self.summary_only = None
         self.task_output_limit = None
 
         super(CallbackModule, self).__init__()
@@ -137,6 +150,8 @@ class CallbackModule(CallbackBase):
             elif self.sort_order == 'none':
                 self.sort_order = None
 
+        self.summary_only = self.get_option('summary_only')
+
         self.task_output_limit = self.get_option('output_limit')
         if self.task_output_limit is not None:
             if self.task_output_limit == 'all':
@@ -144,11 +159,15 @@ class CallbackModule(CallbackBase):
             else:
                 self.task_output_limit = int(self.task_output_limit)
 
+    def _display_tasktime(self):
+        if not self.summary_only:
+            self._display.display(tasktime())
+
     def _record_task(self, task):
         """
         Logs the start of each task
         """
-        self._display.display(tasktime())
+        self._display_tasktime()
         timestamp(self)
 
         # Record the start time of the current task
@@ -171,10 +190,10 @@ class CallbackModule(CallbackBase):
         self._record_task(task)
 
     def playbook_on_setup(self):
-        self._display.display(tasktime())
+        self._display_tasktime()
 
     def playbook_on_stats(self, stats):
-        self._display.display(tasktime())
+        self._display_tasktime()
         self._display.display(filled("", fchar="="))
 
         timestamp(self)

plugins/modules/firewalld.py

@@ -84,13 +84,15 @@ options:
     type: str
   permanent:
     description:
-      - Should this configuration be in the running firewalld configuration or persist across reboots.
+      - Whether to apply this change to the permanent firewalld configuration.
       - As of Ansible 2.3, permanent operations can operate on firewalld configs when it is not running (requires firewalld >= 0.3.9).
-      - Note that if this is C(false), immediate is assumed C(true).
+      - Note that if this is C(false), I(immediate) defaults to C(true).
     type: bool
+    default: false
   immediate:
     description:
-      - Should this configuration be applied immediately, if set as permanent.
+      - Whether to apply this change to the runtime firewalld configuration.
+      - Defaults to C(true) if I(permanent=false).
     type: bool
     default: false
   state:
@@ -112,8 +114,9 @@ options:
     type: str
   offline:
     description:
-      - Whether to run this module even when firewalld is offline.
+      - Ignores I(immediate) if I(permanent=true) and firewalld is not running.
     type: bool
+    default: false
   target:
     description:
       - firewalld Zone target
@@ -142,6 +145,14 @@ author:
 '''
 
 EXAMPLES = r'''
+- name: permanently enable https service, also enable it immediately if possible
+  ansible.posix.firewalld:
+    service: https
+    state: enabled
+    permanent: true
+    immediate: true
+    offline: true
+
 - name: permit traffic in default zone for https service
   ansible.posix.firewalld:
     service: https
@@ -806,12 +817,12 @@ def main():
             zone=dict(type='str'),
             immediate=dict(type='bool', default=False),
             source=dict(type='str'),
-            permanent=dict(type='bool'),
+            permanent=dict(type='bool', default=False),
             state=dict(type='str', required=True, choices=['absent', 'disabled', 'enabled', 'present']),
             timeout=dict(type='int', default=0),
             interface=dict(type='str'),
             masquerade=dict(type='str'),
-            offline=dict(type='bool'),
+            offline=dict(type='bool', default=False),
             target=dict(type='str', choices=['default', 'ACCEPT', 'DROP', '%%REJECT%%']),
         ),
         supports_check_mode=True,
@@ -832,19 +843,29 @@ def main():
     timeout = module.params['timeout']
     interface = module.params['interface']
     masquerade = module.params['masquerade']
+    offline = module.params['offline']
 
     # Sanity checks
     FirewallTransaction.sanity_check(module)
 
-    # If neither permanent or immediate is provided, assume immediate (as
-    # written in the module's docs)
+    # `offline`, `immediate`, and `permanent` have a weird twisty relationship.
+    if offline:
+        # specifying offline without permanent makes no sense
+        if not permanent:
+            module.fail_json(msg='offline cannot be enabled unless permanent changes are allowed')
+
+        # offline overrides immediate to false if firewalld is offline
+        if fw_offline:
+            immediate = False
+
+    # immediate defaults to true if permanent is not enabled
     if not permanent and not immediate:
         immediate = True
 
-    # Verify required params are provided
     if immediate and fw_offline:
         module.fail_json(msg='firewall is not currently running, unable to perform immediate actions without a running firewall daemon')
 
+    # Verify required params are provided
     changed = False
     msgs = []
     icmp_block = module.params['icmp_block']

tests/integration/targets/firewalld/tasks/service_test_cases.yml

@@ -21,6 +21,8 @@
   ansible.posix.firewalld:
     service: https
     permanent: true
+    immediate: true
+    offline: true
     state: enabled
   register: result
 
@@ -33,6 +35,8 @@
   ansible.posix.firewalld:
     service: https
     permanent: true
+    immediate: true
+    offline: true
     state: enabled
   register: result
 
@@ -45,6 +49,8 @@
   ansible.posix.firewalld:
     service: https
     permanent: true
+    immediate: true
+    offline: true
     state: disabled
   register: result
 
@@ -57,6 +63,8 @@
   ansible.posix.firewalld:
     service: https
     permanent: true
+    immediate: true
+    offline: true
     state: disabled
   register: result