-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[EPG Static Port] Make VLAN parameter optional when the state is absent (DCNE-300) #608
base: master
Are you sure you want to change the base?
Conversation
# ADD STATIC PORT WITHOUT VLAN | ||
- name: Add static port 3 (dpc) to EPG6 of AP2 without VLAN (error) | ||
cisco.mso.mso_schema_site_anp_epg_staticport: | ||
<<: *mso_info | ||
schema: '{{ mso_schema | default("ansible_test") }}' | ||
site: '{{ mso_site | default("ansible_test") }}' | ||
template: Template 1 | ||
anp: AP2 | ||
epg: EPG6 | ||
pod: pod-2 | ||
leaf: 102 | ||
path: eth1/3 | ||
deployment_immediacy: lazy | ||
mode: regular | ||
type: dpc | ||
primary_micro_segment_vlan: 199 | ||
state: present | ||
ignore_errors: true | ||
register: nm_add_stat3e6 | ||
|
||
- name: Verify nm_add_stat3e6 | ||
ansible.builtin.assert: | ||
that: | ||
- nm_add_stat3e6 is failed | ||
- nm_add_stat3e6.msg == "state is present or absent but all of the following are missing{{":"}} vlan." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case is to ensure that the change won't affect the present
state. (vlan
is still needed)
# ADD STATIC PORT WITH VLAN | ||
- name: Add static port 3 (dpc) to EPG6 of AP2 with VLAN (success) | ||
cisco.mso.mso_schema_site_anp_epg_staticport: | ||
<<: *mso_info | ||
schema: '{{ mso_schema | default("ansible_test") }}' | ||
site: '{{ mso_site | default("ansible_test") }}' | ||
template: Template 1 | ||
anp: AP2 | ||
epg: EPG6 | ||
pod: pod-2 | ||
leaf: 102 | ||
path: eth1/3 | ||
vlan: 100 | ||
deployment_immediacy: lazy | ||
mode: regular | ||
type: dpc | ||
primary_micro_segment_vlan: 199 | ||
state: present | ||
register: nm_add_stat3e6 | ||
|
||
- name: Verify nm_add_stat3e6 | ||
ansible.builtin.assert: | ||
that: | ||
- nm_add_stat3e6 is changed | ||
- nm_add_stat3e6.previous == {} | ||
- nm_add_stat3e6.current.deploymentImmediacy == 'lazy' | ||
- nm_add_stat3e6.current.portEncapVlan == 100 | ||
- nm_add_stat3e6.current.microSegVlan == 199 | ||
- nm_add_stat3e6.current.path == 'topology/pod-2/paths-102/pathep-[eth1/3]' | ||
- nm_add_stat3e6.current.mode == 'regular' | ||
- nm_add_stat3e6.current.type == 'dpc' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case is to ensure that the change won't affect the present
state. (vlan
is still needed). And this static port will be used for deletion in the next test case.
(I checked topology/pod-2/paths-102/pathep-[eth1/3]
is not currently in use by the other tests.)
# REMOVE STATIC PORT WITHOUT VLAN | ||
- name: Remove static port 3 (dpc) from EPG6 of AP2 without VLAN (success) | ||
cisco.mso.mso_schema_site_anp_epg_staticport: | ||
<<: *mso_info | ||
schema: '{{ mso_schema | default("ansible_test") }}' | ||
site: '{{ mso_site | default("ansible_test") }}' | ||
template: Template 1 | ||
anp: AP2 | ||
epg: EPG6 | ||
pod: pod-2 | ||
leaf: 102 | ||
path: eth1/3 | ||
state: absent | ||
register: nm_add_stat3e6 | ||
|
||
- name: Verify nm_add_stat3e6 | ||
ansible.builtin.assert: | ||
that: | ||
- nm_add_stat3e6 is changed | ||
- nm_add_stat3e6.current == {} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if we can remove the static port without VLAN
# ADD STATIC PORT WITHOUT VLAN (BULK) | ||
- name: Add static port 3 & 4 (dpc) to epg_bulk of anp_bulk without VLAN (error) | ||
cisco.mso.mso_schema_site_anp_epg_staticport: | ||
<<: *mso_info | ||
schema: '{{ mso_schema | default("ansible_test") }}' | ||
site: '{{ mso_site | default("ansible_test") }}' | ||
template: template_bulk | ||
anp: anp_bulk | ||
epg: epg_bulk | ||
static_ports: | ||
- pod: pod-2 | ||
leaf: 102 | ||
path: eth1/3 | ||
deployment_immediacy: lazy | ||
mode: regular | ||
type: dpc | ||
primary_micro_segment_vlan: 199 | ||
- pod: pod-2 | ||
leaf: 102 | ||
path: eth1/4 | ||
deployment_immediacy: lazy | ||
mode: regular | ||
type: dpc | ||
primary_micro_segment_vlan: 199 | ||
state: present | ||
ignore_errors: true | ||
register: nm_add_stat3e6 | ||
|
||
- name: Verify nm_add_stat3e6 | ||
ansible.builtin.assert: | ||
that: | ||
- nm_add_stat3e6 is failed | ||
- nm_add_stat3e6.msg == "state is present but all of the following are missing{{":"}} vlan." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[TEST IN BULK] This test case is to ensure that the change won't affect the present
state. (vlan
is still needed)
# ADD STATIC PORT WITH VLAN (BULK) | ||
- name: Add static port 3 & 4 (dpc) to epg_bulk of anp_bulk with VLAN (success) | ||
cisco.mso.mso_schema_site_anp_epg_staticport: | ||
<<: *mso_info | ||
schema: '{{ mso_schema | default("ansible_test") }}' | ||
site: '{{ mso_site | default("ansible_test") }}' | ||
template: template_bulk | ||
anp: anp_bulk | ||
epg: epg_bulk | ||
static_ports: | ||
- pod: pod-2 | ||
leaf: 102 | ||
path: eth1/3 | ||
vlan: 100 | ||
deployment_immediacy: lazy | ||
mode: regular | ||
type: dpc | ||
primary_micro_segment_vlan: 199 | ||
- pod: pod-2 | ||
leaf: 102 | ||
path: eth1/4 | ||
vlan: 100 | ||
deployment_immediacy: lazy | ||
mode: regular | ||
type: dpc | ||
primary_micro_segment_vlan: 199 | ||
state: present | ||
register: nm_add_bulk_stat3e6 | ||
|
||
- name: Verify nm_add_bulk_stat3e6 | ||
ansible.builtin.assert: | ||
that: | ||
- nm_add_bulk_stat3e6 is changed | ||
- nm_add_bulk_stat3e6.previous | length == 10 | ||
- nm_add_bulk_stat3e6.previous.0.path == "topology/pod-1/paths-101/pathep-[eth1/1]" | ||
- nm_add_bulk_stat3e6.previous.0.portEncapVlan == 1101 | ||
- nm_add_bulk_stat3e6.previous.2.path == "topology/pod-1/paths-101/pathep-[eth1/5]" | ||
- nm_add_bulk_stat3e6.previous.2.portEncapVlan == 1105 | ||
- nm_add_bulk_stat3e6.previous.4.path == "topology/pod-1/paths-101/pathep-[eth1/9]" | ||
- nm_add_bulk_stat3e6.previous.4.portEncapVlan == 1109 | ||
- nm_add_bulk_stat3e6.previous.5.path == "topology/pod-1/paths-101/pathep-[eth2/1]" | ||
- nm_add_bulk_stat3e6.previous.5.portEncapVlan == 1201 | ||
- nm_add_bulk_stat3e6.previous.7.path == "topology/pod-1/paths-101/pathep-[eth2/5]" | ||
- nm_add_bulk_stat3e6.previous.7.portEncapVlan == 1205 | ||
- nm_add_bulk_stat3e6.previous.9.path == "topology/pod-1/paths-101/pathep-[eth2/9]" | ||
- nm_add_bulk_stat3e6.previous.9.portEncapVlan == 1209 | ||
- nm_add_bulk_stat3e6.current | length == 12 | ||
- nm_add_bulk_stat3e6.current.10.deploymentImmediacy == 'lazy' | ||
- nm_add_bulk_stat3e6.current.10.portEncapVlan == 100 | ||
- nm_add_bulk_stat3e6.current.10.microSegVlan == 199 | ||
- nm_add_bulk_stat3e6.current.10.path == 'topology/pod-2/paths-102/pathep-[eth1/3]' | ||
- nm_add_bulk_stat3e6.current.10.mode == 'regular' | ||
- nm_add_bulk_stat3e6.current.10.type == 'dpc' | ||
- nm_add_bulk_stat3e6.current.11.deploymentImmediacy == 'lazy' | ||
- nm_add_bulk_stat3e6.current.11.portEncapVlan == 100 | ||
- nm_add_bulk_stat3e6.current.11.microSegVlan == 199 | ||
- nm_add_bulk_stat3e6.current.11.path == 'topology/pod-2/paths-102/pathep-[eth1/4]' | ||
- nm_add_bulk_stat3e6.current.11.mode == 'regular' | ||
- nm_add_bulk_stat3e6.current.11.type == 'dpc' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[TEST IN BULK] This test case is to ensure that the change won't affect the present
state. (vlan
is still needed). And these 2 static ports will be used for deletion in the next test case.
(I checked topology/pod-2/paths-102/pathep-[eth1/3]
& topology/pod-2/paths-102/pathep-[eth1/4]
are not currently in use by the other tests.)
# REMOVE STATIC PORT WITHOUT VLAN (BULK) | ||
- name: Remove static port 3 & 4 (dpc) from epg_bulk of anp_bulk without VLAN (success) | ||
cisco.mso.mso_schema_site_anp_epg_staticport: | ||
<<: *mso_info | ||
schema: '{{ mso_schema | default("ansible_test") }}' | ||
site: '{{ mso_site | default("ansible_test") }}' | ||
template: template_bulk | ||
anp: anp_bulk | ||
epg: epg_bulk | ||
static_ports: | ||
- pod: pod-2 | ||
leaf: 102 | ||
path: eth1/3 | ||
- pod: pod-2 | ||
leaf: 102 | ||
path: eth1/4 | ||
state: absent | ||
register: nm_rm_bulk_stat3e6 | ||
|
||
- name: Verify nm_rm_bulk_stat3e6 | ||
ansible.builtin.assert: | ||
that: | ||
- nm_rm_bulk_stat3e6 is changed | ||
- nm_rm_bulk_stat3e6.current | length == 10 | ||
- nm_add_bulk_stat3e6.current.0.path == "topology/pod-1/paths-101/pathep-[eth1/1]" | ||
- nm_add_bulk_stat3e6.current.0.portEncapVlan == 1101 | ||
- nm_add_bulk_stat3e6.current.2.path == "topology/pod-1/paths-101/pathep-[eth1/5]" | ||
- nm_add_bulk_stat3e6.current.2.portEncapVlan == 1105 | ||
- nm_add_bulk_stat3e6.current.4.path == "topology/pod-1/paths-101/pathep-[eth1/9]" | ||
- nm_add_bulk_stat3e6.current.4.portEncapVlan == 1109 | ||
- nm_add_bulk_stat3e6.current.5.path == "topology/pod-1/paths-101/pathep-[eth2/1]" | ||
- nm_add_bulk_stat3e6.current.5.portEncapVlan == 1201 | ||
- nm_add_bulk_stat3e6.current.7.path == "topology/pod-1/paths-101/pathep-[eth2/5]" | ||
- nm_add_bulk_stat3e6.current.7.portEncapVlan == 1205 | ||
- nm_add_bulk_stat3e6.current.9.path == "topology/pod-1/paths-101/pathep-[eth2/9]" | ||
- nm_add_bulk_stat3e6.current.9.portEncapVlan == 1209 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[TEST IN BULK] Check if we can remove the 2 static ports without VLAN
Hi @akinross, I've added the integration tests for both single and bulk operations along with the comments above. Feel free to give feedback or let me know if you need any changes. Thanks! Results![]() ![]() Summary (all tests under the module
|
@@ -512,6 +513,85 @@ | |||
that: | |||
- nm_remove_again_stat2e2 is not changed | |||
|
|||
# ADD STATIC PORT WITHOUT VLAN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test seems redundant as this behaviour is already tested
- nm_add_stat3e6.msg == "state is present or absent but all of the following are missing{{":"}} vlan." | ||
|
||
# ADD STATIC PORT WITH VLAN | ||
- name: Add static port 3 (dpc) to EPG6 of AP2 with VLAN (success) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a clear name of what this static port is used for? We assume success and only (error) on the names usually
- nm_add_stat3e6.msg == "state is present or absent but all of the following are missing{{":"}} vlan." | ||
|
||
# ADD STATIC PORT WITH VLAN | ||
- name: Add static port 3 (dpc) to EPG6 of AP2 with VLAN (success) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you create a new section at the bottom ( when the schemas and required parent objects have not been removed ) that has your tests together with a header like ## WITHOUT VLAN REMOVAL TEST ##
type: dpc | ||
primary_micro_segment_vlan: 199 | ||
state: present | ||
register: nm_add_stat3e6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you use more meaningful names for the registered vars
state: present | ||
register: nm_add_stat3e6 | ||
|
||
- name: Verify nm_add_stat3e6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert for add seems redundant as we already test creation with its error and this has not changed in your contribution
210e54d
to
4db8931
Compare
- Updated the code to make the `vlan` parameter optional when the `state` is `absent`. - This change ensures that the `vlan` parameter is not required when the state is set to `absent`, preventing potential errors or unnecessary requirements. - Added appropriate checks to handle the absence of the `vlan` parameter gracefully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description
vlan
parameter optional when thestate
isabsent
.vlan
parameter is not required when the state is set toabsent
, preventing potential errors or unnecessary requirements.vlan
parameter gracefully.New or Affected Module(s):
mso_schema_site_anp_epg_staticport
MSO version and MSO Platform
APIC version and APIC Platform for Site Level Resources
Collection versions
References