Fix fetching old style facts in junos_facts module (#42336)

* Fix fetching old style facts in junos_facts module

Fixes #42298
* To fetch old style facts from junos device it requires
  login credentials in provider, hence while using
  connection=nectonf do not delete provider values
  as it might not be ignored

* Add `ofacts` as one of the options in `gather_subset`

* Minor update in documentation

* Fix CI failure

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

@@ -36,9 +36,13 @@ options:
         with an initial C(M(!)) to specify that a specific subset should
         not be collected. To maintain backward compatbility old style facts
         can be retrieved using all value, this reqires junos-eznc to be installed
-        as a prerequisite.
+        as a prerequisite. Valid value of gather_subset are default, hardware,
+        config, interfaces, ofacts. If C(ofacts) is present in the list it fetches
+        the old style facts (fact keys without 'ansible_' prefix) and it requires
+        junos-eznc library to be installed on control node and the device login credentials
+        must be given in C(provider) option.
     required: false
-    default: "!config"
+    default: ['!config', '!ofacts']
     version_added: "2.3"
   config_format:
     description:
@@ -46,10 +50,11 @@ options:
          when serializing output from the device. This argument is applicable
          only when C(config) value is present in I(gather_subset).
          The I(config_format) should be supported by the junos version running on
-         device.
+         device. This value is not applicable while fetching old style facts that is
+         when value of I(gather_subset) C(all) or C(ofacts) is present in the value.
     required: false
-    default: text
+    default: 'text'
-    choices: ['xml', 'set', 'text', 'json']
+    choices: ['xml', 'text', 'set', 'json']
     version_added: "2.3"
 requirements:
   - ncclient (>=v0.5.2)
@@ -62,6 +67,8 @@ notes:
   - Tested against vSRX JUNOS version 15.1X49-D15.4, vqfx-10000 JUNOS Version 15.1X53-D60.4.
   - Recommended connection is C(netconf). See L(the Junos OS Platform Options,../network/user_guide/platform_junos.html).
   - This module also works with C(local) connections for legacy playbooks.
+  - Fetching old style facts requires junos-eznc library to be installed on control node and the device login credentials
+    must be given in provider option.
 """
 
 EXAMPLES = """
@@ -244,7 +251,7 @@ class Interfaces(FactsBase):
         self.facts['interfaces'] = interfaces
 
 
-class Facts(FactsBase):
+class OFacts(FactsBase):
     def _connect(self, module):
         host = get_param(module, 'host')
 
@@ -260,7 +267,6 @@ class Facts(FactsBase):
             kwargs['ssh_private_key_file'] = get_param(module, 'ssh_keyfile')
 
         kwargs['gather_facts'] = False
-
         try:
             device = Device(host, **kwargs)
             device.open()
@@ -292,7 +298,8 @@ FACT_SUBSETS = dict(
     default=Default,
     hardware=Hardware,
     config=Config,
-    interfaces=Interfaces
+    interfaces=Interfaces,
+    ofacts=OFacts
 )
 
 VALID_SUBSETS = frozenset(FACT_SUBSETS.keys())
@@ -302,7 +309,7 @@ def main():
     """ Main entry point for AnsibleModule
     """
     argument_spec = dict(
-        gather_subset=dict(default=['!config'], type='list'),
+        gather_subset=dict(default=['!config', '!ofacts'], type='list'),
         config_format=dict(default='text', choices=['xml', 'text', 'set', 'json']),
     )
 
@@ -314,7 +321,6 @@ def main():
     get_connection(module)
     warnings = list()
     gather_subset = module.params['gather_subset']
-    ofacts = False
 
     runable_subsets = set()
     exclude_subsets = set()
@@ -322,14 +328,12 @@ def main():
     for subset in gather_subset:
         if subset == 'all':
             runable_subsets.update(VALID_SUBSETS)
-            ofacts = True
             continue
 
         if subset.startswith('!'):
             subset = subset[1:]
             if subset == 'all':
                 exclude_subsets.update(VALID_SUBSETS)
-                ofacts = False
                 continue
             exclude = True
         else:
@@ -354,6 +358,16 @@ def main():
     facts['gather_subset'] = list(runable_subsets)
 
     instances = list()
+    ansible_facts = dict()
+
+    if 'ofacts' in runable_subsets:
+        if HAS_PYEZ:
+            ansible_facts.update(OFacts(module).populate())
+        else:
+            warnings += ['junos-eznc is required to gather old style facts but does not appear to be installed. '
+                         'It can be installed using `pip  install junos-eznc`']
+        runable_subsets.remove('ofacts')
+
     for key in runable_subsets:
         instances.append(FACT_SUBSETS[key](module))
 
@@ -361,17 +375,10 @@ def main():
         inst.populate()
         facts.update(inst.facts)
 
-    ansible_facts = dict()
     for key, value in iteritems(facts):
         key = 'ansible_net_%s' % key
         ansible_facts[key] = value
 
-    if ofacts:
-        if HAS_PYEZ:
-            ansible_facts.update(Facts(module).populate())
-        else:
-            warnings += ['junos-eznc is required to gather old style facts but does not appear to be installed. '
-                         'It can be installed using `pip  install junos-eznc`']
     module.exit_json(ansible_facts=ansible_facts, warnings=warnings)
 
 

lib/ansible/plugins/action/junos.py

@@ -91,8 +91,11 @@ class ActionModule(_ActionModule):
         elif self._play_context.connection in ('netconf', 'network_cli'):
             provider = self._task.args.get('provider', {})
             if any(provider.values()):
-                display.warning('provider is unnecessary when using %s and will be ignored' % self._play_context.connection)
+                # for legacy reasons provider value is required for junos_facts(optional) and junos_package
-                del self._task.args['provider']
+                # modules as it uses junos_eznc library to connect to remote host
+                if not (self._task.action == 'junos_facts' or self._task.action == 'junos_package'):
+                    display.warning('provider is unnecessary when using %s and will be ignored' % self._play_context.connection)
+                    del self._task.args['provider']
 
             if (self._play_context.connection == 'network_cli' and self._task.action not in CLI_SUPPORTED_MODULES) or \
                     (self._play_context.connection == 'netconf' and self._task.action == 'junos_netconf'):