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

smart fact gathering: gather facts when play level requested facts aren't available #44099

Open
wants to merge 6 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@pilou-
Contributor

pilou- commented Aug 14, 2018

SUMMARY

When smart gathering is enabled: facts aren't gathered if setup module had been called before.

Test play (use ANSIBLE_GATHERING=smart ansible-playbook play.yml):

- hosts: localhost
  gather_facts: false
  tasks:
    - setup:
        gather_subset:
          - '!min'
- hosts: localhost
  tasks:
    - debug:
        msg:
          "{{ ansible_distribution_release }}"

This pull request modify smart fact gathering behavior (when smart fact gathering is enabled):

  1. facts aren't gathered when facts already available are a superset of requested facts
  2. facts are gathered in all other cases

Notes:

  • require #44008 (44008 should be merged before this one)
  • integration tests included
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

gather_facts

ANSIBLE VERSION
2.7

@pilou- pilou- force-pushed the pilou-:smart_fact_gathering branch Aug 14, 2018

@pilou- pilou- changed the title from WIP: smart fact gathering to smart fact gathering: gather facts when play level requested facts aren't available Aug 14, 2018

@webknjaz webknjaz requested review from jimi-c, abadger and bcoca Aug 14, 2018

pilou- added some commits Aug 12, 2018

gather_subset is a barelist: use a list as default value
barelist has already been removed
gather_subset is a list of strings
When gather_subset is an integer, a message pointing out the problem
will be displayed.

The current error:

    The full traceback is:
    Traceback (most recent call last):
      File "tmp/ansible-tmp-1534089772.5756419-196565636905886/AnsiballZ_setup.py", line 113, in <module>
        _ansiballz_main()
      File "ansible-tmp-1534089772.5756419-196565636905886/AnsiballZ_setup.py", line 105, in _ansiballz_main
        invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
      File "ansible-tmp-1534089772.5756419-196565636905886/AnsiballZ_setup.py", line 48, in invoke_module
        imp.load_module('__main__', mod, module, MOD_DESC)
      File "__main__.py", line 184, in <module>
      File "__main__.py", line 176, in main
      File "ansible_setup_payload.zip/ansible/module_utils/facts/ansible_collector.py", line 124, in get_ansible_collector
      File "ansible_setup_payload.zip/ansible/module_utils/facts/collector.py", line 388, in collector_classes_from_gather_subset
      File "ansible_setup_payload.zip/ansible/module_utils/facts/collector.py", line 165, in get_collector_names
    AttributeError: 'int' object has no attribute 'startswith'

is replaced by this one:

    ERROR! the field 'gather_subset' should be a list of (<class 'str'>,), but the item '42' is a <class 'int'>

    The error appears to have been in 'play.yml': line 1, column 3, but may
    be elsewhere in the file depending on the exact syntax problem.

    The offending line appears to be:

    - hosts: all
      ^ here
check gathering if smart is on & setup was used
When smart gathering is enabled, check that
1. facts aren't gathered when already gathered facts are a superset of
requested facts
2. facts are gathered in all other cases

Minimal test play:

    $ ANSIBLE_GATHERING=smart ansible-playbook play.yml
    $ cat play.yml
    ---
    - hosts: localhost
      gather_facts: false
      tasks:
        - setup:
            gather_subset:
              - '!min'
    - hosts: localhost
      tasks:
        - debug:
            msg:
              "{{ ansible_distribution_release }}"

@pilou- pilou- force-pushed the pilou-:smart_fact_gathering branch to 6b997ff Aug 21, 2018

When smart gathering is enabled, gather facts when required
When smart gathering is enabled:
1. facts aren't gathered when already gathered facts are a superset of
requested facts
2. facts are gathered in all other cases

@ansibot ansibot removed the small_patch label Aug 22, 2018

@pilou-

This comment has been minimized.

Contributor

pilou- commented Aug 22, 2018

Rebased, I need to analyse this new integration test failure:

2018-08-22 09:01:02 TASK [check gather_subset fact] ************************************************
2018-08-22 09:01:02 task path: /root/ansible/test/integration/targets/gathering_facts/test_smart_gathering_gather_subset.yml:47
2018-08-22 09:01:02 fatal: [facthost1]: FAILED! => {"changed": false, "msg": "expected: ['!min', 'local'], encountered: [u'!min,local']"}
@bcoca

This comment has been minimized.

Member

bcoca commented Aug 22, 2018

This is not really 'fact gathering' itself, but specific subset you want to consume but explicitly avoid collecting beforehand.

@pilou-

This comment has been minimized.

Contributor

pilou- commented Aug 22, 2018

Using the playbook given on the description, the current behavior seems unexpected (because facts aren't gathered at the beginning of the 2nd play).

@bcoca

This comment has been minimized.

Member

bcoca commented Aug 22, 2018

Yes, and we can do similar examples with all kinds of facts, in hardware, networking, etc .. every category has the same issue. If you restrict facts gathered to begin with, they wont be available even when gathering is 'smart' as it assumes that the previous options you selected are the ones you want for the whole run.

@mattclay

This comment has been minimized.

Member

mattclay commented Aug 28, 2018

@bcoca

not something we want as this would only cover a small subset of facts, in the end any customization of fact gathering with 'gathering=smart' will trigger this problem, at most we should warn in the documentation about this.

@pilou-

This comment has been minimized.

Contributor

pilou- commented Aug 28, 2018

I don't understand, the whole point of this PR is to compare what has been gathered by previous setup tasks with what should be gathered in order to comply with the play level gather_subset. Is that something which can not be done ?

@ansibot ansibot added the stale_ci label Sep 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment