Repository: ansible/ansible
Pull request: Add retry to ec2_metadata_facts query
##### SUMMARY
<!--- Describe the change, including rationale and design decisions -->

<!--- 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.-->
In the fetch step for the EC2 metadata, add a retry in case the metadata service is throttled or unavailable. Currently, EC2 metadata service issues show up as a KeyError, which isn't very intuitive or helpful. 

Fixes #38902 

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->

 - Bugfix Pull Request

##### COMPONENT NAME
<!--- Name of the module, plugin, module or task -->
ec2_metadata_facts

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes below -->
```

```


##### 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 -->
```

```

cc/ @roadmapper 


Problem statement:
ec2_metadata_facts returns keyError
##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
ec2_metadata_facts

##### ANSIBLE VERSION
```
ansible 2.5.0
  config file = /home/jwitkowski/throtle-ansible/ansible.cfg
  configured module search path = [u'/home/jwitkowski/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/jwitkowski/throtle-ansible-venv/lib/python2.7/site-packages/ansible
  executable location = /home/jwitkowski/throtle-ansible-venv/bin/ansible
  python version = 2.7.5 (default, Aug  4 2017, 00:39:18) [GCC 4.8.5 20150623 (Red Hat 4.8.5-16)]
```

##### CONFIGURATION
```
(throtle-ansible-venv) [jwitkowski@workstation throtle-ansible]$ ansible-config dump --only-changed
ANSIBLE_PIPELINING(/home/jwitkowski/throtle-ansible/ansible.cfg) = True
ANSIBLE_SSH_ARGS(/home/jwitkowski/throtle-ansible/ansible.cfg) = -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o ForwardAgent=yes
DEFAULT_CALLBACK_PLUGIN_PATH(/home/jwitkowski/throtle-ansible/ansible.cfg) = [u'/home/jwitkowski/throtle-ansible/library/callback']
DEFAULT_FILTER_PLUGIN_PATH(/home/jwitkowski/throtle-ansible/ansible.cfg) = [u'/usr/share/ansible_plugins/filter_plugins', u'/home/jwitkowski/throtle-ansible/library/plugins']
DEFAULT_FORKS(/home/jwitkowski/throtle-ansible/ansible.cfg) = 15
DEFAULT_GATHERING(/home/jwitkowski/throtle-ansible/ansible.cfg) = smart
DEFAULT_GATHER_SUBSET(/home/jwitkowski/throtle-ansible/ansible.cfg) = !hardware,!ohai,!facter
DEFAULT_HOST_LIST(/home/jwitkowski/throtle-ansible/ansible.cfg) = [u'/home/jwitkowski/throtle-ansible/inventory']
DEFAULT_MANAGED_STR(/home/jwitkowski/throtle-ansible/ansible.cfg) = Created by Ansible
DEFAULT_PRIVATE_KEY_FILE(/home/jwitkowski/throtle-ansible/ansible.cfg) = /home/jwitkowski/.ssh/throtle-ansible.key
DEFAULT_ROLES_PATH(/home/jwitkowski/throtle-ansible/ansible.cfg) = [u'/home/jwitkowski/throtle-ansible/roles']
DEFAULT_SCP_IF_SSH(/home/jwitkowski/throtle-ansible/ansible.cfg) = True
DEFAULT_VAULT_PASSWORD_FILE(/home/jwitkowski/throtle-ansible/ansible.cfg) = /home/jwitkowski/throtle-ansible/.vault
DISPLAY_ARGS_TO_STDOUT(/home/jwitkowski/throtle-ansible/ansible.cfg) = True
HOST_KEY_CHECKING(/home/jwitkowski/throtle-ansible/ansible.cfg) = False
RETRY_FILES_SAVE_PATH(/home/jwitkowski/throtle-ansible/ansible.cfg) = /home/jwitkowski/.ansible/retry-files
```

##### OS / ENVIRONMENT
N/A

##### SUMMARY
Intermittently the ec2_metadata_facts module will return a KeyError, always for the same key.

##### STEPS TO REPRODUCE
I simply run the following command, via an "include_tasks:" include.  99% of the time it works without issue but every once in a while it gets stuck and will only return the below error.
```yaml
- name: Main | Retrieve all ec2_metadata_facts
  ec2_metadata_facts:
  tags: ['always']
```

##### EXPECTED RESULTS
ec2 facts will be popualted

##### ACTUAL RESULTS
A KeyError is returned and the task fails:
```
Traceback (most recent call last):
  File \"\/tmp\/ansible_hnBbkT\/ansible_module_ec2_metadata_facts.py\", line 548, in <module>
    main()
  File \"\/tmp\/ansible_hnBbkT\/ansible_module_ec2_metadata_facts.py\", line 541, in main
    ec2_metadata_facts = Ec2Metadata(module).run()
  File \"\/tmp\/ansible_hnBbkT\/ansible_module_ec2_metadata_facts.py\", line 526, in run
    data['ansible_ec2_placement_region'] = data['ansible_ec2_instance_identity_document_region']
KeyError: 'ansible_ec2_instance_identity_document_region'
```

@ryansb

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/modules/cloud/amazon/ec2_metadata_facts.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/amazon/ec2_metadata_facts.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 --->
cc @roadmapper @silviud @willthames
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
FYI discussed this a bit with @ryansb on IRC.  Issue does not seem to be with the module but with the EC2 instance not being able to talk to the metadata service.  We discussed the possibility of reporting an event as such.
Seems reasonable. I know @ryansb had posed [this problem](https://github.com/ansible/ansible/pull/21532#pullrequestreview-37409062) in the past but I wasn't sure of the best way to fix the problem. Is the premise of the fix to change the response logic so that `module.fail_json` is called when the response from the metadata service is not valid? Or should the request be retried with some backoff logic?
@roadmapper yes, that's what I was thinking. I was going to implement like this: 

- Request metadata
- If request fails, wait 3 seconds and try again
- If second request fails, use `fail_json` and raise the error

I didn't want to do a ton of retries, because the request timeout is 10 seconds and I didn't want the module to take minutes to fail completely. I have most of a pull request implemented, I'll add you as a reviewer when I submit it.

Commit message: Add retry to ec2_metadata_facts query

Patch to review:
diff --git a/lib/ansible/modules/cloud/amazon/ec2_metadata_facts.py b/lib/ansible/modules/cloud/amazon/ec2_metadata_facts.py
index 4594058d9d8a1d..02c9600b98a13e 100644
--- a/lib/ansible/modules/cloud/amazon/ec2_metadata_facts.py
+++ b/lib/ansible/modules/cloud/amazon/ec2_metadata_facts.py
@@ -420,6 +420,7 @@
 import json
 import re
 import socket
+import time
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils._text import to_text
@@ -445,7 +446,16 @@ def __init__(self, module, ec2_metadata_uri=None, ec2_sshdata_uri=None, ec2_user
         self._prefix = 'ansible_ec2_%s'
 
     def _fetch(self, url):
-        (response, info) = fetch_url(self.module, url, force=True)
+        response, info = fetch_url(self.module, url, force=True)
+
+        if not info.get('msg', '').startswith('OK'):
+            time.sleep(3)
+            # request went bad, retry once then raise
+            self.module.warn('Retrying query to metadata service. First attempt failed: {0}'.format(info['msg']))
+            response, info = fetch_url(self.module, url, force=True)
+            if not info.get('msg', '').startswith('OK'):
+                # fail out now
+                self.module.fail_json(msg='Failed to retrieve metadata from AWS: {0}'.format(info['msg']), response=info)
         if response:
             data = response.read()
         else:


Code review comments:
diff_hunk: @@ -445,7 +446,16 @@ def __init__(self, module, ec2_metadata_uri=None, ec2_sshdata_uri=None, ec2_user
         self._prefix = 'ansible_ec2_%s'
 
     def _fetch(self, url):
-        (response, info) = fetch_url(self.module, url, force=True)
+        response, info = fetch_url(self.module, url, force=True)
+
+        if not info.get('msg', '').startswith('OK'):
path: lib/ansible/modules/cloud/amazon/ec2_metadata_facts.py
text: @user1:
Just tested this and it might not be enough to check for just `'OK'`; when the recursive URL calls get to the end of the public key URL it will return a 404. 
```
{'body': '<?xml version="1.0" encoding="iso-8859-1"?>\n<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0
Transitional//EN"\n         "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">\n<html xmlns="http://www.w3.org/1999/xhtml"
xml:lang="en" lang="en">\n <head>\n  <title>404 - Not Found</title>\n </head>\n <body>\n  <h1>404 - Not Found</h1>\n
</body>\n</html>\n', 'date': 'Wed, 18 Apr 2018 18:21:46 GMT', 'content-length': '345', 'status': 404, 'msg': 'HTTP Error 404: Not
Found', 'url': u'http://169.254.169.254/latest/meta-data/public-keys/0=vinay.dandekar', 'content-type': 'text/html', 'connection':
'close', 'server': 'EC2ws'}
```
We could attempt to exclude that? Or maybe for something like: 
```python
if info.get('status') not in [200, 404]:
```