From 4a1045e195df98f6da9dfce0a6e83884f4f8c2af Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Tue, 7 May 2019 12:13:24 -0400 Subject: [PATCH 1/7] Alter tests to pass --- lib/ansible/modules/network/eos/eos_config.py | 2 + .../eos_config/tests/cli/check_mode.yaml | 3 +- .../cli/sublevel_strict_mul_parents.yaml | 103 +++++++++--------- 3 files changed, 57 insertions(+), 51 deletions(-) diff --git a/lib/ansible/modules/network/eos/eos_config.py b/lib/ansible/modules/network/eos/eos_config.py index 1bbd5d1e398d08..d4f20cd06cbf15 100644 --- a/lib/ansible/modules/network/eos/eos_config.py +++ b/lib/ansible/modules/network/eos/eos_config.py @@ -440,6 +440,8 @@ def main(): if module.params['diff_against'] == 'session': if 'diff' in response: result['diff'] = {'prepared': response['diff']} + elif 'session' not in response: + warnings.append('Configuration not checked against session as session was not used') else: result['changed'] = False 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 424b8bd15ee00a..897748b6d6e753 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: 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 36204438fc1c8b..8b8805e23a60e1 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,62 @@ 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 + 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 + 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 + 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 }}" From 43d24d63f136b4f7bfea459f3c2e29cc46b17b27 Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Tue, 7 May 2019 12:41:53 -0400 Subject: [PATCH 2/7] Change diff_against to make changed work again --- .../eos_config/tests/cli/sublevel_strict_mul_parents.yaml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 8b8805e23a60e1..ec99a61c6fffcf 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 @@ -21,12 +21,15 @@ - 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" + - "result.changed == true" - "'set cos 1' in result.updates" - "'set dscp 62' in result.updates" @@ -37,12 +40,13 @@ - 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" + - "result.changed == true" - "'set cos 1' in result.updates" - "'set dscp 62' in result.updates" From 10af202463cda7c596cd09c60b4b3d47dc99f79e Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Tue, 7 May 2019 16:02:26 -0400 Subject: [PATCH 3/7] Add another diff_against --- .../eos_config/tests/cli/sublevel_strict_mul_parents.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 ec99a61c6fffcf..4db54724cf7d99 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 @@ -53,12 +53,13 @@ - 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 + #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 From 20153f092c919e98f97e2d9235d89426bb1a0015 Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Wed, 8 May 2019 10:51:41 -0400 Subject: [PATCH 4/7] Expose supports_sessions across all EOS connection types --- lib/ansible/module_utils/network/eos/eos.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/ansible/module_utils/network/eos/eos.py b/lib/ansible/module_utils/network/eos/eos.py index 3aaafc5d63b7b3..6a18fbabb3cc03 100644 --- a/lib/ansible/module_utils/network/eos/eos.py +++ b/lib/ansible/module_utils/network/eos/eos.py @@ -121,6 +121,13 @@ def __init__(self, module): self._session_support = None self._connection = None + @property + def supports_sessions(self): + if self._session_support: + return self._session_support + self._session_support = self._get_connection().supports_sessions + return self._session_support + def _get_connection(self): if self._connection: return self._connection @@ -428,6 +435,13 @@ def _connection(self): return self._connection_obj + @property + def supports_sessions(self): + if self._session_support: + return self._session_support + 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 """ From b0d60bfd5598060a12125bf309543fed20a46e40 Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Wed, 8 May 2019 10:53:03 -0400 Subject: [PATCH 5/7] Change session warning to failure --- lib/ansible/modules/network/eos/eos_config.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/network/eos/eos_config.py b/lib/ansible/modules/network/eos/eos_config.py index d4f20cd06cbf15..52479a9dbe0bb3 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) @@ -440,8 +444,6 @@ def main(): if module.params['diff_against'] == 'session': if 'diff' in response: result['diff'] = {'prepared': response['diff']} - elif 'session' not in response: - warnings.append('Configuration not checked against session as session was not used') else: result['changed'] = False From 9837a20580354b36af40ee642f95f95dc6e521c7 Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Wed, 8 May 2019 11:18:48 -0400 Subject: [PATCH 6/7] supports_sessions needs to be a method to survive the rpc boundary --- lib/ansible/module_utils/network/eos/eos.py | 17 +++++++---------- lib/ansible/plugins/cliconf/eos.py | 17 ++++++++--------- lib/ansible/plugins/httpapi/eos.py | 9 ++++----- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/lib/ansible/module_utils/network/eos/eos.py b/lib/ansible/module_utils/network/eos/eos.py index 6a18fbabb3cc03..784113981d1266 100644 --- a/lib/ansible/module_utils/network/eos/eos.py +++ b/lib/ansible/module_utils/network/eos/eos.py @@ -123,9 +123,8 @@ def __init__(self, module): @property def supports_sessions(self): - if self._session_support: - return self._session_support - self._session_support = self._get_connection().supports_sessions + if self._session_support is None: + self._session_support = self._get_connection().supports_sessions() return self._session_support def _get_connection(self): @@ -237,10 +236,9 @@ def __init__(self, module): @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): @@ -437,9 +435,8 @@ def _connection(self): @property def supports_sessions(self): - if self._session_support: - return self._session_support - self._session_support = self._connection.supports_sessions + 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): diff --git a/lib/ansible/plugins/cliconf/eos.py b/lib/ansible/plugins/cliconf/eos.py index 291788d19620c0..56c6defb133fa7 100644 --- a/lib/ansible/plugins/cliconf/eos.py +++ b/lib/ansible/plugins/cliconf/eos.py @@ -83,12 +83,12 @@ def edit_config(self, candidate=None, commit=True, replace=None, comment=None): 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 @@ def edit_config(self, candidate=None, commit=True, replace=None, comment=None): 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 @@ def commit(self): 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 @@ def get_diff(self, candidate=None, running=None, diff_match='line', diff_ignore_ 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 @@ def get_device_info(self): 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 72306d19f9c9eb..2221eb8c85876f 100644 --- a/lib/ansible/plugins/httpapi/eos.py +++ b/lib/ansible/plugins/httpapi/eos.py @@ -48,7 +48,6 @@ def __init__(self, *args, **kwargs): 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 @@ def get_device_info(self): 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): From de8e8df0238ceb8a424c3077421e8a6c9e379fb8 Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Wed, 8 May 2019 11:19:44 -0400 Subject: [PATCH 7/7] Alter tests to match --- .../eos_config/tests/cli/check_mode.yaml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 897748b6d6e753..3961239547d264 100644 --- a/test/integration/targets/eos_config/tests/cli/check_mode.yaml +++ b/test/integration/targets/eos_config/tests/cli/check_mode.yaml @@ -40,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: @@ -61,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