Revert "daemon: automatically set network EnableIPv6 if needed"#47306
Merged
thaJeztah merged 2 commits intomoby:masterfrom Feb 2, 2024
Merged
Revert "daemon: automatically set network EnableIPv6 if needed"#47306thaJeztah merged 2 commits intomoby:masterfrom
thaJeztah merged 2 commits intomoby:masterfrom
Conversation
This reverts commit 5d5eeac. Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Commit 4f47013 introduced a new validation step to make sure no IPv6 subnet is configured on a network which has EnableIPv6=false. Commit 5d5eeac then removed that validation step and automatically enabled IPv6 for networks with a v6 subnet. But this specific commit was reverted in c59e93a and now the error introduced by 4f47013 is re-introduced. But it turns out some users expect a network created with an IPv6 subnet and EnableIPv6=false to actually have no IPv6 connectivity. This restores that behavior. Signed-off-by: Albin Kerouanton <albinker@gmail.com>
6d9f16e to
e37172c
Compare
corhere
approved these changes
Feb 2, 2024
Member
|
I thought I commented here but ETOOMANYTHREADS; this is what we were discussing in our Slack; The problem here (again) looks like our API doesn’t use a pointer for the “enable IPv6” option, so we don’t have an option to detect difference between “not set (auto)“, “true” (enable), or “false” (explicitly disabled by user) I wonder if we can transition;
(This is why I hate booleans, but the alternative would be a new API field (enum with |
thaJeztah
approved these changes
Feb 2, 2024
Member
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM to fix the issue (let's open a new ticket for looking at follow-ups?)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
- What I did
Turns out some users expect a network created with an IPv6 subnet and EnableIPv6=false to actually have no IPv6 connectivity. This PR restores that behavior.
- How to verify it
The unit test
TestNetworkWithInvalidIPAMhas been updated to make sure IPv6 subnets are ignored byValidateIPAMwhen EnableIPv6=false.- Description for the changelog
EnableIPv6isn't ignored.