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

Add hetzner as cloud provider #59301

Merged
merged 4 commits into from
Feb 1, 2021
Merged

Conversation

fkantelberg
Copy link
Contributor

@fkantelberg fkantelberg commented Jan 15, 2021

What does this PR do?

This PR adds the Hetzner GmbH cloud as cloud provider to salt-cloud. It wraps around the hcloud package provided by Hetzner GmbH.

Merge requirements satisfied?

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

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@fkantelberg fkantelberg requested a review from a team as a code owner January 15, 2021 14:19
@fkantelberg fkantelberg requested review from Ch3LL and removed request for a team January 15, 2021 14:19
@welcome
Copy link

welcome bot commented Jan 15, 2021

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at core@saltstack.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires test coverage for all of the new functionality and a changelog entry. Since we do not have access to the hetzner cloud, unit tests will be the best approach for now.

@fkantelberg fkantelberg force-pushed the cloud-provider-hetzner branch 2 times, most recently from dc9ce09 to b57d049 Compare January 22, 2021 09:32
@fkantelberg
Copy link
Contributor Author

@Ch3LL Thank you. As requested I added the unittests and the changelog entry.

But I have no idea why the debian9 pytest is failing. Is this related to my changed when I see problems in FAILED tests/integration/modules/test_state.py::StateModuleTest::test_requisites_onfail_all?

salt/cloud/clouds/hetzner.py Outdated Show resolved Hide resolved
salt/cloud/clouds/hetzner.py Outdated Show resolved Hide resolved
salt/cloud/clouds/hetzner.py Outdated Show resolved Hide resolved
salt/cloud/clouds/hetzner.py Outdated Show resolved Hide resolved
salt/cloud/clouds/hetzner.py Show resolved Hide resolved

KEY = "abcdefgh"

VM_NAME = "myserver"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try to stray away from these module level variables as they can cause headache in the future and hard to find if anyone ever ends up importing this test module.

Also another ask, we have recently migrated to pytest. Would you mind migrating this test to tests/pytests/unit/cloud/. You will need to create the cloud directory since there isn't currently one there. thanks :)

Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to ignore that test failure. I am re-running it to see if it fails again

fkantelberg and others added 2 commits January 25, 2021 16:10
Co-authored-by: Megan Wilhite <megan.wilhite@gmail.com>
Copy link
Contributor Author

@fkantelberg fkantelberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I migrated the tests to pytest and fixed the mentioned comments and all unittests seem to be green now.

salt/cloud/clouds/hetzner.py Show resolved Hide resolved
Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Ch3LL Ch3LL added the Aluminium Release Post Mg and Pre Si label Jan 28, 2021
@Ch3LL Ch3LL merged commit fbc5522 into saltstack:master Feb 1, 2021
@welcome
Copy link

welcome bot commented Feb 1, 2021

Congratulations on your first PR being merged! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aluminium Release Post Mg and Pre Si
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants