bugfix: Fix no ipv6_redirects command not being sent from module to device"
#913
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NilashishC
reviewed
Dec 9, 2024
| @@ -1044,3 +1044,26 @@ def test_replaced_tag(self): | |||
| playbook["state"] = "replaced" | |||
| set_module_args(playbook, ignore_provider_arg) | |||
| self.execute_module(changed=True, commands=commands) | |||
|
|
|||
| def test_ipv6_redirects(self): | |||
There was a problem hiding this comment.
This test case seems to pass even without the changes in this PR. I don't think this covers the bug being solved.
NilashishC
reviewed
Dec 9, 2024
| no_cmd = "no " if diff["ipv6_redirects"] is False else "" | ||
| commands.append(no_cmd + "ipv6 redirects") | ||
| self.cmd_order_fixup(commands, name) | ||
| # if diff["ipv6_redirects"] != self.check_existing(name, "ipv6_redirects"): |
There was a problem hiding this comment.
This check must have been added previously for a reason and we should not drop it without a proper justification.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
When an interface is already configured with both an ipv4 address and ipv6 address (dual stacked), the
no ipv6 redirectcommand is NOT sent from the module to the device.ISSUE TYPE
COMPONENT NAME
nxos.l3_interfacesADDITIONAL INFORMATION
So the NX-OS module has a built-in behaviour where the device may auto-disable IPv6 redirects in certain scenarios (e.g., dual-stack configuration), and explicitly setting both redirects and ipv6_redirects may be required to achieve the desired state.
The note in the add_commands function, indicates the following:
means that the device has a built-in behavior where it auto-disables IPv6 redirects in certain scenarios, such as dual-stacked interfaces (both IPv4 and IPv6 configured) or when multiple IP addresses are present (secondary addresses).
The no ipv6 redirects command may depend on the state of redirects (IPv4 redirects). This is an inherent behavior of the device.
1. Expected Behavior:
When ipv6_redirects: false is specified in the playbook, the module should send the command:
no ipv6 redirects2. Actual Behavior: (My observation)
If only ipv6_redirects: false is set, the no ipv6 redirects command may be skipped because the device already auto-disables IPv6 redirects.
Setting both redirects: false and ipv6_redirects: false ensures both commands (no ip redirects and no ipv6 redirects) are sent, overriding the device's implicit behavior.
3. Proposed solution 1: Update the playbook
this would execute the following commands:
Proposed solution 2: Update the code to execute the command when explicitly configured in the playbook, regardless of the device's current behavior