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.

Sign up for GitHub

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

Jump to bottom

Make it so callback plugins can act on implicit/explicit meta tasks #71009

Merged
merged 11 commits into from
Aug 18, 2020
Merged

Make it so callback plugins can act on implicit/explicit meta tasks #71009

merged 11 commits into from
Aug 18, 2020

Conversation

relrod
Copy link
Member

@relrod relrod commented Jul 31, 2020 •
edited

SUMMARY

Allow callbacks to receive meta tasks

Change:

  • Send meta executions to v2_playbook_on_task_start
  • Allow callback plugins to opt-in to implicit tasks

Test Plan:

  • New integration tests

Tickets:

Signed-off-by: Rick Elrod rick@elrod.me

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • callback
  • strategy
  • meta

@relrod relrod changed the title Plugins/callback strategy/metahook Add v2_playbook_on_meta_start callback hook Jul 31, 2020
@relrod relrod mentioned this pull request Jul 31, 2020
Closed
@ansibot ansibot added affects_2.11 core_review In order to be merged, this PR must follow the core review workflow. feature This issue/PR relates to a feature request. has_issue needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jul 31, 2020
@relrod relrod force-pushed the plugins/callback-strategy/metahook branch from 11e5faf to 342874d Compare July 31, 2020 04:32
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 31, 2020
@bcoca
Copy link
Member

bcoca commented Jul 31, 2020

Instead of creating a new event, we should just issue existing 'task events', but only for explicit meta task from playbooks.

We could add an 'internal' property to the meta tasks we auto generate and skip issuing callbacks for those.

@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Aug 4, 2020
@relrod relrod force-pushed the plugins/callback-strategy/metahook branch from 342874d to b5cf6a3 Compare August 4, 2020 21:29
@relrod relrod marked this pull request as draft August 4, 2020 21:37
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Aug 4, 2020
@samdoran samdoran added the ci_verified Changes made in this PR are causing tests to fail. label Aug 5, 2020
@relrod relrod force-pushed the plugins/callback-strategy/metahook branch from ca3791d to 26e0d1d Compare August 9, 2020 06:07
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. docs This issue/PR relates to or includes documentation. docsite This issue/PR relates to the documentation website. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. 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. core_review In order to be merged, this PR must follow the core review workflow. labels Aug 9, 2020
@relrod relrod changed the title WIP: Make it so callback plugins can act on implicit/explicit meta tasks Make it so callback plugins can act on implicit/explicit meta tasks Aug 12, 2020
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. core_review In order to be merged, this PR must follow the core review workflow. labels Aug 12, 2020
s-hertel
s-hertel reviewed Aug 17, 2020
View reviewed changes
Copy link
Contributor

@s-hertel s-hertel left a comment

Choose a reason for hiding this comment

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

Looks good to me

lib/ansible/plugins/strategy/__init__.py Show resolved Hide resolved
@relrod
test fix
64b2dd3 
Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod
Add v2_playbook_on_meta_start callback hook
467044a 
Change:
- Introduces a new `v2_playbook_on_meta_start` hook in callback
- Calls it in strategy _execute_meta()

Test Plan:
- New integration tests

Tickets:
- Indrectly fixes ansible#71007 by allowing custom callbacks with this data

Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod
updates from meeting discussion
1e651fe 
Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod
tests and docs
a1af7af 
Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod
undo these
beb2db3 
Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod
Update 71007_callback_on_meta.yml
b15485b
@relrod
porting guide
e873cb4 
Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod
use a Task attr instead
a719f3c 
Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod
fix up porting guide paths
9bfac99 
Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod
dedupe a bit
9ea235d 
Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod
add a default wants_implicit_tasks to CallbackBase
f657b22 
Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod relrod force-pushed the plugins/callback-strategy/metahook branch from c42a3e4 to f657b22 Compare August 18, 2020 18:54
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. labels Aug 18, 2020
@relrod relrod merged commit ea58d7c into ansible:devel Aug 18, 2020
@mkrizek mkrizek mentioned this pull request Aug 19, 2020
Merged
relrod added a commit to relrod/ansible that referenced this pull request Aug 19, 2020
@relrod
Add skip_reason to meta task skips
11fc7a0 
Change:
- Make them more consistent with other tasks.

Test Plan:
- CI, new test

Tickets:
- Refs ansible#71009

Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod relrod mentioned this pull request Aug 19, 2020
Merged
bcoca pushed a commit to bcoca/ansible that referenced this pull request Aug 19, 2020
@relrod @bcoca
Make it so callback plugins can act on implicit/explicit meta tasks (a…
0cbe236 
…nsible#71009)

Change:
- Now sends meta tasks to the task start callback
- Lets callback plugins opt-in to receiving implicit tasks

Test Plan:
- New integration tests

Tickets:
- Indirectly fixes ansible#71007 by allowing custom callbacks with this data

Signed-off-by: Rick Elrod <rick@elrod.me>
@s-hertel s-hertel mentioned this pull request Aug 19, 2020
Merged
relrod added a commit that referenced this pull request Aug 20, 2020
@relrod
Add skip_reason to meta task skips (#71355)
e5bb7b1 
Change:
- Make them more consistent with other tasks.

Test Plan:
- CI, new test

Tickets:
- Refs #71009

Signed-off-by: Rick Elrod <rick@elrod.me>
@ansible ansible locked and limited conversation to collaborators Sep 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sivel sivel sivel left review comments

@bcoca bcoca bcoca requested changes

@s-hertel s-hertel s-hertel requested changes

Assignees
No one assigned
Labels
affects_2.11 docs This issue/PR relates to or includes documentation. docsite This issue/PR relates to the documentation website. feature This issue/PR relates to a feature request. has_issue needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

meta: refresh_inventory doesn't show in output, not even skipped
6 participants
@relrod @bcoca @ansibot @sivel @s-hertel @samdoran