From 97070c968841c534147501c20bd240f3b77d7ae8 Mon Sep 17 00:00:00 2001 From: saichint Date: Tue, 22 May 2018 01:49:34 -0700 Subject: [PATCH] fix nxos_system issues (#40347) --- .../modules/network/nxos/nxos_system.py | 105 ++++++++++------- .../tests/cli/set_name_servers.yaml | 25 ++++- .../nxos_system/tests/common/sanity.yaml | 106 +++++++++++++++++- .../tests/common/set_hostname.yaml | 4 +- .../tests/nxapi/set_name_servers.yaml | 29 ++++- 5 files changed, 209 insertions(+), 60 deletions(-) diff --git a/lib/ansible/modules/network/nxos/nxos_system.py b/lib/ansible/modules/network/nxos/nxos_system.py index c41760b7e84..b2db9105153 100644 --- a/lib/ansible/modules/network/nxos/nxos_system.py +++ b/lib/ansible/modules/network/nxos/nxos_system.py @@ -36,14 +36,15 @@ description: options: hostname: description: - - Configure the device hostname parameter. This option takes an ASCII string value. + - Configure the device hostname parameter. This option takes an ASCII string value + or keyword 'default' domain_name: description: - Configures the default domain name suffix to be used when referencing this node by its FQDN. This argument accepts either a list of domain names or - a list of dicts that configure the domain name and VRF name. See - examples. + a list of dicts that configure the domain name and VRF name or + keyword 'default'. See examples. domain_lookup: description: - Enables or disables the DNS @@ -55,17 +56,17 @@ options: - Configures a list of domain name suffixes to search when performing DNS name resolution. This argument accepts either a list of domain names or - a list of dicts that configure the domain name and VRF name. See - examples. + a list of dicts that configure the domain name and VRF name or + keyword 'default'. See examples. name_servers: description: - List of DNS name servers by IP address to use to perform name resolution lookups. This argument accepts either a list of DNS servers or - a list of hashes that configure the name server and VRF name. See - examples. + a list of hashes that configure the name server and VRF name or + keyword 'default'. See examples. system_mtu: description: - - Specifies the mtu, must be an integer. + - Specifies the mtu, must be an integer or keyword 'default'. state: description: - State of the configuration @@ -174,40 +175,67 @@ def map_obj_to_commands(want, have, module): if state == 'present': if needs_update('hostname'): - commands.append('hostname %s' % want['hostname']) - - if needs_update('domain_lookup'): - cmd = 'ip domain-lookup' - if want['domain_lookup'] is False: - cmd = 'no %s' % cmd - commands.append(cmd) + if want['hostname'] == 'default': + if have['hostname']: + commands.append('no hostname') + else: + commands.append('hostname %s' % want['hostname']) + + if want.get('domain_lookup') is not None: + if have.get('domain_lookup') != want.get('domain_lookup'): + cmd = 'ip domain-lookup' + if want['domain_lookup'] is False: + cmd = 'no %s' % cmd + commands.append(cmd) if want['domain_name']: - for item in difference(have, want, 'domain_name'): - cmd = 'no ip domain-name %s' % item['name'] - remove(cmd, commands, item['vrf']) - for item in difference(want, have, 'domain_name'): - cmd = 'ip domain-name %s' % item['name'] - add(cmd, commands, item['vrf']) + if want.get('domain_name')[0]['name'] == 'default': + if have['domain_name']: + for item in have['domain_name']: + cmd = 'no ip domain-name %s' % item['name'] + remove(cmd, commands, item['vrf']) + else: + for item in difference(have, want, 'domain_name'): + cmd = 'no ip domain-name %s' % item['name'] + remove(cmd, commands, item['vrf']) + for item in difference(want, have, 'domain_name'): + cmd = 'ip domain-name %s' % item['name'] + add(cmd, commands, item['vrf']) if want['domain_search']: - for item in difference(have, want, 'domain_search'): - cmd = 'no ip domain-list %s' % item['name'] - remove(cmd, commands, item['vrf']) - for item in difference(want, have, 'domain_search'): - cmd = 'ip domain-list %s' % item['name'] - add(cmd, commands, item['vrf']) + if want.get('domain_search')[0]['name'] == 'default': + if have['domain_search']: + for item in have['domain_search']: + cmd = 'no ip domain-list %s' % item['name'] + remove(cmd, commands, item['vrf']) + else: + for item in difference(have, want, 'domain_search'): + cmd = 'no ip domain-list %s' % item['name'] + remove(cmd, commands, item['vrf']) + for item in difference(want, have, 'domain_search'): + cmd = 'ip domain-list %s' % item['name'] + add(cmd, commands, item['vrf']) if want['name_servers']: - for item in difference(have, want, 'name_servers'): - cmd = 'no ip name-server %s' % item['server'] - remove(cmd, commands, item['vrf']) - for item in difference(want, have, 'name_servers'): - cmd = 'ip name-server %s' % item['server'] - add(cmd, commands, item['vrf']) + if want.get('name_servers')[0]['server'] == 'default': + if have['name_servers']: + for item in have['name_servers']: + cmd = 'no ip name-server %s' % item['server'] + remove(cmd, commands, item['vrf']) + else: + for item in difference(have, want, 'name_servers'): + cmd = 'no ip name-server %s' % item['server'] + remove(cmd, commands, item['vrf']) + for item in difference(want, have, 'name_servers'): + cmd = 'ip name-server %s' % item['server'] + add(cmd, commands, item['vrf']) if needs_update('system_mtu'): - commands.append('system jumbomtu %s' % want['system_mtu']) + if want['system_mtu'] == 'default': + if have['system_mtu']: + commands.append('no system jumbomtu') + else: + commands.append('system jumbomtu %s' % want['system_mtu']) return commands @@ -269,7 +297,7 @@ def parse_name_servers(config, vrf_config, vrfs): def parse_system_mtu(config): match = re.search(r'^system jumbomtu (\d+)', config, re.M) if match: - return int(match.group(1)) + return match.group(1) def map_config_to_obj(module): @@ -293,11 +321,6 @@ def map_config_to_obj(module): } -def validate_system_mtu(value, module): - if not 1500 <= value <= 9216: - module.fail_json(msg='system_mtu must be between 1500 and 9216') - - def map_params_to_obj(module): obj = { 'hostname': module.params['hostname'], @@ -346,7 +369,7 @@ def main(): # { server: ; vrf: } name_servers=dict(type='list'), - system_mtu=dict(type='int'), + system_mtu=dict(type='str'), state=dict(default='present', choices=['present', 'absent']) ) diff --git a/test/integration/targets/nxos_system/tests/cli/set_name_servers.yaml b/test/integration/targets/nxos_system/tests/cli/set_name_servers.yaml index 7a593f64624..950d9768915 100644 --- a/test/integration/targets/nxos_system/tests/cli/set_name_servers.yaml +++ b/test/integration/targets/nxos_system/tests/cli/set_name_servers.yaml @@ -2,7 +2,7 @@ - debug: msg="START cli/set_name_servers.yaml" - name: setup - nxos_config: + nxos_config: &reset lines: - no ip name-server 1.1.1.1 - no ip name-server 2.2.2.2 @@ -76,12 +76,25 @@ - result.commands|length == 1 - "'no ip name-server 3.3.3.3' in result.commands" +- name: default name server + nxos_system: &defns + name_servers: default + register: result + +- assert: + that: + - result.changed == true + +- name: Idempotent check + nxos_system: *defns + register: result + +- assert: + that: + - result.changed == false + - name: teardown - nxos_config: - lines: - - no ip lookup source-interface - match: none + nxos_config: *reset ignore_errors: yes - # FIXME Copied from iosxr, not sure what we need here - debug: msg="END cli/set_name_servers.yaml" diff --git a/test/integration/targets/nxos_system/tests/common/sanity.yaml b/test/integration/targets/nxos_system/tests/common/sanity.yaml index 642b42745f0..2220a0f7d2c 100644 --- a/test/integration/targets/nxos_system/tests/common/sanity.yaml +++ b/test/integration/targets/nxos_system/tests/common/sanity.yaml @@ -4,28 +4,124 @@ when: ansible_connection == "local" - block: + - name: remove configuration + nxos_system: &remove + state: absent + register: result + ignore_errors: yes + + - name: configure domain lookup + nxos_system: &dlo + domain_lookup: true + state: present + register: result + - name: configure hostname and domain-name nxos_system: &hostname hostname: switch domain_name: test.example.com + register: result - - name: remove configuration - nxos_system: - state: absent + - assert: &true + that: + - "result.changed == true" + + - name: Idempotence check + nxos_system: *hostname + register: result + + - assert: &false + that: + - "result.changed == false" - name: configure name servers - nxos_system: + nxos_system: &ns name_servers: - 8.8.8.8 - 8.8.4.4 + register: result + + - assert: *true + + - name: Idempotence check + nxos_system: *ns + register: result + + - assert: *false - name: configure name servers with VRF support - nxos_system: + nxos_system: &nsv name_servers: - { server: 8.8.8.8, vrf: management } - { server: 8.8.4.4, vrf: management } + register: result + + - assert: *true + + - name: Idempotence check + nxos_system: *nsv + register: result + + - assert: *false + + - name: configure domain lookup1 + nxos_system: &ndlo + domain_lookup: false + register: result + + - assert: *true + + - name: Idempotence check + nxos_system: *ndlo + register: result + + - assert: *false + + - name: configure domain lookup2 + nxos_system: *dlo + register: result + + - assert: *true + + - name: Idempotence check + nxos_system: *dlo + register: result + + - assert: *false + + - name: configure system mtu + nxos_system: &sysmtu + system_mtu: 3000 + register: result + + - assert: *true + + - name: Idempotence check + nxos_system: *sysmtu + register: result + + - assert: *false + + - name: default configuration + nxos_system: &default + hostname: default + domain_name: default + name_servers: default + system_mtu: default + register: result + + - assert: *true + + - name: Idempotence check + nxos_system: *default + register: result + + - assert: *false always: + - name: remove configuration + nxos_system: *remove + - name: Re-configure hostname nxos_system: *hostname diff --git a/test/integration/targets/nxos_system/tests/common/set_hostname.yaml b/test/integration/targets/nxos_system/tests/common/set_hostname.yaml index 81c61a28f26..6d491b1ff23 100644 --- a/test/integration/targets/nxos_system/tests/common/set_hostname.yaml +++ b/test/integration/targets/nxos_system/tests/common/set_hostname.yaml @@ -6,7 +6,7 @@ - block: - name: setup nxos_config: - lines: hostname switch + lines: "hostname {{ inventory_hostname }}" match: none - name: configure hostname @@ -30,7 +30,7 @@ always: - name: teardown nxos_config: - lines: hostname switch + lines: "hostname {{ inventory_hostname }}" match: none diff --git a/test/integration/targets/nxos_system/tests/nxapi/set_name_servers.yaml b/test/integration/targets/nxos_system/tests/nxapi/set_name_servers.yaml index 99fc9e791f8..2394903bf6f 100644 --- a/test/integration/targets/nxos_system/tests/nxapi/set_name_servers.yaml +++ b/test/integration/targets/nxos_system/tests/nxapi/set_name_servers.yaml @@ -5,7 +5,7 @@ # nxapi will error if you try and remove a non-existent entry, # Therefore we do this as a with_items loop with ignore_errors - name: setup - nxos_config: + nxos_config: &reset lines: - no ip name-server {{ item }} match: none @@ -82,12 +82,29 @@ - result.commands|length == 1 - "'no ip name-server 3.3.3.3' in result.commands" +- name: default name server + nxos_system: &defns + name_servers: default + register: result + +- assert: + that: + - result.changed == true + +- name: Idempotent check + nxos_system: *defns + register: result + +- assert: + that: + - result.changed == false + - name: teardown - nxos_config: - lines: - - no ip lookup source-interface - match: none + nxos_config: *reset + with_items: + - 1.1.1.1 + - 2.2.2.2 + - 3.3.3.3 ignore_errors: yes - # FIXME Copied from iosxr, not sure what we need here - debug: msg="END nxapi/set_name_servers.yaml"