Add Linode driver (v3 API) by decentral1se · Pull Request #1458 · ansible/molecule

decentral1se · 2018-08-29T18:51:53Z

This is still a work in progress. I've manually tested parts of it but not a full end to end use.

Just wanted to get this up here to see that it is all going in the right direction in case I miss something.

Will close #1450.

retr0h · 2018-08-31T03:11:44Z

Looking good so far.... I suggest looking at #1436 and how the model was implemented along with tests. Docker and vagrant are the only drivers which really implement an API from molecule to create.yml. Might be good to define that as part of this.

As a result of direction in the following comment: > ansible#1458 (comment) And the good work and lead of ansible#1436 in: > ansible#1436

decentral1se · 2018-09-09T23:18:03Z

I suggest looking at #1436 and how the model was implemented along with tests. Docker and vagrant are the only drivers which really implement an API from molecule to create.yml. Might be good to define that as part of this.

OK, I've added the model support in d3484c0, I think.

There are now unit tests in place to guard for a proper schema:

(.venv) ➜  molecule (add-linode-driver) ✔ py.test test/unit/model/v2/ -k linode
=============================================================== test session starts ===============================================================
platform linux2 -- Python 2.7.13, pytest-3.0.7, py-1.6.0, pluggy-0.4.0
rootdir: /home/decentral1se/hobby/molecule, inifile: pytest.ini
plugins: testinfra-1.14.1, verbose-parametrize-1.3.0, mock-1.10.0, helpers-namespace-2017.11.11, cov-2.6.0
collected 59 items 

test/unit/model/v2/test_platforms_section.py .......

=============================================================== 52 tests deselected ===============================================================
===================================================== 7 passed, 52 deselected in 0.16 seconds =====================================================
(.venv) ➜  molecule (add-linode-driver) ✔

Unfortunately, I've found a fairly major bug while E2E testing this which stops this feature from actually being any use at all. That is reported at ansible/ansible#45403 where the bug stops the Linode module destroying the instances it creates 🤒

I'll be heading back over that way to fix that bug. Hopefully, I can get back here in a reasonable time.

decentral1se · 2018-09-14T17:29:07Z

OK, false alarm on ansible/ansible#45403, it was just a docs issue (submitted ansible/ansible#45659 for that). I'll explain the short of it, since it matters here: when a linode is created a linode_id is generated and that is needed to specify that linode on deletion. So, specifying linode_id in the model doesn't make sense because it won't match the generated value, so I am going to try and adapt my destroy.yml to match what is in the ec2/destroy.yml using the molecule_instance_config lookup.

Closes ansible#1458.

decentral1se · 2018-09-17T09:44:43Z

OK, @retr0h, this is ready for a review!

I see I just missed the latest release but perhaps this is better off on the devel branch for now ...

So, the manual testing I've done for this:

molecule init role -r foobar -d linode
molecule test
add 3 instances and run molecule test
molecule create && molecule login --host instance1

It seems to be in working order!

decentral1se · 2018-09-17T10:10:59Z

Related: #1479.

decentral1se · 2018-09-18T07:44:37Z

@retr0h, can I clarify if you're leaning towards rejecting this PR or accepting it?

Closes ansible#1458.

retr0h · 2018-09-20T01:04:21Z

@lwm Linode has been one I wanted for quite a while.

retr0h · 2018-09-20T01:06:23Z

@lwm Nicely implemented! I know adding drivers is a pain.

retr0h · 2018-09-20T01:16:07Z

@lwm Can you please run tox -e format. Looks like formatting is off.

Closes ansible#1458.

decentral1se · 2018-09-20T07:25:49Z

@lwm Linode has been one I wanted for quite a while. @lwm Nicely implemented! I know adding drivers is a pain.

✋ ✋ ✋

@lwm Can you please run tox -e format. Looks like formatting is off.

Done and rebased!

retr0h · 2018-09-20T22:36:37Z

This is missing a bunch of unit tests. I'm going through it now on my checkout. Hold off on submitting any additional work.

decentral1se · 2018-09-23T12:19:23Z

If it makes you feel any better, we're already using this across multiple roles on every CI run over at https://git.coop/aptivate/ansible-roles. Some logs, for example are over at https://git.coop/aptivate/ansible-roles/mysql-server/-/jobs/4071. Let me know if I can add more testing to help.

retr0h · 2018-10-04T15:06:36Z

Sorry for the delay on this @lwm -- when I add the unit tests for the linode driver, I am getting all kinds of other problems, which means I stumbled across a bug in how I test drivers. I have been busy the last couple weeks and apologize for not looking into this sooner.

decentral1se · 2018-10-04T19:11:21Z

No worries! Thanks for the update. Happy to have helped smoke out a new bug :)

gundalow · 2018-10-08T19:14:54Z

Just in case others look at this, Linode & Molecule both have Working Groups within IRC
#ansible-linode https://github.com/ansible/community/tree/master/group-linode
#molecule-users

retr0h · 2018-10-10T03:36:02Z

I'm going to hold off from merging this. As you know Ansible is adopting Molecule, therefore I do not want to merge anything that may impact future architectural decisions, as I will no longer be leading the development of this project.

webknjaz · 2018-12-09T19:48:10Z

Thanks for the explanation. No ideas so far but I believe you're probably right about broken teardown: I've fixed numerous problems with it already after the acquisition and I expect that there may be more issues left unnoticed.
We'll need to tackle schema validation closer now.

gundalow · 2018-12-12T12:00:08Z

Do we need something like https://github.com/ansible/ansible/blob/devel/test/sanity/validate-modules/schema.py to validate the he schema?

webknjaz · 2018-12-12T15:56:17Z

@gundalow there's schema validation built-in Molecule already: https://github.com/ansible/molecule/blob/master/molecule/model/schema_v2.py

decentral1se · 2019-01-01T20:13:38Z

#1639 might get me moving forward ...

Closes ansible#1458. Signed-off-by: Luke Murphy <lukewm@riseup.net>

decentral1se · 2019-01-15T22:58:55Z

OK, I'm still waiting for the build to run have seen it all go green on my local. @webknjaz, can you give this another pass? There are 3 additional commits now which aim to fix the failing tests.

decentral1se · 2019-01-16T11:05:33Z

CI IS GREEN

EDIT: (very intense GIF) https://media0.giphy.com/media/26tPghhb310muUkEw/giphy.gif?cid=3640f6095c3f0fa370336b3655c45568)

webknjaz

There's a few minor things to fix

webknjaz · 2019-01-17T13:13:25Z

...driver/linode/{{cookiecutter.molecule_directory}}/{{cookiecutter.scenario_name}}/INSTALL.rst

+
+.. code-block:: bash
+
+    $ pip install linode-python


I think you should add it since all other drivers in master have their extras entries now.

molecule/driver/linode.py

test/unit/driver/test_linode.py

test/unit/model/v2/test_platforms_section.py

Closes ansible#1458. Signed-off-by: Luke Murphy <lukewm@riseup.net>

@mickelmayers

Full credit to @mickelmayers in ansible#1639. Originall submitted with message like: Platforms_base_schema destroy copy.deepcopy(base_schema). Then when validating multiply molecules it's running with errors. Eg. two moleule with different drivers. One Azure second docker . If docker will be validated first from it schema we are getting. Moving platforms_base_schema to base_schema and changing the util.merge_dicts fixes the issue. Closes ansible#1639. Signed-off-by: Luke Murphy <lukewm@riseup.net>

This change works towards stopping schema merging clobbering themselves. Without this change, the `test_dependency_shell_has_errors` test is failing due to an incorrect schema. Signed-off-by: Luke Murphy <lukewm@riseup.net>

The `test_platforms_docker_has_errors` test is failing otherwise. Signed-off-by: Luke Murphy <lukewm@riseup.net>

decentral1se · 2019-01-17T22:25:13Z

I think you should add it since all other drivers in master have their extras entries now.

Ah, it was already added. So, fixed that documentation now as well.

I've addressed all your comments and pushed. Ready for another pass.

gundalow · 2019-01-18T08:19:32Z

Thank you to everybody that's help with this!

webknjaz · 2019-01-18T10:44:35Z

🎉

@mickelmayers

* Add Linode driver. Closes ansible#1458. Signed-off-by: Luke Murphy <lukewm@riseup.net> * Fix incorrect copying of schema for validation. Full credit to @mickelmayers in ansible#1639. Originall submitted with message like: Platforms_base_schema destroy copy.deepcopy(base_schema). Then when validating multiply molecules it's running with errors. Eg. two moleule with different drivers. One Azure second docker . If docker will be validated first from it schema we are getting. Moving platforms_base_schema to base_schema and changing the util.merge_dicts fixes the issue. Closes ansible#1639. Signed-off-by: Luke Murphy <lukewm@riseup.net> * Merge schema only once. This change works towards stopping schema merging clobbering themselves. Without this change, the `test_dependency_shell_has_errors` test is failing due to an incorrect schema. Signed-off-by: Luke Murphy <lukewm@riseup.net> * Remove incorrect driver for fixture. The `test_platforms_docker_has_errors` test is failing otherwise. Signed-off-by: Luke Murphy <lukewm@riseup.net>

decentral1se force-pushed the add-linode-driver branch from 35c4051 to d3484c0 Compare September 9, 2018 23:15

decentral1se added a commit to decentral1se/molecule that referenced this pull request Sep 9, 2018

Add model testing.

d3484c0

As a result of direction in the following comment: > ansible#1458 (comment) And the good work and lead of ansible#1436 in: > ansible#1436

decentral1se force-pushed the add-linode-driver branch from d1e67d6 to f753d41 Compare September 17, 2018 09:42

decentral1se added a commit to decentral1se/molecule that referenced this pull request Sep 17, 2018

Add Linode driver.

f753d41

Closes ansible#1458.

decentral1se changed the title ~~WIP: Add Linode driver~~ Add Linode driver Sep 17, 2018

decentral1se mentioned this pull request Sep 17, 2018

molecule-driver-contrib: community managed drivers #1479

Closed

decentral1se mentioned this pull request Sep 17, 2018

Support Linode as a test driver #1450

Closed

decentral1se force-pushed the add-linode-driver branch from f753d41 to 4b9268d Compare September 18, 2018 08:25

decentral1se added a commit to decentral1se/molecule that referenced this pull request Sep 18, 2018

Add Linode driver.

4b9268d

Closes ansible#1458.

retr0h added the v2.19 label Sep 20, 2018

decentral1se force-pushed the add-linode-driver branch from 4b9268d to 19e9d89 Compare September 20, 2018 07:24

decentral1se added a commit to decentral1se/molecule that referenced this pull request Sep 20, 2018

Add Linode driver.

19e9d89

Closes ansible#1458.

decentral1se force-pushed the add-linode-driver branch from 19e9d89 to e0ab6a7 Compare September 20, 2018 07:25

decentral1se added a commit to decentral1se/molecule that referenced this pull request Sep 20, 2018

Add Linode driver.

e0ab6a7

Closes ansible#1458.

decentral1se mentioned this pull request Jan 1, 2019

Update schema_v2.py #1639

Closed

decentral1se force-pushed the add-linode-driver branch from a92ca92 to 80f5fdf Compare January 2, 2019 00:29

decentral1se added a commit to decentral1se/molecule that referenced this pull request Jan 2, 2019

Add Linode driver.

98fa728

Closes ansible#1458. Signed-off-by: Luke Murphy <lukewm@riseup.net>

decentral1se force-pushed the add-linode-driver branch from 80f5fdf to 70e3543 Compare January 15, 2019 21:08

decentral1se added a commit to decentral1se/molecule that referenced this pull request Jan 15, 2019

Add Linode driver.

bf5327b

Closes ansible#1458. Signed-off-by: Luke Murphy <lukewm@riseup.net>

decentral1se force-pushed the add-linode-driver branch 2 times, most recently from 1d3534d to d9401d3 Compare January 15, 2019 21:31

webknjaz suggested changes Jan 17, 2019

View reviewed changes

decentral1se and others added 4 commits January 17, 2019 23:19

Add Linode driver.

7e1ca00

Closes ansible#1458. Signed-off-by: Luke Murphy <lukewm@riseup.net>

Merge schema only once.

ce809bd

This change works towards stopping schema merging clobbering themselves. Without this change, the `test_dependency_shell_has_errors` test is failing due to an incorrect schema. Signed-off-by: Luke Murphy <lukewm@riseup.net>

Remove incorrect driver for fixture.

b5dc640

The `test_platforms_docker_has_errors` test is failing otherwise. Signed-off-by: Luke Murphy <lukewm@riseup.net>

decentral1se force-pushed the add-linode-driver branch from 6592601 to b5dc640 Compare January 17, 2019 22:24

decentral1se changed the title ~~Add Linode driver~~ Add Linode driver (v3 API) Jan 17, 2019

gundalow approved these changes Jan 18, 2019

View reviewed changes

gundalow merged commit f09b039 into ansible:master Jan 18, 2019

decentral1se deleted the add-linode-driver branch January 18, 2019 10:47

decentral1se mentioned this pull request Jan 18, 2019

Migrate to the new linode_v4 module #1678

Closed

This was referenced Feb 25, 2019

Add change log entries for v2.20 release #1772

Closed

Respect PY_COLORS variable #1533

Merged

Conversation

decentral1se commented Aug 29, 2018

Uh oh!

retr0h commented Aug 31, 2018

Uh oh!

decentral1se commented Sep 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

decentral1se commented Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

decentral1se commented Sep 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

decentral1se commented Sep 17, 2018

Uh oh!

decentral1se commented Sep 18, 2018

Uh oh!

retr0h commented Sep 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

retr0h commented Sep 20, 2018

Uh oh!

retr0h commented Sep 20, 2018

Uh oh!

decentral1se commented Sep 20, 2018

Uh oh!

retr0h commented Sep 20, 2018

Uh oh!

decentral1se commented Sep 23, 2018

Uh oh!

retr0h commented Oct 4, 2018

Uh oh!

decentral1se commented Oct 4, 2018

Uh oh!

gundalow commented Oct 8, 2018

Uh oh!

retr0h commented Oct 10, 2018

Uh oh!

webknjaz commented Dec 9, 2018

Uh oh!

gundalow commented Dec 12, 2018

Uh oh!

webknjaz commented Dec 12, 2018

Uh oh!

decentral1se commented Jan 1, 2019

Uh oh!

decentral1se commented Jan 15, 2019

Uh oh!

decentral1se commented Jan 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webknjaz left a comment

Choose a reason for hiding this comment

Uh oh!

webknjaz Jan 17, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

decentral1se commented Jan 17, 2019

Uh oh!

gundalow commented Jan 18, 2019

Uh oh!

webknjaz commented Jan 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

decentral1se commented Sep 9, 2018 •

edited

Loading

decentral1se commented Sep 14, 2018 •

edited

Loading

decentral1se commented Sep 17, 2018 •

edited

Loading

retr0h commented Sep 20, 2018 •

edited

Loading

decentral1se commented Jan 16, 2019 •

edited

Loading