Remove choices from gather_network_resources facts and fix negated all, min (#61362)

* remove choices from gather_network_resources facts and allow negating subset without needing to add a new subset specific for negation

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

* negated all, min should not return any fact

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
(cherry picked from commit c1e02d5c7a)

changelogs/fragments/gather_facts_networkfix29.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+- Fix negated all,min for network_facts and remove choices (https://github.com/ansible/ansible/pull/61362).

lib/ansible/module_utils/network/common/facts/facts.py

@@ -31,18 +31,21 @@ class FactsBase(object):
         if not self._gather_network_resources:
             self._gather_network_resources = ['!all']
 
-    def gen_runable(self, subsets, valid_subsets):
+    def gen_runable(self, subsets, valid_subsets, resource_facts=False):
         """ Generate the runable subset
 
         :param module: The module instance
         :param subsets: The provided subsets
         :param valid_subsets: The valid subsets
+        :param resource_facts: A boolean flag
         :rtype: list
         :returns: The runable subsets
         """
         runable_subsets = set()
         exclude_subsets = set()
-        minimal_gather_subset = frozenset(['default'])
+        minimal_gather_subset = set()
+        if not resource_facts:
+            minimal_gather_subset = frozenset(['default'])
 
         for subset in subsets:
             if subset == 'all':
@@ -78,43 +81,39 @@ class FactsBase(object):
         runable_subsets.difference_update(exclude_subsets)
         return runable_subsets
 
-    def get_network_resources_facts(self, net_res_choices, facts_resource_obj_map, resource_facts_type=None, data=None):
+    def get_network_resources_facts(self, facts_resource_obj_map, resource_facts_type=None, data=None):
         """
-        :param net_res_choices:
         :param fact_resource_subsets:
         :param data: previously collected configuration
         :return:
         """
-        if net_res_choices:
-            if 'all' in net_res_choices:
-                net_res_choices.remove('all')
-
-        if net_res_choices:
-            if not resource_facts_type:
-                resource_facts_type = self._gather_network_resources
-
-            restorun_subsets = self.gen_runable(resource_facts_type, frozenset(net_res_choices))
-            if restorun_subsets:
-                self.ansible_facts['ansible_net_gather_network_resources'] = list(restorun_subsets)
-                instances = list()
-                for key in restorun_subsets:
-                    fact_cls_obj = facts_resource_obj_map.get(key)
-                    if fact_cls_obj:
-                        instances.append(fact_cls_obj(self._module))
-                    else:
-                        self._warnings.extend(["network resource fact gathering for '%s' is not supported" % key])
-
-                for inst in instances:
-                    inst.populate_facts(self._connection, self.ansible_facts, data)
+        if not resource_facts_type:
+            resource_facts_type = self._gather_network_resources
+
+        restorun_subsets = self.gen_runable(resource_facts_type, frozenset(facts_resource_obj_map.keys()), resource_facts=True)
+        if restorun_subsets:
+            self.ansible_facts['ansible_net_gather_network_resources'] = list(restorun_subsets)
+            instances = list()
+            for key in restorun_subsets:
+                fact_cls_obj = facts_resource_obj_map.get(key)
+                if fact_cls_obj:
+                    instances.append(fact_cls_obj(self._module))
+                else:
+                    self._warnings.extend(["network resource fact gathering for '%s' is not supported" % key])
+
+            for inst in instances:
+                inst.populate_facts(self._connection, self.ansible_facts, data)
 
     def get_network_legacy_facts(self, fact_legacy_obj_map, legacy_facts_type=None):
         if not legacy_facts_type:
             legacy_facts_type = self._gather_subset
 
         runable_subsets = self.gen_runable(legacy_facts_type, frozenset(fact_legacy_obj_map.keys()))
-        runable_subsets.add('default')
         if runable_subsets:
             facts = dict()
+            # default subset should always returned be with legacy facts subsets
+            if 'default' not in runable_subsets:
+                runable_subsets.add('default')
             self.ansible_facts['ansible_net_gather_subset'] = list(runable_subsets)
 
             instances = list()

lib/ansible/module_utils/network/eos/argspec/facts/facts.py

@@ -8,29 +8,6 @@ The arg spec for the eos facts module.
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-CHOICES = [
-    'all',
-    '!all',
-    'interfaces',
-    '!interfaces',
-    'l2_interfaces',
-    '!l2_interfaces',
-    'l3_interfaces',
-    '!l3_interfaces',
-    'lacp',
-    '!lacp',
-    'lacp_interfaces',
-    '!lacp_interfaces',
-    'lag_interfaces',
-    '!lag_interfaces',
-    'lldp_global',
-    '!lldp_global',
-    'lldp_interfaces',
-    '!lldp_interfaces',
-    'vlans',
-    '!vlans',
-]
-
 
 class FactsArgs(object):
     """ The arg spec for the eos facts module
@@ -41,5 +18,5 @@ class FactsArgs(object):
 
     argument_spec = {
         'gather_subset': dict(default=['!config'], type='list'),
-        'gather_network_resources': dict(choices=CHOICES, type='list'),
+        'gather_network_resources': dict(type='list'),
     }

lib/ansible/module_utils/network/eos/facts/facts.py

@@ -58,9 +58,8 @@ class Facts(FactsBase):
         :rtype: dict
         :return: the facts gathered
         """
-        netres_choices = FactsArgs.argument_spec['gather_network_resources'].get('choices', [])
         if self.VALID_RESOURCE_SUBSETS:
-            self.get_network_resources_facts(netres_choices, FACT_RESOURCE_SUBSETS, resource_facts_type, data)
+            self.get_network_resources_facts(FACT_RESOURCE_SUBSETS, resource_facts_type, data)
 
         if self.VALID_LEGACY_GATHER_SUBSETS:
             self.get_network_legacy_facts(FACT_LEGACY_SUBSETS, legacy_facts_type)

lib/ansible/module_utils/network/exos/argspec/facts/facts.py

@@ -17,15 +17,7 @@ class FactsArgs(object):  # pylint: disable=R0903
     def __init__(self, **kwargs):
         pass
 
-    choices = [
-        'all',
-        '!all',
-        'lldp_global',
-        '!lldp_global'
-    ]
-
     argument_spec = {
         'gather_subset': dict(default=['!config'], type='list'),
-        'gather_network_resources': dict(choices=choices,
-                                         type='list'),
+        'gather_network_resources': dict(type='list'),
     }

lib/ansible/module_utils/network/exos/facts/facts.py

@@ -46,9 +46,8 @@ class Facts(FactsBase):
         :rtype: dict
         :return: the facts gathered
         """
-        netres_choices = FactsArgs.argument_spec['gather_network_resources'].get('choices', [])
         if self.VALID_RESOURCE_SUBSETS:
-            self.get_network_resources_facts(netres_choices, FACT_RESOURCE_SUBSETS, resource_facts_type, data)
+            self.get_network_resources_facts(FACT_RESOURCE_SUBSETS, resource_facts_type, data)
 
         if self.VALID_LEGACY_GATHER_SUBSETS:
             self.get_network_legacy_facts(FACT_LEGACY_SUBSETS, legacy_facts_type)

lib/ansible/module_utils/network/ios/argspec/facts/facts.py

@@ -18,30 +18,7 @@ class FactsArgs(object):
     def __init__(self, **kwargs):
         pass
 
-    choices = [
-        'all',
-        '!all',
-        'interfaces',
-        '!interfaces',
-        'l2_interfaces',
-        '!l2_interfaces',
-        'vlans',
-        '!vlans',
-        'lag_interfaces',
-        '!lag_interfaces',
-        'lacp',
-        '!lacp',
-        'lacp_interfaces',
-        '!lacp_interfaces',
-        'lldp_global',
-        '!lldp_global',
-        'lldp_interfaces',
-        '!lldp_interfaces',
-        'l3_interfaces',
-        '!l3_interfaces',
-    ]
-
     argument_spec = {
         'gather_subset': dict(default=['!config'], type='list'),
-        'gather_network_resources': dict(choices=choices, type='list'),
+        'gather_network_resources': dict(type='list'),
     }

lib/ansible/module_utils/network/ios/facts/facts.py

@@ -65,9 +65,8 @@ class Facts(FactsBase):
         :rtype: dict
         :return: the facts gathered
         """
-        netres_choices = FactsArgs.argument_spec['gather_network_resources'].get('choices', [])
         if self.VALID_RESOURCE_SUBSETS:
-            self.get_network_resources_facts(netres_choices, FACT_RESOURCE_SUBSETS, resource_facts_type, data)
+            self.get_network_resources_facts(FACT_RESOURCE_SUBSETS, resource_facts_type, data)
 
         if self.VALID_LEGACY_GATHER_SUBSETS:
             self.get_network_legacy_facts(FACT_LEGACY_SUBSETS, legacy_facts_type)

lib/ansible/module_utils/network/iosxr/argspec/facts/facts.py

@@ -18,27 +18,7 @@ class FactsArgs(object):  # pylint: disable=R0903
     def __init__(self, **kwargs):
         pass
 
-    choices = [
-        'all',
-        'lacp',
-        '!lacp',
-        'lacp_interfaces',
-        '!lacp_interfaces',
-        'lldp_global',
-        '!lldp_global',
-        'lldp_interfaces',
-        '!lldp_interfaces',
-        'interfaces',
-        '!interfaces',
-        'l2_interfaces',
-        '!l2_interfaces',
-        'lag_interfaces',
-        '!lag_interfaces',
-        'l3_interfaces',
-        '!l3_interfaces',
-    ]
-
     argument_spec = {
         'gather_subset': dict(default=['!config'], type='list'),
-        'gather_network_resources': dict(choices=choices, type='list'),
+        'gather_network_resources': dict(type='list'),
     }

lib/ansible/module_utils/network/iosxr/facts/facts.py

@@ -63,9 +63,8 @@ class Facts(FactsBase):
         :rtype: dict
         :return: the facts gathered
         """
-        netres_choices = FactsArgs.argument_spec['gather_network_resources'].get('choices', [])
         if self.VALID_RESOURCE_SUBSETS:
-            self.get_network_resources_facts(netres_choices, FACT_RESOURCE_SUBSETS, resource_facts_type, data)
+            self.get_network_resources_facts(FACT_RESOURCE_SUBSETS, resource_facts_type, data)
 
         if self.VALID_LEGACY_GATHER_SUBSETS:
             self.get_network_legacy_facts(FACT_LEGACY_SUBSETS, legacy_facts_type)

lib/ansible/module_utils/network/junos/argspec/facts/facts.py

@@ -6,28 +6,6 @@
 """
 The arg spec for the junos facts module.
 """
-CHOICES = [
-    'all',
-    '!all',
-    'interfaces',
-    '!interfaces',
-    'lacp',
-    '!lacp',
-    'lacp_interfaces',
-    '!lacp_interfaces',
-    'lag_interfaces',
-    '!lag_interfaces',
-    'l2_interfaces',
-    '!l2_interfaces',
-    'l3_interfaces',
-    '!l3_interfaces',
-    'lldp_global',
-    '!lldp_global',
-    'lldp_interfaces',
-    '!lldp_interfaces',
-    'vlans',
-    '!vlans',
-]
 
 
 class FactsArgs(object):
@@ -40,5 +18,5 @@ class FactsArgs(object):
     argument_spec = {
         'gather_subset': dict(default=['!config'], type='list'),
         'config_format': dict(default='text', choices=['xml', 'text', 'set', 'json']),
-        'gather_network_resources': dict(choices=CHOICES, type='list'),
+        'gather_network_resources': dict(type='list'),
     }

lib/ansible/module_utils/network/junos/facts/facts.py

@@ -59,9 +59,8 @@ class Facts(FactsBase):
         :rtype: dict
         :return: the facts gathered
         """
-        netres_choices = FactsArgs.argument_spec['gather_network_resources'].get('choices', [])
         if self.VALID_RESOURCE_SUBSETS:
-            self.get_network_resources_facts(netres_choices, FACT_RESOURCE_SUBSETS, resource_facts_type, data)
+            self.get_network_resources_facts(FACT_RESOURCE_SUBSETS, resource_facts_type, data)
 
         if not legacy_facts_type:
             legacy_facts_type = self._gather_subset

lib/ansible/module_utils/network/nxos/argspec/facts/facts.py

@@ -7,20 +7,6 @@
 The arg spec for the nxos facts module.
 """
 
-CHOICES = [
-    'all',
-    'bfd_interfaces',
-    'lag_interfaces',
-    'lldp_global',
-    'telemetry',
-    'vlans',
-    'lacp',
-    'lacp_interfaces',
-    'interfaces',
-    'l3_interfaces',
-    'l2_interfaces',
-]
-
 
 class FactsArgs(object):
     """ The arg spec for the nxos facts module
@@ -31,5 +17,5 @@ class FactsArgs(object):
 
     argument_spec = {
         'gather_subset': dict(default=['!config'], type='list'),
-        'gather_network_resources': dict(choices=CHOICES, type='list'),
+        'gather_network_resources': dict(type='list'),
     }

lib/ansible/module_utils/network/nxos/facts/facts.py

@@ -64,9 +64,8 @@ class Facts(FactsBase):
         :rtype: dict
         :return: the facts gathered
         """
-        netres_choices = FactsArgs.argument_spec['gather_network_resources'].get('choices', [])
         if self.VALID_RESOURCE_SUBSETS:
-            self.get_network_resources_facts(netres_choices, FACT_RESOURCE_SUBSETS, resource_facts_type, data)
+            self.get_network_resources_facts(FACT_RESOURCE_SUBSETS, resource_facts_type, data)
 
         if self.VALID_LEGACY_GATHER_SUBSETS:
             self.get_network_legacy_facts(FACT_LEGACY_SUBSETS, legacy_facts_type)

lib/ansible/module_utils/network/vyos/argspec/facts/facts.py

@@ -15,21 +15,7 @@ class FactsArgs(object):  # pylint: disable=R0903
     def __init__(self, **kwargs):
         pass
 
-    choices = [
-        'all',
-        'interfaces',
-        '!interfaces',
-        'l3_interfaces',
-        '!l3_interfaces',
-        'lag_interfaces',
-        '!lag_interfaces',
-        'lldp_global',
-        '!lldp_global',
-        'lldp_interfaces',
-        '!lldp_interfaces'
-    ]
-
     argument_spec = {
         'gather_subset': dict(default=['!config'], type='list'),
-        'gather_network_resources': dict(choices=choices, type='list'),
+        'gather_network_resources': dict(type='list'),
     }

lib/ansible/module_utils/network/vyos/facts/facts.py

@@ -50,10 +50,8 @@ class Facts(FactsBase):
         :rtype: dict
         :return: the facts gathered
         """
-        netres_choices = FactsArgs.argument_spec['gather_network_resources'].get('choices', [])
         if self.VALID_RESOURCE_SUBSETS:
-            self.get_network_resources_facts(netres_choices, FACT_RESOURCE_SUBSETS,
-                                             resource_facts_type, data)
+            self.get_network_resources_facts(FACT_RESOURCE_SUBSETS, resource_facts_type, data)
         if self.VALID_LEGACY_GATHER_SUBSETS:
             self.get_network_legacy_facts(FACT_LEGACY_SUBSETS, legacy_facts_type)
 

lib/ansible/modules/network/eos/eos_facts.py

@@ -45,15 +45,14 @@ options:
         all and the resources like interfaces, vlans etc.
         Can specify a list of values to include a larger subset. Values
         can also be used with an initial C(M(!)) to specify that a
-        specific subset should not be collected.
+        specific subset should not be collected. Values can also be used
+        with an initial C(M(!)) to specify that a specific subset should
+        not be collected.
+        Valid subsets are 'all', 'interfaces', 'l2_interfaces', 'l3_interfaces',
+        'lacp', 'lacp_interfaces', 'lag_interfaces', 'lldp_global', 'lldp_interfaces',
+        'vlans'.
     required: false
     type: list
-    choices: [
-        'all', '!all', 'interfaces', '!interfaces', 'l2_interfaces', '!l2_interfaces',
-        'l3_interfaces', '!l3_interfaces', 'lacp', '!lacp', 'lacp_interfaces', '!lacp_interfaces',
-        'lag_interfaces', '!lag_interfaces', 'lldp_global', '!lldp_global', 'lldp_interfaces', '!lldp_interfaces',
-        'vlans', '!vlans',
-    ]
     version_added: "2.9"
 """
 

lib/ansible/modules/network/exos/exos_facts.py

@@ -60,7 +60,9 @@ options:
         to a given subset. Possible values for this argument include
         all and the resources like interfaces, vlans etc.
         Can specify a list of values to include a larger subset.
-    choices: ['all', '!all', 'lldp_global', '!lldp_global']
+        Values can also be used with an initial C(M(!)) to specify that
+        a specific subset should not be collected.
+        Valid subsets are 'all', 'lldp_global'.
     type: list
     version_added: "2.9"
 """

lib/ansible/modules/network/ios/ios_facts.py

@@ -54,9 +54,11 @@ options:
         to a given subset. Possible values for this argument include
         all and the resources like interfaces, vlans etc.
         Can specify a list of values to include a larger subset.
-    choices: ['all', '!all', 'interfaces', '!interfaces', 'l2_interfaces', '!l2_interfaces', 'vlans', '!vlans',
-    'lag_interfaces', '!lag_interfaces', 'lacp', '!lacp', 'lacp_interfaces', '!lacp_interfaces', 'lldp_global',
-    '!lldp_global', 'lldp_interfaces', '!lldp_interfaces', 'l3_interfaces', '!l3_interfaces']
+        Values can also be used with an initial C(M(!)) to specify that
+        a specific subset should not be collected.
+        Valid subsets are 'all', 'interfaces', 'l2_interfaces', 'vlans',
+        'lag_interfaces', 'lacp', 'lacp_interfaces', 'lldp_global',
+        'lldp_interfaces', 'l3_interfaces'.
     version_added: "2.9"
 """
 

lib/ansible/modules/network/iosxr/iosxr_facts.py

@@ -53,10 +53,10 @@ options:
         Can specify a list of values to include a larger subset. Values
         can also be used with an initial C(M(!)) to specify that a
         specific subset should not be collected.
+        Valid subsets are 'all', 'lacp', 'lacp_interfaces', 'lldp_global',
+        'lldp_interfaces', 'interfaces', 'l2_interfaces', 'l3_interfaces',
+        'lag_interfaces'.
     required: false
-    choices: ['all', 'lacp', '!lacp', 'lacp_interfaces', '!lacp_interfaces', 'lldp_global', '!lldp_global',
-    'lldp_interfaces', '!lldp_interfaces', 'interfaces', '!interfaces', 'l2_interfaces', '!l2_interfaces',
-    'lag_interfaces', '!lag_interfaces', 'l3_interfaces', '!l3_interfaces']
     version_added: "2.9"
 """
 

lib/ansible/modules/network/junos/junos_facts.py

@@ -63,16 +63,11 @@ options:
         to a given subset. Possible values for this argument include
         all and the resources like interfaces, vlans etc.
         Can specify a list of values to include a larger subset.
-    choices: ['all', '!all',
-              'interfaces', '!interfaces',
-              'lacp', '!lacp',
-              'lacp_interfaces', '!lacp_interfaces',
-              'lag_interfaces', '!lag_interfaces',
-              'l2_interfaces', '!l2_interfaces',
-              'l3_interfaces', '!l3_interfaces',
-              'lldp_global', '!lldp_global',
-              'lldp_interfaces', '!lldp_interfaces',
-              'vlans', '!vlans']
+        Values can also be used with an initial C(M(!)) to specify that
+        a specific subset should not be collected.
+        Valid subsets are 'all', 'interfaces', 'lacp', 'lacp_interfaces',
+              'lag_interfaces', 'l2_interfaces', 'l3_interfaces', 'lldp_global',
+              'lldp_interfaces', 'vlans'.
     required: false
     version_added: "2.9"
 requirements:

lib/ansible/modules/network/nxos/nxos_facts.py

@@ -55,20 +55,13 @@ options:
     description:
       - When supplied, this argument will restrict the facts collected
         to a given subset. Possible values for this argument include
-        all and the resources like interfaces, vlans etc.
-        Can specify a list of values to include a larger subset.
-    choices:
-      - all
-      - bfd_interfaces
-      - lag_interfaces
-      - telemetry
-      - vlans
-      - lacp
-      - lacp_interfaces
-      - interfaces
-      - l3_interfaces
-      - l2_interfaces
-      - lldp_global
+        all and the resources like interfaces, vlans etc. Can specify a
+        list of values to include a larger subset. Values can also be
+        used with an initial C(M(!)) to specify that a specific subset
+        should not be collected.
+        Valid subsets are 'all', 'bfd_interfaces', 'lag_interfaces', 'telemetry',
+        'vlans', 'lacp', 'lacp_interfaces', 'interfaces', 'l3_interfaces',
+        'l2_interfaces', 'lldp_global'.
     required: false
     version_added: "2.9"
 """

lib/ansible/modules/network/vyos/vyos_facts.py

@@ -51,10 +51,10 @@ options:
         Can specify a list of values to include a larger subset. Values
         can also be used with an initial C(M(!)) to specify that a
         specific subset should not be collected.
+        Valid subsets are 'all', 'interfaces', 'l3_interfaces', 'lag_interfaces',
+        'lldp_global', 'lldp_interfaces'.
     required: false
     version_added: "2.9"
-    choices: ['all', 'interfaces', '!interfaces', 'l3_interfaces', '!l3_interfaces','lag_interfaces', '!lag_interfaces',
-             'lldp_global', '!lldp_global','lldp_interfaces', '!lldp_interfaces']
 """
 
 EXAMPLES = """