Initial commit for Scaleway Compute #38285

remyleone · 2018-04-04T16:24:54Z

SUMMARY

This PR adds support for the Scaleway Compute API.
It can create, start, stop, restart and destroy Scaleway Compute instances.

It also got a playbook to test the different steps.

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

scaleway_compute

ANSIBLE VERSION

ansible 2.6.0 (scaleway_compute 6ecb37354e) last updated 2018/04/04 18:20:23 (GMT +200)
  config file = None
  configured module search path = [u'/Users/sieben/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/sieben/workspace/ansible/lib/ansible
  executable location = /Users/sieben/workspace/ansible/bin/ansible
  python version = 2.7.14 (default, Mar  9 2018, 23:57:12) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.39.2)]

remyleone · 2018-04-05T12:54:51Z

lib/ansible/modules/cloud/scaleway/scaleway_compute.py

+        compute_api.module.fail_json(msg="Error while checking if attributes should be changed")
+
+
+def server_change_attributes():


This function is being worked on.

remyleone · 2018-04-05T14:28:42Z

Would you suggest using an object containing the different methods? Pro: it would avoid passing the compute_api object around. Cons: This class might get quite big...

pilou- · 2018-04-05T15:20:26Z

lib/ansible/modules/cloud/scaleway/scaleway_compute.py

+)
+
+
+def print_warning(compute_api, msg):


Not sure if module.debug should be used instead.

pilou- · 2018-04-05T15:25:04Z

lib/ansible/modules/cloud/scaleway/scaleway_compute.py

+        return self.status_code in (200, 201, 202, 204)
+
+
+class ScalewayComputeAPI(object):


This class should be moved to module_utils/scaleway.py and reused by scaleway_sshkey module.

pilou- · 2018-04-05T15:26:08Z

lib/ansible/modules/cloud/scaleway/scaleway_compute.py

+        return self.send("UPDATE", path, data, headers)
+
+
+def is_present(compute_api, server):


This method is unused and must be removed.

pilou- · 2018-04-05T15:29:02Z

lib/ansible/modules/cloud/scaleway/scaleway_compute.py

+
+    if wait:
+        print_warning(compute_api, "(create_server) Wait is enabled")
+        wait_to_complete_state_transition(compute_api=compute_api, server=target_server)


test of wait parameter could be moved inside the wait_to_complete_state_transition method.

pilou- · 2018-04-05T15:30:40Z

lib/ansible/modules/cloud/scaleway/scaleway_compute.py

+    return response
+
+
+def start_server(compute_api, server):


methods start_server/stop_server/restart_server are almost the same and could be factorized.

pilou- · 2018-04-05T15:38:17Z

lib/ansible/modules/cloud/scaleway/scaleway_compute.py

+
+    if not query_results:
+        changed = True
+        if compute_api.module.check_mode:


Here and below: I would prefer using something like return changed, ... than calling compute_api.module.exit_json because the caller calls exit_json anyway.

pilou- · 2018-04-05T15:41:27Z

lib/ansible/modules/cloud/scaleway/scaleway_compute.py

+            msg = 'Error while stopping server [{0}: {1}]'.format(response.status_code, response.json)
+            compute_api.module.fail_json(msg=msg)
+
+    return changed, {"status": "stopped"}


Should not a string be used instead of {"status": "stopped"} (the same for restart_strategy method) (because dict is used as msg parameter there) ?

pilou- · 2018-04-05T15:43:26Z

lib/ansible/modules/cloud/scaleway/scaleway_compute.py

+                required=True,
+            ),
+            image=dict(required=True),
+            name=dict(),


In the module documentation name is required.

A default name will be provided if a name is missing, therefore, it's not a required parameter. I've fixed the documentation.

maxamillion · 2018-04-05T21:44:38Z

+1 the the feedback from @pilou- and other than that I think the code looks good. Please let me know when it's complete (no longer a Work In Progress) and I'm happy to provide a final review.

ansibot · 2018-04-06T12:31:30Z

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

lib/ansible/modules/cloud/scaleway/scaleway_compute.py:522:86: E128 continuation line under-indented for visual indent
lib/ansible/modules/cloud/scaleway/scaleway_compute.py:530:86: E128 continuation line under-indented for visual indent

click here for bot help

remyleone · 2018-04-06T14:24:02Z

@maxamillion I think the patch is ready for its final review :)

ansibot · 2018-04-06T14:39:57Z

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/cloud/scaleway/scaleway_compute.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/cloud/scaleway/scaleway_compute.py:554:11: SyntaxError: for x in PATCH_MUTABLE_SERVER_ATTRIBUTES

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/cloud/scaleway/scaleway_compute.py:554:11: SyntaxError: invalid syntax

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

lib/ansible/modules/cloud/scaleway/scaleway_compute.py:215:1: E303 too many blank lines (3)

click here for bot help

remyleone mentioned this pull request Apr 5, 2018

Public Ansible Project Meeting Agenda - Apr 2018 ansible/community#301

Closed

remyleone commented Apr 5, 2018

View reviewed changes

remyleone changed the title ~~Initial commit for Scaleway Compute~~ [WIP] - Initial commit for Scaleway Compute Apr 5, 2018

ansibot added the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Apr 5, 2018

pilou- reviewed Apr 5, 2018

View reviewed changes

samdoran removed the needs_triage Needs a first human triage before being processed. label Apr 5, 2018

samdoran requested a review from maxamillion April 5, 2018 17:05

remyleone force-pushed the scaleway_compute branch 5 times, most recently from 09108b3 to 5281a87 Compare April 6, 2018 12:22

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Apr 6, 2018

remyleone force-pushed the scaleway_compute branch from 5281a87 to dfec9e4 Compare April 6, 2018 12:32

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Apr 6, 2018

remyleone force-pushed the scaleway_compute branch from dfec9e4 to 279b203 Compare April 6, 2018 14:15

ansibot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. labels Apr 6, 2018

remyleone force-pushed the scaleway_compute branch from 279b203 to b786850 Compare April 6, 2018 14:22

remyleone changed the title ~~[WIP] - Initial commit for Scaleway Compute~~ Initial commit for Scaleway Compute Apr 6, 2018

ansibot removed the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Apr 6, 2018

ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Apr 6, 2018

remyleone force-pushed the scaleway_compute branch from b786850 to 9d6d44e Compare April 6, 2018 16:03

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Apr 6, 2018

Initial commit for Scaleway Compute

9476ca0

remyleone force-pushed the scaleway_compute branch from 9d6d44e to 9476ca0 Compare April 6, 2018 16:09

ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Apr 6, 2018

maxamillion approved these changes Apr 9, 2018

View reviewed changes

maxamillion merged commit eccccfe into ansible:devel Apr 9, 2018

ilicmilan pushed a commit to ilicmilan/ansible that referenced this pull request Nov 7, 2018

Initial commit for Scaleway Compute (ansible#38285)

a2971ee

ansible locked and limited conversation to collaborators Apr 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial commit for Scaleway Compute #38285

Initial commit for Scaleway Compute #38285

remyleone commented Apr 4, 2018

remyleone Apr 5, 2018

remyleone Apr 6, 2018

remyleone commented Apr 5, 2018

pilou- Apr 5, 2018

pilou- Apr 5, 2018

pilou- Apr 5, 2018

pilou- Apr 5, 2018

pilou- Apr 5, 2018

pilou- Apr 5, 2018

remyleone Apr 6, 2018

pilou- Apr 5, 2018

pilou- Apr 5, 2018

remyleone Apr 6, 2018

maxamillion commented Apr 5, 2018

ansibot commented Apr 6, 2018

remyleone commented Apr 6, 2018

ansibot commented Apr 6, 2018

		compute_api.module.fail_json(msg="Error while checking if attributes should be changed")


		def server_change_attributes():

		return self.status_code in (200, 201, 202, 204)


		class ScalewayComputeAPI(object):

		return self.send("UPDATE", path, data, headers)


		def is_present(compute_api, server):

Initial commit for Scaleway Compute #38285

Initial commit for Scaleway Compute #38285

Conversation

remyleone commented Apr 4, 2018

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

Choose a reason for hiding this comment

Choose a reason for hiding this comment

remyleone commented Apr 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxamillion commented Apr 5, 2018

ansibot commented Apr 6, 2018

remyleone commented Apr 6, 2018

ansibot commented Apr 6, 2018