From 6a94090e7fea601eb38d2e85d1f07d7f1b1d96a5 Mon Sep 17 00:00:00 2001
From: Ganesh Nalawade <ganesh634@gmail.com>
Date: Thu, 12 Jul 2018 09:53:03 +0530
Subject: [PATCH] Fix junos facts integration test failures (#42623)

*  Make fetching old style facts optional
   and try to fetch the old style facts only when
   `ofacts` value is present in `gather_facts`
* Fix in junos_facts integration test
---
 .../modules/network/junos/junos_facts.py       | 18 +++++++++++-------
 .../junos_facts/tests/netconf/facts.yaml       |  4 ++--
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/lib/ansible/modules/network/junos/junos_facts.py b/lib/ansible/modules/network/junos/junos_facts.py
index c68b2f708a2..66e2d3689a6 100644
--- a/lib/ansible/modules/network/junos/junos_facts.py
+++ b/lib/ansible/modules/network/junos/junos_facts.py
@@ -35,10 +35,10 @@ options:
         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. To maintain backward compatbility old style facts
-        can be retrieved using all value, this reqires junos-eznc to be installed
-        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
+        can be retrieved by explicilty adding C(ofacts)  to value, this reqires
+        junos-eznc to be installed 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
@@ -51,7 +51,7 @@ options:
          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. 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.
+         when C(ofacts) value is present in value if I(gather_subset) value.
     required: false
     default: 'text'
     choices: ['xml', 'text', 'set', 'json']
@@ -354,19 +354,23 @@ def main():
     runable_subsets.difference_update(exclude_subsets)
     runable_subsets.add('default')
 
+    # handle fetching old style facts seperately
+    runable_subsets.discard('ofacts')
+
     facts = dict()
     facts['gather_subset'] = list(runable_subsets)
 
     instances = list()
     ansible_facts = dict()
 
-    if 'ofacts' in runable_subsets:
+    # fetch old style facts only when explicitly mentioned in gather_subset option
+    if 'ofacts' in gather_subset:
         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')
+        facts['gather_subset'].append('ofacts')
 
     for key in runable_subsets:
         instances.append(FACT_SUBSETS[key](module))
diff --git a/test/integration/targets/junos_facts/tests/netconf/facts.yaml b/test/integration/targets/junos_facts/tests/netconf/facts.yaml
index d16dc338704..092283af1c9 100644
--- a/test/integration/targets/junos_facts/tests/netconf/facts.yaml
+++ b/test/integration/targets/junos_facts/tests/netconf/facts.yaml
@@ -50,7 +50,7 @@
 - assert:
     that:
       - "result.failed == true"
-      - "result.msg == 'Subset must be one of [hardware, default, interfaces, config], got test'"
+      - "result.msg == 'Subset must be one of [hardware, default, ofacts, config, interfaces], got test'"
 
 - name: Collect config facts from device in set format
   junos_facts:
@@ -92,7 +92,7 @@
 - assert:
     that:
       - "result.changed == false"
-      - "'{{ result['ansible_facts']['ansible_net_config']['configuration']['system']['service']['netconf'] }}' is defined"
+      - "'ssh' in result['ansible_facts']['ansible_net_config']['configuration']['system']['services']['netconf']"
   when: ansible_net_version == "17.3R1.10"
 
 - name: Collect config facts from device in text format