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.

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

Linode apiv4 support #58093

Merged
merged 11 commits into from Sep 8, 2020
Merged

Linode apiv4 support #58093

merged 11 commits into from Sep 8, 2020

Conversation

0xch4z
Copy link
Contributor

@0xch4z 0xch4z commented Jul 30, 2020

What does this PR do?

This PR adds opt-in Linode APIv4 support to the Linode cloud module via the api_version cloud configuration parameter.

What issues does this PR fix or reference?

Fixes: #56319

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

@0xch4z 0xch4z requested a review from a team as a code owner July 30, 2020 13:27
@ghost ghost requested review from twangboy and removed request for a team July 30, 2020 13:27
@garethgreenaway
Copy link
Member

re-run ubuntu

@garethgreenaway
Copy link
Member

re-run windows

@garethgreenaway
Copy link
Member

re-run docs

@0xch4z 0xch4z force-pushed the linode-apiv4-support branch 2 times, most recently from 5c68864 to 045b242 Compare August 11, 2020 18:47
@0xch4z
Copy link
Contributor Author

0xch4z commented Aug 12, 2020

@dwoz, @garethgreenaway, @twangboy Can you take another look when you get a sec 😃

The lint stage is failing because of unrelated changes, but all tests are passing now.

@bryceml
Copy link
Contributor

bryceml commented Aug 19, 2020

We should actually run this against the cloudtests, doing that here: #58248

The links are wierd because github records statuses by commit, rather than commit on a pr.

@bryceml
Copy link
Contributor

bryceml commented Aug 19, 2020

Also, all of the class docs show up in the html docs which makes them not too pretty. I think there's a flag in sphinx to hide them, but I can't remember what it is.

@bryceml
Copy link
Contributor

bryceml commented Aug 19, 2020

no linode cloud test failures, so we're good there.

Test Result (8 failures )

    integration.cloud.clouds.test_ec2.EC2Test.test_instance
    integration.cloud.clouds.test_ec2.EC2Test.test_instance_rename
    integration.cloud.clouds.test_ec2.EC2Test.test_win2012r2_psexec
    integration.cloud.clouds.test_ec2.EC2Test.test_win2012r2_winrm
    integration.cloud.clouds.test_ec2.EC2Test.test_win2016_psexec
    integration.cloud.clouds.test_ec2.EC2Test.test_win2016_winrm
    integration.cloud.clouds.test_gce.GCETest.test_instance_extra
    integration.cloud.clouds.test_msazure.AzureTest.test_instance

which is the same as the master branch currently.

@bryceml
Copy link
Contributor

bryceml commented Aug 20, 2020

something like

diff --git a/doc/ref/clouds/all/salt.cloud.clouds.linode.rst b/doc/ref/clouds/all/salt.cloud.clouds.linode.rst
index 30f987dcbc..d5b0c23411 100644
--- a/doc/ref/clouds/all/salt.cloud.clouds.linode.rst
+++ b/doc/ref/clouds/all/salt.cloud.clouds.linode.rst
@@ -3,4 +3,5 @@ salt.cloud.clouds.linode
 ========================
 
 .. automodule:: salt.cloud.clouds.linode
-    :members:
\ No newline at end of file
+    :members:
+    :exclude-members: LinodeAPI, LinodeAPIv3, LinodeAPIv4

should fix up the docs

@0xch4z
Copy link
Contributor Author

0xch4z commented Aug 24, 2020

@bryceml all fixed!

@0xch4z 0xch4z requested a review from bryceml August 24, 2020 16:20
@0xch4z
Copy link
Contributor Author

0xch4z commented Sep 4, 2020

@Akm0d @sagetherage @bryceml Anything I can do to get this merged?

@sagetherage
Copy link
Contributor

I believe we have been working to fix test failures (outside of this PR) before merging new code, so it seems we are delayed or forgetful, however the team is focused on fixing test suite issues so merging is smoother in the near future. That being said, I will see about this today and early next week to get it merged. Thank you for the mention!

@sagetherage sagetherage added Magnesium Mg release after Na prior to Al Salt-Cloud Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around labels Sep 4, 2020
@dwoz dwoz merged commit 00af98a into saltstack:master Sep 8, 2020
28 checks passed
@0xch4z
Copy link
Contributor Author

0xch4z commented Sep 8, 2020

Thanks, everyone for getting this merged in!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Magnesium Mg release after Na prior to Al Salt-Cloud severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

Successfully merging this pull request may close these issues.

salt-cloud doesn't support linode v4 API (and it's now impossible to get v3 API keys)
7 participants