New compute driver OUTSCALE #1476

tgn-outscale · 2020-07-21T12:00:51Z

Description

This PR add a new compute driver named OUTSCALE, it was added in file types.py of compute section, in a new osc.py file for authentification in common section and in a new file named outscale.py to perform actions of the OUTSCALE API using requests. See the documentation link: https://docs.outscale.com/api

This PR is a reworked version of #1345 using python requests lib instead of using outscale python sdk.

Status

done, ready for review

Checklist (tick everything that applies)

Code linting (required, can be done after the PR checks)
Documentation (do the documentation refer to the docstring ?)
Tests
ICLA (required for bigger changes)

Happy to receive thoughs.

Signed-off-by: Tio Gobin <tio.gobin@outscale.com> Signed-off-by: pauline-p <pauline.piauger@gmail.com>

Kami · 2020-08-02T12:21:04Z

@tgn-outscale Thanks for the contribution and sorry for the delay. I will have a more detailed look a bit later.

For now, I'm just trying to understand how this fits along with the existing Outscale driver which is based on the EC2 one? Is the EC2 based one still relevant, or should we remove it?

tgn-outscale · 2020-08-03T09:38:22Z

Hi, @Kami, thanks for your reply

The Outscale EC2 driver is using our old API, and this implementation is using the new one.
As the old version is still in use and the new version still lacking some features, I think the EC2 version is still relevant for now.

Kami · 2020-08-14T14:06:26Z

@tgn-outscale Thanks for the clarification. It would be a good idea to also point this out in the driver documentation page inside docs/ directory.

Kami · 2020-08-14T14:06:54Z

libcloud/common/osc.py

+    Class which handles signing the outgoing AWS requests.
+    """
+
+    def __init__(self, access_key, access_secret, version, connection):


It you get a chance, it would also be good to add type annotations for MyPy.

Kami · 2020-08-14T14:09:06Z

libcloud/compute/drivers/outscale.py

+        """
+        action = "ReadRegions"
+        data = json.dumps({"DryRun": dry_run})
+        signer = OSCRequestSignerAlgorithmV4(access_key=self.key,


Looks like the next couple of lines are duplicated across all the driver methods.

To avoid duplication it would be good to refactor that in some common method.

In addition to that, it looks like OSCRequestSignerAlgorithmV4 could also be instantiated inside the driver constructor since it doesn't seem to depend on any method specific arguments.

Kami · 2020-08-14T14:10:17Z

libcloud/compute/drivers/outscale.py

+                                               action)
+        return requests.post(endpoint, data=data, headers=headers)
+
+    def create_public_ip(self, dry_run=False):


Methods which are not part of a standard Libcloud API need to be prefixed with ex_ - so ex_create_public_ip, ex_delete_public_ip, etc.

Kami · 2020-08-14T14:11:09Z

libcloud/compute/drivers/outscale.py

+    def create_node(self,
+                    image_id,
+                    dry_run=False,
+                    block_device_mapping=None,


Argument which are not part of the Libcloud standard API (aka are not defined on the base NodeDriver class) need to be prefixed with ex_ - ex_bsu_optimized, ex_nics, etc.

Kami · 2020-08-14T14:12:05Z

libcloud/compute/drivers/outscale.py

+                                               action)
+        return requests.post(endpoint, data=data, headers=headers)
+
+    def reboot_node(self, node_ids):


To comply with the libcloud standard API, this method needs to take a single node argument which must be Node class instance.

If you also want to support rebooting multiple notes, you should add a new extension method. For example: ex_reboot_nodes(self, node_ids).

Kami · 2020-08-14T14:12:40Z

libcloud/compute/drivers/outscale.py

+                                               action)
+        return requests.post(endpoint, data=data, headers=headers)
+
+    def list_nodes(self, data="{}"):


To comply with the base Libcloud API, this method needs to return a list of Node objects.

Kami · 2020-08-14T14:12:58Z

libcloud/compute/drivers/outscale.py

+                                               action)
+        return requests.post(endpoint, data=data, headers=headers)
+
+    def delete_node(self, node_ids):


Same here - should take a single node argument which represents Node class instance.

Kami · 2020-08-14T14:13:43Z

libcloud/compute/drivers/outscale.py

+                                               action)
+        return requests.post(endpoint, data=data, headers=headers)
+
+    def list_images(self, data="{}"):


Needs to return a list of NodeImage objects. data is a non-standard argument so it needs to be prefixed with ex_.

Kami · 2020-08-14T14:14:05Z

libcloud/compute/drivers/outscale.py

+                                               action)
+        return requests.post(endpoint, data=data, headers=headers)
+
+    def delete_image(self, image_id):


Needs to take NodeImage object.

Kami · 2020-08-14T14:16:08Z

I've added some in-line comments.

Overall, the changes mostly look good, but the code needs a bit more work to reduce duplication and to make sure the methods and method arguments comply with the base Libcloud API - that's one of the main ideas behind Libcloud - all the provider drivers should expose the same API (where possible), so users can easily switch between different providers and non-standard methods and arguments should be prefixed with ex_.

tgn-outscale · 2020-08-14T16:59:45Z

Thanks for your review @Kami, I will make these changes and come back to you as soon as possible.

Kami · 2020-08-14T20:01:57Z

You are welcome.

If you need any additional clarification on the extension method and argument naming, please let me know.

…vider

… with libcloud standard provider methods

…rganised outscale driver methods order

tgn-outscale · 2020-08-25T14:36:17Z

Hello @Kami,
I have made the changes according to what you said for more compliance with the base driver.
Let me know if I missed or misunderstood anything.

Kami · 2020-08-30T14:36:17Z

libcloud/compute/drivers/outscale.py

+    website = 'http://www.outscale.com'
+
+    def __init__(self,
+                 key: str = None,


Thanks for adding type annotations 👍

Kami · 2020-08-30T14:37:50Z

libcloud/compute/drivers/outscale.py

+                            public_ip: str = None,
+                            public_ip_id: str = None):
+        """
+        Delete instances.


Some comments are out of sync with code, but I will update / fix that when merging the pr :)

Kami · 2020-08-30T14:40:48Z

libcloud/compute/drivers/outscale.py

+        """
+        Create a new instance.
+
+        :param      ex_image_id: The ID of the OMI used to create the VM.


That's fine for now, but ideally this method would comply with the standard API in the future - this means take name, image and size arguments.

This would make it much easier for users to switch from other providers to this one without needing to change much code.

Kami · 2020-08-30T14:51:03Z

@tgn-outscale Thanks for addressing the review feedback.

I merged the PR into trunk, but there are still some places which would ideally be improved in the near future:

Try to make more methods comply with the Libcloud standard API (where possible) - this would make it easier for users to switch to this driver (right now it utilizes a lot of non-standard argument names which means Libcloud code which works with other provider drivers won't work with this one). This includes both - the argument the methods take and method return values - e.g. create_node() should take name, size and image arguments and return Node instance class (and not a dictionary). Same goes for some other methods.
Need to add some documentation with examples to docs/compute/drivers/

tgn-outscale · 2020-08-31T08:56:45Z

Thanks for your help @Kami, I will try to implement those changes when I will find time to do so,
Best regards

linaup-m and others added 12 commits July 27, 2020 14:11

Create new compute driver outscale sdk

869b0d3

Basic calls for compute driver outscale sdk

f263c39

Add test file for compute driver outscale sdk

d3810a4

added an osc signature class and some unit tests for it

d4fa505

Signed-off-by: Tio Gobin <tio.gobin@outscale.com> Signed-off-by: pauline-p <pauline.piauger@gmail.com>

outscale sdk replacement start

e34ccc7

Signed-off-by: Tio Gobin <tio.gobin@outscale.com> Signed-off-by: pauline-p <pauline.piauger@gmail.com>

All is working, before refactoring

c509400

Signed-off-by: Tio Gobin <tio.gobin@outscale.com> Signed-off-by: pauline-p <pauline.piauger@gmail.com>

Providers added

100bee9

Signed-off-by: Tio Gobin <tio.gobin@outscale.com> Signed-off-by: pauline-p <pauline.piauger@gmail.com>

Refactored

e21580f

Signed-off-by: Tio Gobin <tio.gobin@outscale.com> Signed-off-by: pauline-p <pauline.piauger@gmail.com>

Added some unit tests

69fd665

Signed-off-by: Tio Gobin <tio.gobin@outscale.com> Signed-off-by: pauline-p <pauline.piauger@gmail.com>

Contribution Guideline friendly code changes

692ae5c

Signed-off-by: Tio Gobin <tio.gobin@outscale.com> Signed-off-by: pauline-p <pauline.piauger@gmail.com>

Remove oustcale_sdk implementation

fea49fe

Signed-off-by: Tio Gobin <tio.gobin@outscale.com> Signed-off-by: pauline-p <pauline.piauger@gmail.com>

Resolve some merge errors

7dbc5f2

tgn-outscale force-pushed the trunk branch from d027b24 to 7dbc5f2 Compare July 27, 2020 12:21

tgn-outscale added 7 commits July 27, 2020 14:53

Resolve some linting errors

dc26248

changes for Flake8 compliance

87cc2df

splited every line over 79 character for Flake8 compliance

a3af73f

Flake8 indent corrections

9c1e71b

unittest reworked

d54a569

resolved a date issue in test_osc

5e7fc13

Changed an endpoint on list_nodes method for Outscale API

956c95e

Kami added api: compute drivers: outscale labels Aug 2, 2020

Kami reviewed Aug 14, 2020

View reviewed changes

type annotation for MyPy, changed signer creation and usage

64a0d90

Aikoho and others added 6 commits August 15, 2020 17:00

Worked on Outscale Provider comply more with Libcloud API

8921ede

Changed code so it use with Libcloud resources' class on Outscale Pro…

bf576d1

…vider

Changed some utils function and some return value for more compliance…

fe5e236

… with libcloud standard provider methods

modified list_location for better compliance with base Driver and reo…

882d2fa

…rganised outscale driver methods order

Flake8 and MyPy compliance for outscale driver

6f78c1a

Reduced duplication in outscale driver

48fa2fa

Kami reviewed Aug 30, 2020

View reviewed changes

asfgit merged commit 86ef1f1 into apache:trunk Aug 30, 2020

Kami mentioned this pull request Aug 30, 2020

New compute driver OUTSCALE SDK #1345

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New compute driver OUTSCALE #1476

New compute driver OUTSCALE #1476

tgn-outscale commented Jul 21, 2020 •

edited

Kami commented Aug 2, 2020

tgn-outscale commented Aug 3, 2020

Kami commented Aug 14, 2020

Kami Aug 14, 2020

Kami Aug 14, 2020

Kami Aug 14, 2020

Kami Aug 14, 2020

Kami Aug 14, 2020

Kami Aug 14, 2020

Kami Aug 14, 2020

Kami Aug 14, 2020

Kami Aug 14, 2020

Kami commented Aug 14, 2020

tgn-outscale commented Aug 14, 2020

Kami commented Aug 14, 2020

tgn-outscale commented Aug 25, 2020 •

edited

Kami Aug 30, 2020

Kami Aug 30, 2020

Kami Aug 30, 2020

Kami commented Aug 30, 2020

tgn-outscale commented Aug 31, 2020

New compute driver OUTSCALE #1476

New compute driver OUTSCALE #1476

Conversation

tgn-outscale commented Jul 21, 2020 • edited

Description

Status

Checklist (tick everything that applies)

Kami commented Aug 2, 2020

tgn-outscale commented Aug 3, 2020

Kami commented Aug 14, 2020

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

Kami commented Aug 14, 2020

tgn-outscale commented Aug 14, 2020

Kami commented Aug 14, 2020

tgn-outscale commented Aug 25, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kami commented Aug 30, 2020

tgn-outscale commented Aug 31, 2020

tgn-outscale commented Jul 21, 2020 •

edited

tgn-outscale commented Aug 25, 2020 •

edited