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

New module to manage OVH's failover IPs. #34837

Open
wants to merge 4 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@pascalheraud
Contributor

pascalheraud commented Jan 13, 2018

SUMMARY

This new module allows to handle IP address into OVH provider.
It uses OVH python API and its main goal for now is to switch an IP between hosts (services).
The module is mainly inspired from the other module I did for OVH (ovh_ip_loadbalancing_backend)

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

clould/ovh_ip_failover

ANSIBLE VERSION

ansible 2.5.0


@ansibot

This comment has been minimized.

Contributor

ansibot commented Jan 13, 2018

The test ansible-test sanity --test pep8 [?] failed with the following errors:

lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:24:68: W291 trailing whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:25:76: W291 trailing whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:37:75: W291 trailing whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:58:67: W291 trailing whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:64:58: W291 trailing whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:109:26: E127 continuation line over-indented for visual indent
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:110:26: E127 continuation line over-indented for visual indent
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:131:1: W293 blank line contains whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:132:11: E225 missing whitespace around operator
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:183:58: E203 whitespace before ':'
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:185:56: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:185:58: E251 unexpected spaces around keyword / parameter equals

The test ansible-test sanity --test pylint [?] failed with the following error:

lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:131:0: trailing-whitespace Trailing whitespace

The test ansible-test sanity --test validate-modules [?] failed with the following error:

lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:0:0: E307 version_added should be 2.5. Currently 2.4

click here for bot help

@pascalheraud pascalheraud force-pushed the pascalheraud:module_ovh_ip_failover branch Jan 14, 2018

@ansibot ansibot removed the ci_verified label Jan 14, 2018

@pascalheraud pascalheraud force-pushed the pascalheraud:module_ovh_ip_failover branch Jan 14, 2018

@sivel sivel removed the needs_triage label Jan 15, 2018

Show resolved Hide resolved lib/ansible/modules/cloud/ovh/ovh_ip_failover.py Outdated

@pascalheraud pascalheraud force-pushed the pascalheraud:module_ovh_ip_failover branch 2 times, most recently Jan 15, 2018

@ansibot ansibot added the stale_ci label Jan 24, 2018

@pascalheraud pascalheraud force-pushed the pascalheraud:module_ovh_ip_failover branch Feb 18, 2018

@ansibot ansibot removed the stale_ci label Feb 18, 2018

@pascalheraud

This comment has been minimized.

Contributor

pascalheraud commented Feb 18, 2018

ready_for_review

@ansibot

This comment has been minimized.

Contributor

ansibot commented Feb 18, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:0:0: E307 version_added should be 2.6. Currently 2.5

click here for bot help

@pascalheraud pascalheraud force-pushed the pascalheraud:module_ovh_ip_failover branch to 5676eb2 Feb 18, 2018

@pascalheraud

This comment has been minimized.

Contributor

pascalheraud commented Mar 7, 2018

ready_for_review

@ansibot ansibot added the stale_ci label Mar 15, 2018

@pascalheraud

This comment has been minimized.

Contributor

pascalheraud commented May 18, 2018

This module is 4 month old now, do I have a chance it will be merged ? I'd like this to be included in next release.

@pascalheraud

This comment has been minimized.

Contributor

pascalheraud commented Nov 14, 2018

up

@hedii

This comment has been minimized.

hedii commented Dec 5, 2018

@pascalheraud when I test this module, I get this error:

(venv) ➜  ansible git:(module_ovh_ip_failover) ✗ python lib/ansible/modules/cloud/ovh/ovh_ip_failover.py ~/args3.json
Traceback (most recent call last):
  File "lib/ansible/modules/cloud/ovh/ovh_ip_failover.py", line 197, in <module>
    main()
  File "lib/ansible/modules/cloud/ovh/ovh_ip_failover.py", line 163, in main
    if not waitForNoTask(client, name, timeout):
  File "lib/ansible/modules/cloud/ovh/ovh_ip_failover.py", line 110, in waitForNoTask
    while client.get('/ip/{0}/task'.format(urllib.quote_plus(name)),
AttributeError: module 'urllib' has no attribute 'quote_plus'

you have to use urllib.parse.quote_plus() instead

@hedii

quote_plus is now included in parse.
Use urllib.parse.quote_plus()

if ipproperties['routedTo']['serviceName'] != service:
if not module.check_mode:
client.post('/ip/{0}/move'.format(urllib.quote_plus(name)), to=service)

This comment has been minimized.

@hedii

hedii Dec 5, 2018

Suggested change Beta
client.post('/ip/{0}/move'.format(urllib.quote_plus(name)), to=service)
client.post('/ip/{0}/move'.format(urllib.parse.quote_plus(name)), to=service)

This comment has been minimized.

@pascalheraud

pascalheraud Dec 5, 2018

Contributor

it works perfectly for me but I'm running python 2.7. Maybe it's a python 3 issue ?
How can make code running in both python 2.7 and 3 ?

This comment has been minimized.

@hedii

This comment has been minimized.

@pascalheraud

pascalheraud Dec 10, 2018

Contributor

OK, thank you for help. I changed import of quote_plus

.format(apiError))
try:
ipproperties = client.get('/ip/{0}'.format(urllib.quote_plus(name)))

This comment has been minimized.

@hedii

hedii Dec 5, 2018

Suggested change Beta
ipproperties = client.get('/ip/{0}'.format(urllib.quote_plus(name)))
ipproperties = client.get('/ip/{0}'.format(urllib.parse.quote_plus(name)))

This comment has been minimized.

@pascalheraud

pascalheraud Dec 10, 2018

Contributor

changed here as well

def waitForNoTask(client, name, timeout):
currentTimeout = timeout
while client.get('/ip/{0}/task'.format(urllib.quote_plus(name)),

This comment has been minimized.

@hedii

hedii Dec 5, 2018

Suggested change Beta
while client.get('/ip/{0}/task'.format(urllib.quote_plus(name)),
while client.get('/ip/{0}/task'.format(urllib.parse.quote_plus(name)),

This comment has been minimized.

@pascalheraud

pascalheraud Dec 10, 2018

Contributor

changed here as well

if ipproperties['routedTo']['serviceName'] != service:
if not module.check_mode:
client.post('/ip/{0}/move'.format(urllib.quote_plus(name)), to=service)
time.sleep(3) # Delay for 3 secs to wait for task created

This comment has been minimized.

@hedii

hedii Dec 5, 2018

The 3 sec delay is not good.

We have to check with an api call if the task is created. if not, continue polling until timeout.

I just tested the module, I get a changed=true, whereas the ip was not moved yet. What happened:

  • the module created the task
  • the module wait for 3 sec (but the task is still not created yet after 3 sec)
  • the module wait for no task (waitForNoTask function)
  • the module immediately returns true for waitForNoTask, because there is no task with status='todo' and function='genericMoveFloatingIp' (not created yet after 3 sec)
  • the module ends with changed=true, but the ip move is not finished yet on the OVH side

This comment has been minimized.

@pascalheraud

pascalheraud Dec 10, 2018

Contributor

OK. I removed the 3 sec sleep and tested, the module works fine.

This comment has been minimized.

@hedii

hedii Dec 11, 2018

@pascalheraud Removing 3sec sleep does not change what I wrote above. The module exit with changed=true before the ip has moved to its new location (I just tested right now).

The move is planned, but not done.

As is, your code does not check for a task completed at all, because the task is not created yet when the waitForNoTask function is called. The way waitForNoTask function is written, it returns true if the task does not exists (it checks tasks with a status=todo). It should indeed wait for the task created and then check the task status.

PS: I am testing with ips inside public cloud projects, maybe that is why the move task creation takes longer than with other services.

This comment has been minimized.

@pascalheraud

pascalheraud Dec 11, 2018

Contributor

Ok i get it. When the task is marked completed it does not mean the move has been done. There is a delay before it's really done by OVH backend. The move can be more or less long depending of the backend's workload.

What we usually do in production after FO move : we use a bash script testing that the move is done (by calling http request for instance)

This comment has been minimized.

@hedii

hedii Dec 11, 2018

https://api.ovh.com/console/#/ip/%7Bip%7D/task#GET

status may be in:

  • cancelled
  • customerError
  • doing
  • done
  • init
  • ovhError
  • todo

When a task has the status done, the task is completed on the ovh backend. I use this when I write deployment scripts and the task status from the api reflects the real ip state. This should be handled by this module : the module exit only when we are 100% sure the ip has moved to its new location.

I am sorry I insist, and thanks for your work

This comment has been minimized.

@pascalheraud

pascalheraud Dec 11, 2018

Contributor

oh good to know, I'll try it !

This comment has been minimized.

@pascalheraud

pascalheraud Dec 11, 2018

Contributor

Your comment bumped a feature I wanted to code. The idea would be to move a set of IPs without waiting for completion, then waiting for completion of this set in a second time.

The idea would be add a parameter "wait_completion". If false, the module will not wait for the task to be completed, if true it will wait the "done" status. In this case, the module will return the created taskId

Another parameter "wait_task_completion" allows to pass a previously created taskId to wait for its completion.

By combining the two parameters, we manage to move a set of IPs then wait for completion if more optimized manner than doing it sequentially.

I'll commit code as soon as it works fine.

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 5, 2018

@pascalheraud this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@pascalheraud pascalheraud force-pushed the pascalheraud:module_ovh_ip_failover branch from 05f68da to edce0a5 Dec 5, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 5, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:0:0: E305 DOCUMENTATION.author: Invalid author for dictionary value @ data['author']. Got 'Pascal HERAUD @pascalheraud'

click here for bot help

@ansibot ansibot added the ci_verified label Dec 5, 2018

pascal heraud added some commits Dec 10, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 12, 2018

The test ansible-test sanity --test pylint [explain] failed with 10 errors:

lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:156:100: trailing-whitespace Trailing whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:189:19: bad-whitespace Exactly one space required around assignment     wait_completion=module.params.get('wait_completion')                    ^
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:190:24: bad-whitespace Exactly one space required around assignment     wait_task_completion=module.params.get('wait_task_completion')                         ^
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:191:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:232:55: trailing-whitespace Trailing whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:233:20: bad-whitespace Exactly one space required around assignment                 task=client.post('/ip/{0}/move'.format(quote_plus(name)), to=service)                     ^
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:234:22: bad-whitespace Exactly one space required around assignment                 taskId=task['taskId']                       ^
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:240:28: bad-whitespace Exactly one space required around assignment             result['taskId']=taskId                             ^
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:241:54: bad-whitespace Exactly one space required around comparison             if wait_completion or wait_task_completion!=0 :                                                       ^^
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:241:58: bad-whitespace No space allowed before :             if wait_completion or wait_task_completion!=0 :                                                           ^

The test ansible-test sanity --test pep8 [explain] failed with 16 errors:

lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:59:70: W291 trailing whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:125:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:150:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:156:101: W291 trailing whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:162:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:189:20: E225 missing whitespace around operator
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:190:25: E225 missing whitespace around operator
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:191:1: W293 blank line contains whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:232:17: E265 block comment should start with '# '
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:232:56: W291 trailing whitespace
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:233:21: E225 missing whitespace around operator
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:234:23: E225 missing whitespace around operator
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:237:17: E265 block comment should start with '# '
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:240:29: E225 missing whitespace around operator
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:241:55: E225 missing whitespace around operator
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:241:58: E203 whitespace before ':'

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:0:0: E305 DOCUMENTATION.author: Invalid author for dictionary value @ data['author']. Got 'Pascal HERAUD @pascalheraud'
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:0:0: E325 argument_spec for "wait_completion" defines type="bool" but documentation does not
lib/ansible/modules/cloud/ovh/ovh_ip_failover.py:103:28: E311 EXAMPLES is not valid YAML

click here for bot help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment