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

gather_facts action plugin: Fix loading network facts modules for smart gathering #59856

Merged
merged 4 commits into from Sep 17, 2019

Conversation

trishnaguha
Copy link
Member

@trishnaguha trishnaguha commented Jul 31, 2019

SUMMARY

Fix loading network facts modules for smart gathering.
Currently the connection plugin is loaded but not the network_os which is why the <network_os>_facts module is not invoked.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

plugins/action/gather_facts.py

Copy link
Member

@bcoca bcoca left a comment

Choose a reason for hiding this comment

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

I would add a changelog since this would need backporting

also some test cases so this does not break in the future

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. 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. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jul 31, 2019
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Aug 1, 2019
@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 Aug 9, 2019
@ikhan2010
Copy link

@trishnaguha is this ready to merge?

@ikhan2010 ikhan2010 added the P2 Priority 2 - Issue Blocks Release label Aug 27, 2019
@abadger
Copy link
Contributor

abadger commented Aug 29, 2019

As beta1 is being released now, all future merges need to have changelog entries to tell users of pre-releases what has changed since beta1.

This appears to be an approved bugfix which is eligible to be merged before rc1 but it does need to have the changelog added.

@ansibot ansibot added has_issue new_plugin This PR includes a new plugin. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Sep 11, 2019
@trishnaguha
Copy link
Member Author

trishnaguha commented Sep 12, 2019

@bcoca Will you please review the PR? We need this in 2.9.
Thanks!

@abadger
Copy link
Contributor

abadger commented Sep 16, 2019

@bcoca bcoca says he's given a review of this via slack. So this is now waiting on changes from @trishnaguha

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
@trishnaguha
Copy link
Member Author

trishnaguha commented Sep 17, 2019

The PR now addresses the review comment.

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
@trishnaguha trishnaguha merged commit 5cd3be9 into ansible:devel Sep 17, 2019
@trishnaguha trishnaguha deleted the gather_facts_action_plugin branch September 17, 2019 13:05
trishnaguha added a commit to trishnaguha/ansible that referenced this pull request Sep 17, 2019
…rt gathering (ansible#59856)

* fix smart gathering for network_os in gather_facts action plugin

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Fix detection of network_os for smart gathering

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Add unittest

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* make pep8 happy

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
(cherry picked from commit 5cd3be9)
@trishnaguha
Copy link
Member Author

@abadger Backporting PR #62413

trishnaguha added a commit to trishnaguha/ansible that referenced this pull request Sep 17, 2019
…rt gathering (ansible#59856)

* fix smart gathering for network_os in gather_facts action plugin

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Fix detection of network_os for smart gathering

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Add unittest

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* make pep8 happy

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
(cherry picked from commit 5cd3be9)
@abadger abadger moved this from To do to Done in 2.9 post beta firedrills Sep 17, 2019
abadger pushed a commit that referenced this pull request Sep 17, 2019
…rt gathering (#59856)

* fix smart gathering for network_os in gather_facts action plugin

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Fix detection of network_os for smart gathering

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Add unittest

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* make pep8 happy

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
(cherry picked from commit 5cd3be9)
anas-shami pushed a commit to anas-shami/ansible that referenced this pull request Sep 23, 2019
…rt gathering (ansible#59856)

* fix smart gathering for network_os in gather_facts action plugin

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Fix detection of network_os for smart gathering

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Add unittest

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* make pep8 happy

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
@ansible ansible locked and limited conversation to collaborators Oct 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. has_issue needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_plugin This PR includes a new plugin. P2 Priority 2 - Issue Blocks Release support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants