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.

Sign up for GitHub

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

Jump to bottom

vultr: fix for unreliable API behavior #45712

Merged
merged 2 commits into from
Sep 18, 2018
Merged

vultr: fix for unreliable API behavior #45712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10a72f8
vultr: fix for unreliable API behavior
resmo Sep 17, 2018
f35279d
code styling
resmo Sep 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions lib/ansible/module_utils/vultr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,7 @@ def api_query(self, path="/", method="GET", data=None):
timeout=self.api_config['api_timeout'],
)

# Did we hit the rate limit?
if info.get('status') and info.get('status') != 503:
if info.get('status') == 200:
break

# Vultr has a rate limiting requests per second, try to be polite
Expand Down
10 changes: 3 additions & 7 deletions lib/ansible/modules/cloud/vultr/vultr_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,6 @@ def _update_auto_backups_setting(self, server, start_server):
method="POST",
data=data
)
server = self._wait_for_state(key='server_state', state='ok')
return server

def _update_ipv6_setting(self, server, start_server):
Expand All @@ -554,7 +553,6 @@ def _update_ipv6_setting(self, server, start_server):
method="POST",
data=data
)
server = self._wait_for_state(key='server_state', state='ok')
server = self._wait_for_state(key='v6_main_ip')
return server

Expand All @@ -579,7 +577,6 @@ def _update_private_network_setting(self, server, start_server):
method="POST",
data=data
)
server = self._wait_for_state(key='server_state', state='ok')
return server

def _update_plan_setting(self, server, start_server):
Expand All @@ -602,7 +599,6 @@ def _update_plan_setting(self, server, start_server):
method="POST",
data=data
)
server = self._wait_for_state(key='server_state', state='ok')
return server

def _handle_power_status_for_update(self, server, start_server):
Expand Down Expand Up @@ -702,7 +698,7 @@ def _update_server(self, server=None, start_server=True):
if self.server_power_state in ['starting', 'running'] and start_server:
server = self.start_server(skip_results=True)

server = self._wait_for_state(key='server_state', state='ok')
server = self._wait_for_state(key='status', state='active')
return server

def absent_server(self):
Expand All @@ -720,7 +716,7 @@ def absent_server(self):
method="POST",
data=data
)
for s in range(0, 30):
for s in range(0, 60):
Copy link
Contributor

Choose a reason for hiding this comment

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

nit question: Why this change? Is it taking more time to get a server deleted? Also, should this be a parameter, (maybe reuse api_timeout) ? Is the behavior observed for server_state update also observed here or is it to play safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will make it configurable but first, I would like to get this in as a bug fix for 2.7. the timeout was 30 times 2 seconds, I just wanted to make it "engough" long for 2.7 (it seems the time to wait has dramatically increased)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ack, shipit'able for me. Do you want to change the info.get('status') first?

if server is not None:
break
time.sleep(2)
Expand Down Expand Up @@ -764,7 +760,7 @@ def reinstall_server(self):
def _wait_for_state(self, key='power_status', state=None):
time.sleep(1)
server = self.get_server(refresh=True)
for s in range(0, 30):
for s in range(0, 60):
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

# Check for Truely if wanted state is None
if state is None and server.get(key):
break
Expand Down
5 changes: 5 additions & 0 deletions test/legacy/roles/vultr_server/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
that:
- result is success

# Servers can only be destroyed 5 min after creation
- name: wait for 5 min
local_action: wait_for
when: result is changed

- name: test fail if missing name
vultr_server:
register: result
Expand Down