linode - docs/validation changes + minor refactorings (#2410)

* multiple changes:

- documentation fixes
- minor refactorings

* added param deprecation note to the documentation

* added changelog fragment

* Update changelogs/fragments/2410-linode-improvements.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update changelogs/fragments/2410-linode-improvements.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/cloud/linode/linode.py

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>

changelogs/fragments/2410-linode-improvements.yml

@@ -0,0 +1,5 @@
+deprecated_features:
+  - linode - parameter ``backupsenabled`` is deprecated and will be removed in community.general 5.0.0 (https://github.com/ansible-collections/community.general/pull/2410).
+minor_changes:
+  - linode - added proper traceback when failing due to exceptions (https://github.com/ansible-collections/community.general/pull/2410).
+  - linode - parameter ``additional_disks`` is now validated as a list of dictionaries (https://github.com/ansible-collections/community.general/pull/2410).

plugins/modules/cloud/linode/linode.py

@@ -21,8 +21,10 @@ options:
     type: str
   api_key:
     description:
-     - Linode API key
+     - Linode API key.
+     - C(LINODE_API_KEY) env variable can be used instead.
     type: str
+    required: yes
   name:
     description:
      - Name to give the instance (alphanumeric, dashes, underscore).
@@ -46,6 +48,7 @@ options:
       - List of dictionaries for creating additional disks that are added to the Linode configuration settings.
       - Dictionary takes Size, Label, Type. Size is in MB.
     type: list
+    elements: dict
   alert_bwin_enabled:
     description:
     - Set status of bandwidth in alerts.
@@ -86,9 +89,18 @@ options:
     description:
     - Set threshold for average IO ops/sec over 2 hour period.
     type: int
+  backupsenabled:
+    description:
+    - Deprecated parameter, it will be removed in community.general C(5.0.0).
+    - To enable backups pass values to either I(backupweeklyday) or I(backupwindow).
+    type: int
   backupweeklyday:
     description:
-    - Integer value for what day of the week to store weekly backups.
+    - Day of the week to take backups.
+    type: int
+  backupwindow:
+    description:
+    - The time window in which backups will be taken.
     type: int
   plan:
     description:
@@ -153,7 +165,6 @@ author:
 notes:
   - Please note, linode-python does not have python 3 support.
   - This module uses the now deprecated v3 of the Linode API.
-  - C(LINODE_API_KEY) env variable can be used instead.
   - Please review U(https://www.linode.com/api/linode) for determining the required parameters.
 '''
 
@@ -262,7 +273,6 @@ EXAMPLES = '''
   delegate_to: localhost
 '''
 
-import os
 import time
 import traceback
 
@@ -274,7 +284,7 @@ except ImportError:
     LINODE_IMP_ERR = traceback.format_exc()
     HAS_LINODE = False
 
-from ansible.module_utils.basic import AnsibleModule, missing_required_lib
+from ansible.module_utils.basic import AnsibleModule, missing_required_lib, env_fallback
 
 
 def randompass():
@@ -358,7 +368,7 @@ def linodeServers(module, api, state, name,
         if not servers:
             for arg in (name, plan, distribution, datacenter):
                 if not arg:
-                    module.fail_json(msg='%s is required for %s state' % (arg, state))  # @TODO use required_if instead
+                    module.fail_json(msg='%s is required for %s state' % (arg, state))
             # Create linode entity
             new_server = True
 
@@ -383,7 +393,7 @@ def linodeServers(module, api, state, name,
             try:
                 res = api.linode_ip_addprivate(LinodeID=linode_id)
             except Exception as e:
-                module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'])
+                module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'], exception=traceback.format_exc())
 
         if not disks:
             for arg in (name, linode_id, distribution):
@@ -428,7 +438,7 @@ def linodeServers(module, api, state, name,
                 jobs.append(res['JobID'])
             except Exception as e:
                 # TODO: destroy linode ?
-                module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'])
+                module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'], exception=traceback.format_exc())
 
         if not configs:
             for arg in (name, linode_id, distribution):
@@ -471,7 +481,7 @@ def linodeServers(module, api, state, name,
                                          Disklist=disks_list, Label='%s config' % name)
                 configs = api.linode_config_list(LinodeId=linode_id)
             except Exception as e:
-                module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'])
+                module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'], exception=traceback.format_exc())
 
         # Start / Ensure servers are running
         for server in servers:
@@ -517,10 +527,7 @@ def linodeServers(module, api, state, name,
                 instance['password'] = password
             instances.append(instance)
 
-    elif state in ('stopped'):
+    elif state in ('stopped',):
-        if not linode_id:
-            module.fail_json(msg='linode_id is required for stopped state')
-
         if not servers:
             module.fail_json(msg='Server (lid: %s) not found' % (linode_id))
 
@@ -530,17 +537,14 @@ def linodeServers(module, api, state, name,
                 try:
                     res = api.linode_shutdown(LinodeId=linode_id)
                 except Exception as e:
-                    module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'])
+                    module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'], exception=traceback.format_exc())
                 instance['status'] = 'Stopping'
                 changed = True
             else:
                 instance['status'] = 'Stopped'
             instances.append(instance)
 
-    elif state in ('restarted'):
+    elif state in ('restarted',):
-        if not linode_id:
-            module.fail_json(msg='linode_id is required for restarted state')
-
         if not servers:
             module.fail_json(msg='Server (lid: %s) not found' % (linode_id))
 
@@ -549,7 +553,7 @@ def linodeServers(module, api, state, name,
             try:
                 res = api.linode_reboot(LinodeId=server['LINODEID'])
             except Exception as e:
-                module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'])
+                module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'], exception=traceback.format_exc())
             instance['status'] = 'Restarting'
             changed = True
             instances.append(instance)
@@ -560,7 +564,7 @@ def linodeServers(module, api, state, name,
             try:
                 api.linode_delete(LinodeId=server['LINODEID'], skipChecks=True)
             except Exception as e:
-                module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'])
+                module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'], exception=traceback.format_exc())
             instance['status'] = 'Deleting'
             changed = True
             instances.append(instance)
@@ -577,7 +581,7 @@ def main():
         argument_spec=dict(
             state=dict(type='str', default='present',
                        choices=['absent', 'active', 'deleted', 'present', 'restarted', 'started', 'stopped']),
-            api_key=dict(type='str', no_log=True),
+            api_key=dict(type='str', no_log=True, required=True, fallback=(env_fallback, ['LINODE_API_KEY'])),
             name=dict(type='str', required=True),
             alert_bwin_enabled=dict(type='bool'),
             alert_bwin_threshold=dict(type='int'),
@@ -589,12 +593,12 @@ def main():
             alert_cpu_threshold=dict(type='int'),
             alert_diskio_enabled=dict(type='bool'),
             alert_diskio_threshold=dict(type='int'),
-            backupsenabled=dict(type='int'),
+            backupsenabled=dict(type='int', removed_in_version='5.0.0', removed_from_collection='community.general'),
             backupweeklyday=dict(type='int'),
             backupwindow=dict(type='int'),
             displaygroup=dict(type='str', default=''),
             plan=dict(type='int'),
-            additional_disks=dict(type='list'),
+            additional_disks=dict(type='list', elements='dict'),
             distribution=dict(type='int'),
             datacenter=dict(type='int'),
             kernel_id=dict(type='int'),
@@ -608,6 +612,10 @@ def main():
             wait_timeout=dict(type='int', default=300),
             watchdog=dict(type='bool', default=True),
         ),
+        required_if=[
+            ('state', 'restarted', ['linode_id']),
+            ('state', 'stopped', ['linode_id']),
+        ]
     )
 
     if not HAS_LINODE:
@@ -626,7 +634,6 @@ def main():
     alert_cpu_threshold = module.params.get('alert_cpu_threshold')
     alert_diskio_enabled = module.params.get('alert_diskio_enabled')
     alert_diskio_threshold = module.params.get('alert_diskio_threshold')
-    backupsenabled = module.params.get('backupsenabled')
     backupweeklyday = module.params.get('backupweeklyday')
     backupwindow = module.params.get('backupwindow')
     displaygroup = module.params.get('displaygroup')
@@ -642,10 +649,9 @@ def main():
     ssh_pub_key = module.params.get('ssh_pub_key')
     swap = module.params.get('swap')
     wait = module.params.get('wait')
-    wait_timeout = int(module.params.get('wait_timeout'))
+    wait_timeout = module.params.get('wait_timeout')
     watchdog = int(module.params.get('watchdog'))
 
-    kwargs = dict()
     check_items = dict(
         alert_bwin_enabled=alert_bwin_enabled,
         alert_bwin_threshold=alert_bwin_threshold,
@@ -661,23 +667,14 @@ def main():
         backupwindow=backupwindow,
     )
 
-    for key, value in check_items.items():
+    kwargs = dict((k, v) for k, v in check_items.items() if v is not None)
-        if value is not None:
-            kwargs[key] = value
-
-    # Setup the api_key
-    if not api_key:
-        try:
-            api_key = os.environ['LINODE_API_KEY']
-        except KeyError as e:
-            module.fail_json(msg='Unable to load %s' % e.message)
 
     # setup the auth
     try:
         api = linode_api.Api(api_key)
         api.test_echo()
     except Exception as e:
-        module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'])
+        module.fail_json(msg='%s' % e.value[0]['ERRORMESSAGE'], exception=traceback.format_exc())
 
     linodeServers(module, api, state, name,
                   displaygroup, plan,

tests/sanity/ignore-2.10.txt

@@ -1,9 +1,6 @@
 plugins/module_utils/cloud.py pylint:bad-option-value  # a pylint test that is disabled was modified over time
 plugins/module_utils/_mount.py future-import-boilerplate
 plugins/module_utils/_mount.py metaclass-boilerplate
-plugins/modules/cloud/linode/linode.py validate-modules:parameter-list-no-elements
-plugins/modules/cloud/linode/linode.py validate-modules:parameter-type-not-in-doc
-plugins/modules/cloud/linode/linode.py validate-modules:undocumented-parameter
 plugins/modules/cloud/lxc/lxc_container.py use-argspec-type-path
 plugins/modules/cloud/lxc/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/cloud/misc/rhevm.py validate-modules:parameter-state-invalid-choice

tests/sanity/ignore-2.11.txt

@@ -1,8 +1,5 @@
 plugins/module_utils/_mount.py future-import-boilerplate
 plugins/module_utils/_mount.py metaclass-boilerplate
-plugins/modules/cloud/linode/linode.py validate-modules:parameter-list-no-elements
-plugins/modules/cloud/linode/linode.py validate-modules:parameter-type-not-in-doc
-plugins/modules/cloud/linode/linode.py validate-modules:undocumented-parameter
 plugins/modules/cloud/lxc/lxc_container.py use-argspec-type-path
 plugins/modules/cloud/lxc/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/cloud/misc/rhevm.py validate-modules:parameter-state-invalid-choice

tests/sanity/ignore-2.12.txt

@@ -1,8 +1,5 @@
 plugins/module_utils/_mount.py future-import-boilerplate
 plugins/module_utils/_mount.py metaclass-boilerplate
-plugins/modules/cloud/linode/linode.py validate-modules:parameter-list-no-elements
-plugins/modules/cloud/linode/linode.py validate-modules:parameter-type-not-in-doc
-plugins/modules/cloud/linode/linode.py validate-modules:undocumented-parameter
 plugins/modules/cloud/lxc/lxc_container.py use-argspec-type-path
 plugins/modules/cloud/lxc/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/cloud/misc/rhevm.py validate-modules:parameter-state-invalid-choice

tests/sanity/ignore-2.9.txt

@@ -1,8 +1,6 @@
 plugins/module_utils/cloud.py pylint:bad-option-value  # a pylint test that is disabled was modified over time
 plugins/module_utils/_mount.py future-import-boilerplate
 plugins/module_utils/_mount.py metaclass-boilerplate
-plugins/modules/cloud/linode/linode.py validate-modules:parameter-type-not-in-doc
-plugins/modules/cloud/linode/linode.py validate-modules:undocumented-parameter
 plugins/modules/cloud/lxc/lxc_container.py use-argspec-type-path
 plugins/modules/cloud/lxc/lxc_container.py validate-modules:use-run-command-not-popen
 plugins/modules/cloud/online/online_server_info.py validate-modules:return-syntax-error