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

profitbricks: support for update state, check mode, custom API URL & other improvements #34834

Open
wants to merge 23 commits into
base: devel
from

Conversation

@nurfet-becirevic

nurfet-becirevic commented Jan 13, 2018

SUMMARY

This update contains new features and other changes including:

  • Added support for update state. The following ProfitBricks components can be updated now: datacenters, servers, NICs and volumes.
  • Added support for check mode.
  • Added support for custom ProfitBricks API URL.
  • Deprecated redundant module profitbricks_volume_attachments. The module functionality is handled by profitbricks module.
  • Added profitbricks module to module_utils grouping common stuff from ProfitBricks cloud modules.
  • Added ProfitBricks credential parameters username & password replacing subscription_user and subscription_password which are defined as aliases now.
  • Other ProfitBricks related updates: support for API v4, request headers info, missing parameters in the modules etc.
  • Fixed/updated documentation.
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

various ProfitBricks modules

ANSIBLE VERSION
2.6
ADDITIONAL INFORMATION
Several new ProfitBricks modules and dynamic inventory are waiting in the queue for PR. They have been omitted here to facilitate the review.  
@ansibot

This comment has been minimized.

Contributor

ansibot commented Jan 13, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jan 13, 2018

The test ansible-test sanity --test import --python 2.6 [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:249:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:109:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:146:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:177:0: ImportError: cannot import name __version__

The test ansible-test sanity --test import --python 2.7 [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:249:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:109:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:146:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:177:0: ImportError: cannot import name __version__

The test ansible-test sanity --test import --python 3.5 [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:249:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:109:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:146:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:177:0: ImportError: cannot import name '__version__'

The test ansible-test sanity --test import --python 3.6 [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:249:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:109:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:146:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:177:0: ImportError: cannot import name '__version__'

The test ansible-test sanity --test import --python 3.7 [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:249:0: ImportError: cannot import name '__version__' from 'ansible' (/root/ansible/test/runner/import/lib/ansible/__init__.py)
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:109:0: ImportError: cannot import name '__version__' from 'ansible' (/root/ansible/test/runner/import/lib/ansible/__init__.py)
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:146:0: ImportError: cannot import name '__version__' from 'ansible' (/root/ansible/test/runner/import/lib/ansible/__init__.py)
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:177:0: ImportError: cannot import name '__version__' from 'ansible' (/root/ansible/test/runner/import/lib/ansible/__init__.py)

The test ansible-test sanity --test pylint [?] failed with the following errors:

lib/ansible/module_utils/profitbricks.py:39:0: anomalous-backslash-in-string Anomalous backslash in string: '\w'. String constant might be missing an r prefix.
lib/ansible/module_utils/profitbricks.py:39:0: anomalous-backslash-in-string Anomalous backslash in string: '\w'. String constant might be missing an r prefix.
lib/ansible/module_utils/profitbricks.py:39:0: anomalous-backslash-in-string Anomalous backslash in string: '\w'. String constant might be missing an r prefix.
lib/ansible/module_utils/profitbricks.py:39:0: anomalous-backslash-in-string Anomalous backslash in string: '\w'. String constant might be missing an r prefix.
lib/ansible/module_utils/profitbricks.py:39:0: anomalous-backslash-in-string Anomalous backslash in string: '\w'. String constant might be missing an r prefix.

The test ansible-test sanity --test shebang [?] failed with the following error:

Command "test/sanity/code-smell/shebang.sh" returned exit status 1.
>>> Standard Output
./lib/ansible/module_utils/profitbricks.py:#!/usr/bin/python
One or more file(s) listed above have an unexpected shebang.
See test/sanity/code-smell/shebang.sh for the list of acceptable values.

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels Jan 13, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jan 13, 2018

The test ansible-test sanity --test import --python 2.6 [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:249:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:109:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:146:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:177:0: ImportError: cannot import name __version__

The test ansible-test sanity --test import --python 2.7 [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:249:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:109:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:146:0: ImportError: cannot import name __version__
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:177:0: ImportError: cannot import name __version__

The test ansible-test sanity --test import --python 3.5 [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:249:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:109:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:146:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:177:0: ImportError: cannot import name '__version__'

The test ansible-test sanity --test import --python 3.6 [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:249:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:109:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:146:0: ImportError: cannot import name '__version__'
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:177:0: ImportError: cannot import name '__version__'

The test ansible-test sanity --test import --python 3.7 [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:249:0: ImportError: cannot import name '__version__' from 'ansible' (/root/ansible/test/runner/import/lib/ansible/__init__.py)
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:109:0: ImportError: cannot import name '__version__' from 'ansible' (/root/ansible/test/runner/import/lib/ansible/__init__.py)
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:146:0: ImportError: cannot import name '__version__' from 'ansible' (/root/ansible/test/runner/import/lib/ansible/__init__.py)
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:177:0: ImportError: cannot import name '__version__' from 'ansible' (/root/ansible/test/runner/import/lib/ansible/__init__.py)

click here for bot help

@nurfet-becirevic

This comment has been minimized.

nurfet-becirevic commented Jan 13, 2018

I am not sure how to handle ImportError: cannot import name __version__. None of these seems to work:

from ansible import __version__
from ansible import __version__ as ansible_version
from ansible.release import __version__

@sivel

This comment has been minimized.

Member

sivel commented Jan 15, 2018

Instead of trying to import __version__, instead utilize module.ansible_version.

@ansibot ansibot removed the needs_triage label Jan 15, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jan 15, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jan 15, 2018

@nurfet-becirevic

This comment has been minimized.

nurfet-becirevic commented Jan 15, 2018

2018-01-15 21:11:59 ERROR: Command "python3.6 test/sanity/validate-modules/validate-modules --format json --arg-spec lib/ansible/modules/cloud/profitbricks/profitbricks.py lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py --exclude '^(lib/ansible/modules/utilities/logic/async_status.py|lib/ansible/modules/utilities/helper/_accelerate.py)' --base-branch origin/devel" returned exit status 1.
2018-01-15 21:11:59 >>> Standard Error
2018-01-15 21:11:59 Traceback (most recent call last):
2018-01-15 21:11:59   File "test/sanity/validate-modules/validate-modules", line 1361, in <module>
2018-01-15 21:11:59     main()
2018-01-15 21:11:59   File "test/sanity/validate-modules/validate-modules", line 1260, in main
2018-01-15 21:11:59     mv.validate()
2018-01-15 21:11:59   File "test/sanity/validate-modules/validate-modules", line 1152, in validate
2018-01-15 21:11:59     doc_info = self._validate_docs()
2018-01-15 21:11:59   File "test/sanity/validate-modules/validate-modules", line 876, in _validate_docs
2018-01-15 21:11:59     self._check_for_new_args(doc)
2018-01-15 21:11:59   File "test/sanity/validate-modules/validate-modules", line 1067, in _check_for_new_args
2018-01-15 21:11:59     names = [option] + details.get('aliases', [])
2018-01-15 21:11:59 TypeError: can only concatenate list (not "str") to list

Why the CI build is complaining on python 3.6 compatibility? Shouldn't it fail on the following as well?
https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/system/user.py#L2119-L2120

@sivel

This comment has been minimized.

Member

sivel commented Jan 15, 2018

The problem you are experiencing is that aliases needs to be a list, and you have defined it as a string:

https://github.com/ansible/ansible/pull/34834/files#diff-01759fa310331bba471354ce4323085cR133

Should be:

+  username:
+    description:
+      - The ProfitBricks username. Overrides the PROFITBRICKS_USERNAME environment variable.
+    required: false
+    aliases:
+      - subscription_user

An update likely needs to be made to validate-modules to handle this more gracefully

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jan 15, 2018

The test ansible-test sanity --test validate-modules [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:0:0: E309 version_added for new option (api_url) should be 2.5. Currently 2.4
lib/ansible/modules/cloud/profitbricks/profitbricks.py:0:0: E309 version_added for new option (availability_zone) should be 2.5. Currently 2.3
lib/ansible/modules/cloud/profitbricks/profitbricks.py:0:0: E309 version_added for new option (nat) should be 2.5. Currently 2.3
lib/ansible/modules/cloud/profitbricks/profitbricks.py:0:0: E309 version_added for new option (volume_availability_zone) should be 2.5. Currently 2.3
lib/ansible/modules/cloud/profitbricks/profitbricks.py:0:0: E321 Exception attempting to import module for argument_spec introspection
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:0:0: E309 version_added for new option (api_url) should be 2.5. Currently 2.4
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:0:0: E321 Exception attempting to import module for argument_spec introspection
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:0:0: E309 version_added for new option (api_url) should be 2.5. Currently 2.4
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:0:0: E309 version_added for new option (dhcp) should be 2.5. Currently 2.4
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:0:0: E309 version_added for new option (firewall_active) should be 2.5. Currently 2.4
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:0:0: E309 version_added for new option (ips) should be 2.5. Currently 2.4
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:0:0: E309 version_added for new option (nat) should be 2.5. Currently 2.3
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:0:0: E321 Exception attempting to import module for argument_spec introspection
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:0:0: E309 version_added for new option (api_url) should be 2.5. Currently 2.4
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:0:0: E309 version_added for new option (availability_zone) should be 2.5. Currently 2.3
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:0:0: E321 Exception attempting to import module for argument_spec introspection

click here for bot help

@ansibot ansibot added ci_verified and removed ci_verified labels Jan 15, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jan 15, 2018

The test ansible-test sanity --test validate-modules [?] failed with the following errors:

lib/ansible/modules/cloud/profitbricks/profitbricks.py:0:0: E321 Exception attempting to import module for argument_spec introspection
lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py:0:0: E321 Exception attempting to import module for argument_spec introspection
lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py:0:0: E321 Exception attempting to import module for argument_spec introspection
lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py:0:0: E321 Exception attempting to import module for argument_spec introspection

click here for bot help

@ansibot ansibot added the ci_verified label Jan 15, 2018

@nurfet-becirevic

This comment has been minimized.

nurfet-becirevic commented Jan 16, 2018

bot_status

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jan 16, 2018

Components

lib/ansible/module_utils/profitbricks.py
support: core
maintainers:

lib/ansible/modules/cloud/profitbricks/profitbricks.py
support: community
maintainers: baldwinSPC

lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py
support: community
maintainers: baldwinSPC

lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py
support: community
maintainers: baldwinSPC

lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py
support: community
maintainers: baldwinSPC

lib/ansible/modules/cloud/profitbricks/profitbricks_volume_attachments.py
support: community
maintainers: baldwinSPC

Metadata

waiting_on: ansible
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainer or core team member): []
shipit_actors_other: []

click here for bot help

@nurfet-becirevic nurfet-becirevic force-pushed the StackPointCloud:profitbricks-provider-update branch to b0eb041 Sep 17, 2018

@nurfet-becirevic

This comment has been minimized.

nurfet-becirevic commented Sep 17, 2018

I have applied the requested changes @gundalow . CI failure doesn't seem to be related to my latest commit.

@nurfet-becirevic

This comment has been minimized.

nurfet-becirevic commented Sep 18, 2018

bot_status

@ansibot

This comment has been minimized.

Contributor

ansibot commented Sep 18, 2018

Components

.github/BOTMETA.yml
support: core
maintainers:

changelogs/fragments/profitbricks-volume-attach-deprecation.yaml
support: core
maintainers:

lib/ansible/module_utils/cloud/init.py
support: community
maintainers:

lib/ansible/module_utils/cloud/profitbricks/init.py
support: community
maintainers:

lib/ansible/module_utils/cloud/profitbricks/profitbricks.py
support: community
maintainers:

lib/ansible/modules/cloud/profitbricks/_profitbricks_volume_attachments.py
support: community
maintainers: baldwinSPC

lib/ansible/modules/cloud/profitbricks/profitbricks.py
support: core
maintainers: baldwinSPC

lib/ansible/modules/cloud/profitbricks/profitbricks_datacenter.py
support: core
maintainers: baldwinSPC

lib/ansible/modules/cloud/profitbricks/profitbricks_nic.py
support: core
maintainers: baldwinSPC

lib/ansible/modules/cloud/profitbricks/profitbricks_volume.py
support: core
maintainers: baldwinSPC

test/sanity/validate-modules/ignore.txt
support: core
maintainers:

Metadata

waiting_on: nurfet-becirevic
changes_requested_by: null
needs_info: False
needs_revision: True
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: unstable
shippable_status: failure
maintainer_shipits (module maintainers): False
community_shipits (namespace maintainers): False
ansible_shipits (core team members): False
shipit_actors (maintainer or core team member): None
shipit_actors_other:
automerge: automerge shipit test failed

click here for bot help

@@ -0,0 +1,105 @@
# This code is part of Ansible, but is an independent component.

This comment has been minimized.

@mattclay

mattclay Sep 19, 2018

Member

Put this in lib/ansible/module_utils/profitbricks.py instead.

This comment has been minimized.

@nurfet-becirevic

nurfet-becirevic Sep 19, 2018

@mattclay, basically, you are saying I should revert the last commit? Before my last commit profitbricks.py was in lib/ansible/module_utils/.

This comment has been minimized.

@mattclay

mattclay Sep 19, 2018

Member

Yes, although I didn't see the comment from @gundalow earlier that requested the change. We'll need to do something to avoid the conflict. Reverting the change would be the easiest.

@mattclay

This comment has been minimized.

Member

mattclay commented Sep 19, 2018

The CI failure is due to the conflict between:

  • lib/ansible/module_utils/cloud.py
  • lib/ansible/module_utils/cloud/

Using lib/ansible/module_utils/profitbricks.py should resolve that.

@samdoran

This comment has been minimized.

Member

samdoran commented Sep 26, 2018

!waffling ci_verified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment