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

fix inaccurate task headers for host_pinned strategy #61845

Open
wants to merge 3 commits into
base: devel
from

Conversation

@kjoyce77
Copy link

commented Sep 5, 2019

SUMMARY

When using the host_pinned strategy the task header will be inaccurate. This is due to caching the task name. This is not an issue with the free strategy because task name caching is essentially disabled by setting the last task name to None every time a task starts.

I felt the simplest change was to add the host_pinned string to the conditional that already prevents this issue for the free strategy.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

default callback plugin

ADDITIONAL INFORMATION

Using the host_pinned strategy task results do not always match the task header under which they are printed.

**Jumbled, wrong task name headers: (note that CREATE SNAPSHOT DIRECTORY shows 
both skipped and ok on r2)**

TASK [fisv_network_snapshot : CREATE SNAPSHOT DIRECTORY] ***********************
skipping: [r2]

TASK [fisv_network_snapshot : CREATE SNAPSHOT DIRECTORY] ***********************
ok: [r2]

TASK [fisv_network_snapshot : VALIDATION SNAPSHOT] *****************************

TASK [fisv_network_snapshot : VALIDATION SNAPSHOT] *****************************
ok: [r1]

TASK [fisv_network_snapshot : VALIDATION SNAPSHOT] *****************************
ok: [r1] => (item=show version)
ok: [r2] => (item=show version)
ok: [r1] => (item=show cdp neighbors)
ok: [r2] => (item=show cdp neighbors)
ok: [r1] => (item=show ip route)
ok: [r2] => (item=show ip route)
ok: [r1] => (item=show interfaces)
ok: [r2] => (item=show interfaces)
ok: [r1] => (item=show standby brief)
ok: [r2] => (item=show standby brief)
ok: [r1] => (item=show ip eigrp neighbors)
ok: [r2] => (item=show ip eigrp neighbors)
ok: [r1] => (item=show running-config)

TASK [fisv_network_snapshot : MAKE SNAPSHOT DICTIONARY] ************************

TASK [fisv_network_snapshot : MAKE SNAPSHOT DICTIONARY] ************************
ok: [r2] => (item=show running-config)```


**After the change results belong to the appropriate task header**

TASK [fisv_network_snapshot : CREATE SNAPSHOT DIRECTORY] ***********************
ok: [r2]
ok: [r1]

TASK [fisv_network_snapshot : VALIDATION SNAPSHOT] *****************************
ok: [r1] => (item=show version)
ok: [r2] => (item=show version)
ok: [r2] => (item=show cdp neighbors)
ok: [r1] => (item=show cdp neighbors)
ok: [r2] => (item=show ip route)
ok: [r1] => (item=show ip route)
ok: [r2] => (item=show interfaces)
ok: [r1] => (item=show interfaces)
ok: [r1] => (item=show standby brief)
ok: [r2] => (item=show standby brief)
ok: [r1] => (item=show ip eigrp neighbors)
ok: [r2] => (item=show ip eigrp neighbors)
ok: [r1] => (item=show running-config)
ok: [r2] => (item=show running-config)

TASK [fisv_network_snapshot : MAKE SNAPSHOT DICTIONARY] ************************
ok: [r1] => (item=show version)
ok: [r1] => (item=show cdp neighbors)
ok: [r1] => (item=show ip route)
ok: [r1] => (item=show interfaces)
ok: [r1] => (item=show standby brief)
ok: [r1] => (item=show ip eigrp neighbors)
ok: [r1] => (item=show running-config)
ok: [r2] => (item=show version)
ok: [r2] => (item=show cdp neighbors)
ok: [r2] => (item=show ip route)
ok: [r2] => (item=show interfaces)
ok: [r2] => (item=show standby brief)
ok: [r2] => (item=show ip eigrp neighbors)
ok: [r2] => (item=show running-config)

@kjoyce77 kjoyce77 changed the title disable task name caching for host_pinned strategy fix inaccurate task headers for host_pinned strategy Sep 5, 2019

@kjoyce77 kjoyce77 marked this pull request as ready for review Sep 5, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

@kjoyce77 this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@kjoyce77 kjoyce77 force-pushed the kjoyce77:devel branch from 8c1babe to 8ba0141 Sep 5, 2019

if self._play.strategy == 'free':
# Explicitly set to None for strategy 'free' to account for any cached
# task title from a previous non-free play
if self._play.strategy in ['free', 'host_pinned']:

This comment has been minimized.

Copy link
@sivel

sivel Sep 12, 2019

Member

Instead of special casing these in the callback, we need a more general fix, that adds a new property to the strategy, that can then be queried in the callback.

@samdoran samdoran removed the needs_triage label Sep 12, 2019

@ansibot ansibot added needs_revision and removed core_review labels Sep 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.