Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/support openstack metadata #186

Merged

Conversation

roll4life
Copy link
Contributor

refs #183 add initial support for running spilo in kubernetes on openstack

@@ -210,16 +211,21 @@ def deep_update(a, b):

def get_provider():
try:
logging.info("Figuring out my environment (Google? AWS? Local?)")
logging.info("Figuring out my environment (Google? AWS? Openstack? Local?)")
r = requests.get('http://169.254.169.254', timeout=2)
if r.headers.get('Metadata-Flavor', '') == 'Google':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering, may be on Openstack there is something similar available?'
Unique header for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from what I can tell there is not. I ran the following to check

root@patroni-patroni-0:/home/postgres# python3
Python 3.4.3 (default, Nov 17 2016, 01:08:31) 
[GCC 4.8.4] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import requests
>>> r = requests.get('http://169.254.169.254', timeout=2)
>>> r.headers
{'Date': 'Thu, 07 Sep 2017 22:51:27 GMT', 'Content-Type': 'text/plain; charset=UTF-8', 'Connection': 'keep-alive', 'Content-Length': '98'}

I did find that there is an openstack specific path
http://169.254.169.254/openstack/latest/meta_data.json
perhaps this would be a more reliable method of determining that a host is running on openstack.

r = requests.get('http://instance-data/latest/meta-data/ami-id') # accessible on AWS, will fail on Openstack
if r.ok:
return PROVIDER_AWS
except (requests.exceptions.ConnectTimeout, requests.exceptions.ConnectionError):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the #183 it will raise socket.gaierror: [Errno -2] Name or service not known (fail to resolve instance-data) on openshift and therefore will to fix the problem

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyberDem0n the problem was actually discovered in plain Kuberenetes running on OpenStack. However, If a user was running OpenShift on OpenStack, they would likely encounter the same error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, but before your changes if was failing in a different place, because socket.gaierror: exception hasn't been handled properly.
Now it will fail on the line 220, because DNS can't resolve instance-data into ip address.
On the line 223 we are trying to catch only connection error exceptions.
If you do a change:

 -            except (requests.exceptions.ConnectTimeout, requests.exceptions.ConnectionError): 
 +            except Exception: 

It will start working, but in addition to that I think it would make sense to change line 224:

-                r = requests.get('http://169.254.169.254/latest/meta-data/ami-id')
+                r = requests.get('http://169.254.169.254/openstack/latest/meta_data.json')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyberDem0n I have implemented the changes you suggested, I removed instance-data and replaced it with the metadata server ip address which should resolve #191

@CyberDem0n
Copy link
Contributor

👍

1 similar comment
@avaczi
Copy link
Contributor

avaczi commented Sep 29, 2017

👍

@CyberDem0n CyberDem0n merged commit 8b04274 into zalando:master Sep 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants