Repository: ansible/ansible
Pull request: rds: do not update multi-az unless specified - fixes #23385
##### SUMMARY
multi_zone [defaults to False](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/amazon/rds.py#L1084) rather than [None as the documentation states](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/amazon/rds.py#L108) so [this line](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/amazon/rds.py#L1040) does not work as expected when multi-az isn't specified. Other parameters are not updated unless they are specified explicitly. Fixes #23385

Also fixed rds to be pep8 and removed from legacy files.

##### ISSUE TYPE
 - Bugfix Pull Request


##### COMPONENT NAME
lib/ansible/modules/cloud/amazon/rds.py

##### ANSIBLE VERSION
```
2.4.0
```



Problem statement:
rds.py reverts multi-AZ RDS back to single-AZ on any 'modify' action where 'multi_zone' is not specified
##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
rds.py

##### ANSIBLE VERSION
```
ansible 2.3.0.0 (stable-2.3 0fe53eda86) last updated 2017/04/06 22:20:16 (GMT -500)
  config file = 
  configured module search path = Default w/o overrides
  python version = 2.7.13 (default, Dec 18 2016, 07:03:39) [GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)]

```

##### CONFIGURATION
n/a

##### OS / ENVIRONMENT
n/a

##### SUMMARY
Any 'modify' action using the rds.py module taken against a multi-AZ RDS, where the optional `multi_zone` parameter is not explicitly included in the task, results in the RDS being modified to a single-AZ RDS.

##### STEPS TO REPRODUCE
Example playbook:

<!--- Paste example playbooks or commands between quotes below -->
```
---

- name: Update password on RDS
  hosts: localhost
  tasks:
    - name: Apply new password to RDS
      rds:
        region: us-east-1
        command: modify
        instance_name: some-rds-12345
        password: supersekrit123

```

##### EXPECTED RESULTS
The `password` and/or other specified parameters are updated, while the `multi_zone` (multi-AZ) status of the RDS is left alone unless specified.

##### ACTUAL RESULTS
The requested parameters are updated but in addition, the RDS is changed as if we had specified `multi_zone` to false (ie. multi-AZ is disabled).

Hints from issue discussion:
cc @bpennypacker @willthames
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
Confirming this. [multi_zone defaults to False](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/amazon/rds.py#L1084) rather than None so this line https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/amazon/rds.py#L1040 does not work as expected when multi-az isn't specified.

Commit message: allow multi_az to be unmodified if not explicitly specified with command: modify

Patch to review:
diff --git a/lib/ansible/modules/cloud/amazon/rds.py b/lib/ansible/modules/cloud/amazon/rds.py
index e8d33d297b7603..0ca1cfba6aae55 100644
--- a/lib/ansible/modules/cloud/amazon/rds.py
+++ b/lib/ansible/modules/cloud/amazon/rds.py
@@ -1081,7 +1081,7 @@ def main():
         engine_version    = dict(required=False),
         parameter_group   = dict(required=False),
         license_model     = dict(choices=['license-included', 'bring-your-own-license', 'general-public-license', 'postgresql-license'], required=False),
-        multi_zone        = dict(type='bool', default=False),
+        multi_zone        = dict(type='bool', required=False),
         iops              = dict(required=False),
         security_groups   = dict(required=False),
         vpc_security_groups = dict(type='list', required=False),
@@ -1124,6 +1124,9 @@ def main():
         'restore': restore_db_instance,
     }
 
+    if module.params.['command'] != 'modify' and module.params.['multi_zone'] is None:
+        module.params.['multi_zone'] = False
+
     region, ec2_url, aws_connect_params = get_aws_connection_info(module)
     if not region:
         module.fail_json(msg="Region not specified. Unable to determine region from EC2_REGION.")


Code review comments:
diff_hunk: @@ -1124,6 +1124,9 @@ def main():
         'restore': restore_db_instance,
     }
 
+    if module.params.['command'] != 'modify' and module.params.['multi_zone'] is None:
path: lib/ansible/modules/cloud/amazon/rds.py
text: @user1:
shouldn't this just be `module.params['command']`? (without the second `.`)

diff_hunk: @@ -1124,6 +1124,9 @@ def main():
         'restore': restore_db_instance,
     }
 
+    if module.params.['command'] != 'modify' and module.params.['multi_zone'] is None:
+        module.params.['multi_zone'] = False
path: lib/ansible/modules/cloud/amazon/rds.py
text: @user1:
Having said that, I don't believe these two lines are required at all - we should only use multi_zone if it's explicitly set, and let the boto rds2 API handle the default (which it does - if you pass it multi_az=None (it's default), you'll get a non multi AZ RDS instance)

@author:
@user1 Oh, good point. Thanks! Fixed it.