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

Salt Cloud fixes #59374

Merged
merged 13 commits into from Feb 19, 2021
Merged

Salt Cloud fixes #59374

merged 13 commits into from Feb 19, 2021

Conversation

@s0undt3ch s0undt3ch force-pushed the hotfix/cloud branch 3 times, most recently from f8fca46 to 3e76290 Compare February 1, 2021 17:46
@s0undt3ch s0undt3ch marked this pull request as ready for review February 1, 2021 19:28
@s0undt3ch s0undt3ch requested a review from a team as a code owner February 1, 2021 19:28
@s0undt3ch s0undt3ch requested review from Akm0d and removed request for a team February 1, 2021 19:28
@s0undt3ch s0undt3ch added this to the Aluminium milestone Feb 1, 2021
@s0undt3ch s0undt3ch force-pushed the hotfix/cloud branch 8 times, most recently from d663d28 to 14bf8bb Compare February 5, 2021 12:48
krionbsd
krionbsd previously approved these changes Feb 5, 2021
@bryceml
Copy link
Contributor

bryceml commented Feb 6, 2021

@s0undt3ch do we need to update libcloud then? That's going to be a bit of work, most oses provide 2.2 I think.

@s0undt3ch
Copy link
Member Author

The libcloud requirent stays at 1.5.0, however, to use GCE, 2.5.0 is required.
Authenticating against GCE simplify fails with older versions.
Seems related to the fact that we no longer use Pycrypto and libcloud stops using Pycrypto on 2.5.0

krionbsd
krionbsd previously approved these changes Feb 9, 2021
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.

I do not think we are running joyent integration tests anymore. If so can we get some test coverage for that cloud provider since there was multiple changes.

salt/loader.py Show resolved Hide resolved
requirements/static/ci/common.in Show resolved Hide resolved
@Ch3LL
Copy link
Contributor

Ch3LL commented Feb 9, 2021

Yes, unit tests. There is joyent unit tests currently but would be good to add one for the new change with node state (for example this method: joyent_node_state). I'm fine adding the tests in a separate PR.

Ch3LL
Ch3LL previously approved these changes Feb 9, 2021
@s0undt3ch
Copy link
Member Author

FYI, no new functionality was added to joyent. It was relying on a function from libcloud which no longer makes sense, but since we don't run integration tests against joyent, I just copied that function to joyent in order not to break it, that was it.

garethgreenaway
garethgreenaway previously approved these changes Feb 9, 2021
```python
Traceback (most recent call last):
  File "/tmp/kitchen/testing/salt/cloud/__init__.py", line 2349, in run_parallel_map_providers_query
    salt.utils.data.simple_types_filter(cloud.clouds[data["fun"]]()),
  File "/tmp/kitchen/testing/salt/loader.py", line 1182, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/tmp/kitchen/testing/salt/loader.py", line 2204, in run
    return self._last_context.run(self._run_as, method, *args, **kwargs)
  File "/tmp/kitchen/testing/.nox/pytest-cloud-3-coverage-false/lib/python3.6/site-packages/contextvars/__init__.py", line 38, in run
    return callable(*args, **kwargs)
  File "/tmp/kitchen/testing/salt/loader.py", line 2219, in _run_as
    return method(*args, **kwargs)
  File "/tmp/kitchen/testing/salt/cloud/clouds/vultrpy.py", line 163, in list_nodes
    nodes = list_nodes_full()
  File "/tmp/kitchen/testing/salt/cloud/clouds/vultrpy.py", line 176, in list_nodes_full
    nodes = _query("server/list")
  File "/tmp/kitchen/testing/salt/cloud/clouds/vultrpy.py", line 530, in _query
    opts=__opts__,
  File "/tmp/kitchen/testing/salt/loader.py", line 1182, in __call__
    return self.loader.run(run_func, *args, **kwargs)
TypeError: run() got multiple values for argument 'method'
```
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
6 participants