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

Enhancements to the cobbler inventory script #25926

Closed
wants to merge 791 commits into from

Conversation

opoplawski
Copy link
Contributor

SUMMARY

This makes a couple minor and one major change to the cobbler inventory script:

  • Drop note about script not logging in as it appears to be able to do so now
  • Make orderby_keyname a config parameter
  • Handle multiple cobbler servers
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

cobbler inventory

ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

ansibot commented Jun 20, 2017

The test ansible-test sanity --test pep8 failed with the following errors:

contrib/inventory/cobbler.py:153:43: E231 missing whitespace after ','
contrib/inventory/cobbler.py:216:20: E111 indentation is not a multiple of four
contrib/inventory/cobbler.py:218:20: E111 indentation is not a multiple of four

click here for bot help

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 ci_verified Changes made in this PR are causing tests to fail. feature_pull_request needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 20, 2017
@ryansb ryansb removed the needs_triage Needs a first human triage before being processed. label Jun 21, 2017
@ryansb
Copy link
Contributor

ryansb commented Jun 21, 2017

cc/ @alikins

@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jun 29, 2017
@ansibot ansibot added the inventory Inventory category label Jul 18, 2017
@calfonso calfonso requested a review from jimi-c August 7, 2017 18:31
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 24, 2017
@ansibot ansibot added c:inventory/contrib_script support:community This issue/PR relates to code supported by the Ansible community. and removed support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 1, 2017
@opoplawski
Copy link
Contributor Author

Any comments here? I've got some further enhancements - handling sub-profiles as sub-groups, which required shifting to the newer json format of output using 'hosts' and 'children'.

@ansibot ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Sep 14, 2017
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 22, 2017
@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 2, 2018
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 29, 2018
@jimi-c
Copy link
Member

jimi-c commented Mar 27, 2019

@opoplawski sorry for not getting back sooner on this. It's no longer mergable, curious if you'd want to migrate this to an inventory plugin instead?

mattclay and others added 12 commits May 7, 2020 17:57
The module requires updates to work with the current Azure API.
(cherry picked from commit 96f504c)

Co-authored-by: Brian Coca <brian.coca+git@gmail.com>
Since Ansible 2.9.8, if the fileglob plugin is passed a path containing
a subdirectory of a non-existent directory, it will fail. For example:

lookup('fileglob', '/'): ok
lookup('fileglob', '/foo'): (non-existent): ok
lookup('fileglob', '/foo/bar'): (non-existent): FAIL

The exact error depends on Python 2 or 3, but here is the error on
Python 2:

    AttributeError: 'NoneType' object has no attribute 'endswith'

And on Python 3:

    TypeError: expected str, bytes or os.PathLike object, not NoneType

This change fixes the issue by skipping paths that are falsey before
passing them to os.path.join().

Fixes: ansible#69450
…9645)

* [stable-2.9] Pin Docker version at 19.03.1
(cherry picked from commit fe941a4)

Co-authored-by: Sam Doran <sdoran@redhat.com>

* [stable-2.9] Pin docker-ce-cli version in tests (ansible#69620)

Installing docker-ce has a dependency of docker-ce-cli. If the version of docker-ci-cli is not specified, it installs the latest version.

(cherry picked from commit 889da81)
anp, epg are required arguments for module.  fatal error occurs when not specified: "FAILED! => {"changed": false, "msg": "missing required arguments: anp, epg"}"
opoplawski and others added 4 commits May 30, 2020 12:03
- Add exclude_profiles option
- Python 3 support
- Better cache handling
- --debug option
- Remove --ignore-settings option
- Remove environment variable handling
- Create parent profile groups
@opoplawski
Copy link
Contributor Author

This has been rebased against stable-2.9, mainly just so others can see my latest version of the script.

@ansibot
Copy link
Contributor

ansibot commented May 30, 2020

@opoplawski This PR was evaluated as a potentially problematic PR for the following reasons:

  • More than 50 changed files.
  • More than 50 commits.

Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: #ansible-devel on irc.freenode.net

click here for bot help

@ansibot ansibot added has_issue and removed collection:community.general needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md labels May 30, 2020
@jimi-c
Copy link
Member

jimi-c commented Jun 1, 2020

@opoplawski seems like it was a bad rebase? There are quite a few conflicting files in odd places.

@opoplawski
Copy link
Contributor Author

It was rebased against release-2.9 instead of master. It appears impossible to change the target branch of the PR. I suppose I could close and open one against that if possible. Or figure out where the script lives now if anywhere..

@jimi-c
Copy link
Member

jimi-c commented Jun 1, 2020

Yeah I'd close and re-create, this would not be back-ported to 2.9. Scripts are overall getting deprecated in favor of inventory plugins which work much better. You may want to look at converting it and publishing it via a collection instead.

@mattclay
Copy link
Member

Closing, since inventory scripts are no longer in the devel branch and features/enhancements to the stable branches are not being accepted.

The cobbler inventory script has been moved to the community.general collection [1], although as @jimi-c points out, it would be better to provide an inventory plugin instead of continuing to work on the inventory script.

[1] https://github.com/ansible-collections/community.general/blob/master/scripts/inventory/cobbler.py

@mattclay mattclay closed this Jun 18, 2020
@opoplawski
Copy link
Contributor Author

@ansible ansible locked and limited conversation to collaborators Jul 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 c:inventory/contrib_script collection Related to Ansible Collections work feature This issue/PR relates to a feature request. has_issue inventory Inventory category needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet