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

Strategy Plugin Support Custom Options #69671

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from

Conversation

kevin-j-smith
Copy link

@kevin-j-smith kevin-j-smith commented May 22, 2020

Signed-off-by: Kevin J. Smith kevin.j.smith2@cerner.com

SUMMARY

Added custom options to strategy by inheriting ansible.plugins.AnsiblePlugin and adding host_pinned as an option to free to deprecate host_pinned.

Fixes #69668

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

Strategy Plugin Support Custom Options through Inheriting AnsiblePlugin

ADDITIONAL INFORMATION

See changes made to ansible.plugins.strategy.free that now obsoletes ansible.plugins.strategy.host_pinned

See how host_pinned can be used through env:
test/integration/targets/blocks/runme.sh:L27

…egy by inheriting AnsiblePlugin and adding host_pinned as a option to free to deprecate host_pinned

Signed-off-by: Kevin J. Smith <kevin.j.smith2@cerner.com>
@ansibot ansibot added affects_2.10 core_review feature needs_triage new_contributor support:community support:core labels May 22, 2020
…itespace

Signed-off-by: Kevin J. Smith <kevin.j.smith2@cerner.com>
@ansibot
Copy link
Contributor

ansibot commented May 22, 2020

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/plugins/strategy/free.py:37:112: W291: trailing whitespace

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels May 22, 2020
@kevin-j-smith
Copy link
Author

kevin-j-smith commented May 22, 2020

I see that this failed, but it is not clear how to correct it or even test it.

@kevin-j-smith
Copy link
Author

kevin-j-smith commented May 22, 2020

These 2 failing tests passed before I removed the extra space on the initial run's only failure. Is this a test timing issue?

Signed-off-by: Kevin J. Smith <kevin.j.smith2@cerner.com>
@ansibot ansibot added core_review needs_ci stale_ci needs_revision and removed needs_revision needs_ci stale_ci core_review labels May 22, 2020
@kevin-j-smith
Copy link
Author

kevin-j-smith commented May 22, 2020

Yeah, just as I suspected. I made a trivial change, check-ed it in and the 2 test failure now pass and 1 new test failure. :(

@bcoca bcoca removed the needs_triage label May 26, 2020
@bcoca
Copy link
Member

bcoca commented May 26, 2020

i was also thinking of expanding strategy in the playbook itself:

strategy:
  name: free
  host_pinned: true|false

with 'if string' auto assigning to name as backwards compatible change.

@kevin-j-smith
Copy link
Author

kevin-j-smith commented May 26, 2020

@bcoca that was another thought I had which also works. I am thinking both solutions would work well together, if that implementation doesn't make the code too complex. Your piece would be handy when playbooks have multiple plays with different strategies. In production, we have built a utility node whose /etc/ansible/ansible.cfg is what is used by all executions. But in other scenarios overriding that within the playbook would be handy.

Do you know how I can re-run the failed tests so that I can get a passible shippable run?

@ansibot ansibot added core_review and removed needs_revision labels May 26, 2020
@mkrizek
Copy link
Contributor

mkrizek commented May 26, 2020

@kevin-j-smith The tests have been re-run manually and all passed now but FYI there are bot commands that you can use to rebuild either all or failed tests, see https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md#commands.

@ansibot ansibot added the stale_ci label Jun 3, 2020
@ansibot ansibot added needs_revision pre_azp and removed core_review stale_ci labels Dec 5, 2020
@bcoca bcoca self-assigned this Jan 12, 2022
@ansibot ansibot added the needs_rebase label Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.10 feature needs_rebase needs_revision new_contributor pre_azp support:community support:core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants