From 0bead3672f7a750dd4e8f6a9ea2586dcecfc2125 Mon Sep 17 00:00:00 2001
From: Nathaniel Case <ncase@redhat.com>
Date: Thu, 9 May 2019 08:32:25 -0400
Subject: [PATCH] eos_config: Fix test issues (#56180)

* Alter tests to pass

* Change diff_against to make changed work again

* Add another diff_against

* Expose supports_sessions across all EOS connection types

* Change session warning to failure

* supports_sessions needs to be a method to survive the rpc boundary

* Alter tests to match
---
 lib/ansible/module_utils/network/eos/eos.py   |  19 ++-
 lib/ansible/modules/network/eos/eos_config.py |   4 +
 lib/ansible/plugins/cliconf/eos.py            |  17 ++-
 lib/ansible/plugins/httpapi/eos.py            |   9 +-
 .../eos_config/tests/cli/check_mode.yaml      |  21 +++-
 .../cli/sublevel_strict_mul_parents.yaml      | 108 ++++++++++--------
 6 files changed, 109 insertions(+), 69 deletions(-)

diff --git a/lib/ansible/module_utils/network/eos/eos.py b/lib/ansible/module_utils/network/eos/eos.py
index 6a2640f3e32..77a09045cb0 100644
--- a/lib/ansible/module_utils/network/eos/eos.py
+++ b/lib/ansible/module_utils/network/eos/eos.py
@@ -121,6 +121,12 @@ class Cli:
         self._session_support = None
         self._connection = None
 
+    @property
+    def supports_sessions(self):
+        if self._session_support is None:
+            self._session_support = self._get_connection().supports_sessions()
+        return self._session_support
+
     def _get_connection(self):
         if self._connection:
             return self._connection
@@ -230,10 +236,9 @@ class LocalEapi:
 
     @property
     def supports_sessions(self):
-        if self._session_support:
-            return self._session_support
-        response = self.send_request(['show configuration sessions'])
-        self._session_support = 'error' not in response
+        if self._session_support is None:
+            response = self.send_request(['show configuration sessions'])
+            self._session_support = 'error' not in response
         return self._session_support
 
     def _request_builder(self, commands, output, reqid=None):
@@ -428,6 +433,12 @@ class HttpApi:
 
         return self._connection_obj
 
+    @property
+    def supports_sessions(self):
+        if self._session_support is None:
+            self._session_support = self._connection.supports_sessions()
+        return self._session_support
+
     def run_commands(self, commands, check_rc=True):
         """Runs list of commands on remote device and returns results
         """
diff --git a/lib/ansible/modules/network/eos/eos_config.py b/lib/ansible/modules/network/eos/eos_config.py
index 1bbd5d1e398..52479a9dbe0 100644
--- a/lib/ansible/modules/network/eos/eos_config.py
+++ b/lib/ansible/modules/network/eos/eos_config.py
@@ -396,6 +396,10 @@ def main():
     flags = ['all'] if module.params['defaults'] else []
     connection = get_connection(module)
 
+    # Refuse to diff_against: session if essions are disabled
+    if module.params['diff_against'] == 'session' and not connection.supports_sessions:
+        module.fail_json(msg="Cannot diff against sessions when sessions are disabled. Please change diff_against to another value")
+
     if module.params['backup'] or (module._diff and module.params['diff_against'] == 'running'):
         contents = get_config(module, flags=flags)
         config = NetworkConfig(indent=1, contents=contents)
diff --git a/lib/ansible/plugins/cliconf/eos.py b/lib/ansible/plugins/cliconf/eos.py
index 4662f454916..8fca11254f3 100644
--- a/lib/ansible/plugins/cliconf/eos.py
+++ b/lib/ansible/plugins/cliconf/eos.py
@@ -83,12 +83,12 @@ class Cliconf(CliconfBase):
         operations = self.get_device_operations()
         self.check_edit_config_capability(operations, candidate, commit, replace, comment)
 
-        if (commit is False) and (not self.supports_sessions):
+        if (commit is False) and (not self.supports_sessions()):
             raise ValueError('check mode is not supported without configuration session')
 
         resp = {}
         session = None
-        if self.supports_sessions:
+        if self.supports_sessions():
             session = 'ansible_%s' % int(time.time())
             resp.update({'session': session})
             self.send_command('configure session %s' % session)
@@ -125,7 +125,7 @@ class Cliconf(CliconfBase):
 
         resp['request'] = requests
         resp['response'] = results
-        if self.supports_sessions:
+        if self.supports_sessions():
             out = self.send_command('show session-config diffs')
             if out:
                 resp['diff'] = out.strip()
@@ -148,7 +148,7 @@ class Cliconf(CliconfBase):
 
     def discard_changes(self, session=None):
         commands = ['end']
-        if self.supports_sessions:
+        if self.supports_sessions():
             # to close session gracefully execute abort in top level session prompt.
             commands.extend(['configure session %s' % session, 'abort'])
 
@@ -213,7 +213,6 @@ class Cliconf(CliconfBase):
         diff['config_diff'] = dumps(configdiffobjs, 'commands') if configdiffobjs else ''
         return diff
 
-    @property
     def supports_sessions(self):
         use_session = self.get_option('eos_use_sessions')
         try:
@@ -262,16 +261,16 @@ class Cliconf(CliconfBase):
     def get_device_operations(self):
         return {
             'supports_diff_replace': True,
-            'supports_commit': True if self.supports_sessions else False,
+            'supports_commit': bool(self.supports_sessions()),
             'supports_rollback': False,
             'supports_defaults': False,
-            'supports_onbox_diff': True if self.supports_sessions else False,
+            'supports_onbox_diff': bool(self.supports_sessions()),
             'supports_commit_comment': False,
             'supports_multiline_delimiter': False,
             'supports_diff_match': True,
             'supports_diff_ignore_lines': True,
-            'supports_generate_diff': False if self.supports_sessions else True,
-            'supports_replace': True if self.supports_sessions else False
+            'supports_generate_diff': not bool(self.supports_sessions()),
+            'supports_replace': bool(self.supports_sessions()),
         }
 
     def get_option_values(self):
diff --git a/lib/ansible/plugins/httpapi/eos.py b/lib/ansible/plugins/httpapi/eos.py
index 72306d19f9c..2221eb8c858 100644
--- a/lib/ansible/plugins/httpapi/eos.py
+++ b/lib/ansible/plugins/httpapi/eos.py
@@ -48,7 +48,6 @@ class HttpApi(HttpApiBase):
         self._device_info = None
         self._session_support = None
 
-    @property
     def supports_sessions(self):
         use_session = self.get_option('eos_use_sessions')
         try:
@@ -119,16 +118,16 @@ class HttpApi(HttpApiBase):
     def get_device_operations(self):
         return {
             'supports_diff_replace': True,
-            'supports_commit': bool(self.supports_sessions),
+            'supports_commit': bool(self.supports_sessions()),
             'supports_rollback': False,
             'supports_defaults': False,
-            'supports_onbox_diff': bool(self.supports_sessions),
+            'supports_onbox_diff': bool(self.supports_sessions()),
             'supports_commit_comment': False,
             'supports_multiline_delimiter': False,
             'supports_diff_match': True,
             'supports_diff_ignore_lines': True,
-            'supports_generate_diff': not bool(self.supports_sessions),
-            'supports_replace': bool(self.supports_sessions),
+            'supports_generate_diff': not bool(self.supports_sessions()),
+            'supports_replace': bool(self.supports_sessions()),
         }
 
     def get_capabilities(self):
diff --git a/test/integration/targets/eos_config/tests/cli/check_mode.yaml b/test/integration/targets/eos_config/tests/cli/check_mode.yaml
index 424b8bd15ee..3961239547d 100644
--- a/test/integration/targets/eos_config/tests/cli/check_mode.yaml
+++ b/test/integration/targets/eos_config/tests/cli/check_mode.yaml
@@ -1,5 +1,6 @@
 ---
-- debug: msg="START cli/check_mode.yaml on connection={{ ansible_connection }}"
+- debug:
+    msg: "START cli/check_mode.yaml on connection={{ ansible_connection }}"
 
 - name: invalid configuration in check mode
   eos_config:
@@ -39,11 +40,28 @@
    that:
      - "config.session not in result.stdout[0].sessions"
 
+- name: configuration in check mode + no config session
+  eos_config:
+     lines:
+         - ip address 119.31.1.1 255.255.255.254
+     parents: interface Loopback911
+  become: yes
+  check_mode: 1
+  vars:
+    ansible_eos_use_sessions: 0
+  register: result
+  ignore_errors: yes
+
+- assert:
+   that:
+   - "result.failed == true"
+
 - name: invalid configuration in check mode + no config session
   eos_config:
      lines:
          - ip address 119.31.1.1 255.255.255.256
      parents: interface Loopback911
+     diff_against: running
   become: yes
   check_mode: 1
   vars:
@@ -60,6 +78,7 @@
      lines:
          - ip address 119.31.1.1 255.255.255.255
      parents: interface Loopback911
+     diff_against: running
   become: yes
   check_mode: yes
   register: result
diff --git a/test/integration/targets/eos_config/tests/cli/sublevel_strict_mul_parents.yaml b/test/integration/targets/eos_config/tests/cli/sublevel_strict_mul_parents.yaml
index 36204438fc1..4db54724cf7 100644
--- a/test/integration/targets/eos_config/tests/cli/sublevel_strict_mul_parents.yaml
+++ b/test/integration/targets/eos_config/tests/cli/sublevel_strict_mul_parents.yaml
@@ -1,5 +1,6 @@
 ---
-- debug: msg="START cli/sublevel_strict_mul_parents.yaml on connection={{ ansible_connection }}"
+- debug:
+    msg: "START cli/sublevel_strict_mul_parents.yaml on connection={{ ansible_connection }}"
 
 - name: setup
   eos_config:
@@ -12,60 +13,67 @@
     match: none
   become: yes
 
-- name: configure sub level command using strict match
-  eos_config:
-    lines:
-      - set cos 1
-      - set dscp 62
-    parents: ['policy-map type qos p1', 'class c1']
-    match: strict
-  register: result
-  become: yes
+- block:
+  - name: configure sub level command using strict match
+    eos_config:
+      lines:
+        - set cos 1
+        - set dscp 62
+      parents: ['policy-map type qos p1', 'class c1']
+      match: strict
+      # session-config diffs does not produce a diff here for some reason.
+      # check against running-config instead.
+      diff_against: running
+    register: result
+    become: yes
 
-- assert:
-    that:
-      - "result.changed == true"
-      - "'set cos 1' in result.updates"
-      - "'set dscp 62' in result.updates"
+  - assert:
+      that:
+        - "result.changed == true"
+        - "'set cos 1' in result.updates"
+        - "'set dscp 62' in result.updates"
 
-- name: change sub level command order and config with strict match
-  eos_config:
-    lines:
-      - set dscp 62
-      - set cos 1
-    parents: ['policy-map type qos p1', 'class c1']
-    match: strict
-  register: result
-  become: yes
+  - name: change sub level command order and config with strict match
+    eos_config:
+      lines:
+        - set dscp 62
+        - set cos 1
+      parents: ['policy-map type qos p1', 'class c1']
+      match: strict
+      diff_against: running
+    register: result
+    become: yes
 
-- assert:
-    that:
-      - "result.changed == true"
-      - "'set cos 1' in result.updates"
-      - "'set dscp 62' in result.updates"
+  - assert:
+      that:
+        - "result.changed == true"
+        - "'set cos 1' in result.updates"
+        - "'set dscp 62' in result.updates"
 
-- name: Config sub level command with strict match (Idempotency)
-  eos_config:
-    lines:
-#EOS does not change order of class action if reconfigured
-#so we have to use old order for Idempotency
-      - set cos 1
-      - set dscp 62
-    parents: ['policy-map type qos p1', 'class c1']
-    match: strict
-  register: result
-  become: yes
+  - name: Config sub level command with strict match (Idempotency)
+    eos_config:
+      lines:
+        #EOS does not change order of class action if reconfigured
+        #so we have to use old order for Idempotency
+        - set cos 1
+        - set dscp 62
+      parents: ['policy-map type qos p1', 'class c1']
+      match: strict
+      diff_against: running
+    register: result
+    become: yes
 
-- assert:
-    that:
-      - "result.changed == false"
+  - assert:
+      that:
+        - "result.changed == false"
 
-- name: teardown
-  eos_config:
-    lines:
-      - no policy-map type qos p1
-      - no class-map type qos match-any c1
-    match: none
-  become: yes
+  always:
+  - name: teardown
+    eos_config:
+      lines:
+        - no policy-map type qos p1
+        - no class-map type qos match-any c1
+      match: none
+    become: yes
 
 - debug: msg="END cli/sublevel_strict_mul_parents.yaml on connection={{ ansible_connection }}"