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
[release-4.9] Bug 2028812: Modification of ClusterIPs shall trigger svc update #872
[release-4.9] Bug 2028812: Modification of ClusterIPs shall trigger svc update #872
Conversation
@andreaskaris: This pull request references Bugzilla bug 2028812, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/bugzilla refresh |
@andreaskaris: This pull request references Bugzilla bug 2028812, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
How to test:Test setupDeploy kind with:
Then, deploy a single stack service and then migrate it to requiredualstack:
Wait until the pod and service are up:
Then apply the following patch:
Verification:Broken:Display the service configuration:
Connect to the host that runs the pod for the next steps:
Check the node's IPv4 and IPv6 address on breth0:
Curl to the IPv4 address from the "outside":
Curl to the IPv6 address from the "outside":
Curl to the IPv6 address from the host itself is broken, too:
Observe that there are no ip6tables rules:
Working:After the fix, this should look as follows: Display the service configuration:
Connect to the host that runs the pod for the next steps:
Check the node's IPv4 and IPv6 address on breth0:
Curl to the IPv4 address from the "outside" should work:
Curl to the IPv6 address from the "outside" should work:
Curl to the IPv6 address from the host itself should work:
Observe that there are ip6tables rules for the service:
|
@andreaskaris: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@andreaskaris: This pull request references Bugzilla bug 2028812, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
2 similar comments
@andreaskaris: This pull request references Bugzilla bug 2028812, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@andreaskaris: This pull request references Bugzilla bug 2028812, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/retest-required |
/bugzilla refresh The requirements for Bugzilla bugs have changed, recalculating validity. |
@openshift-bot: This pull request references Bugzilla bug 2028812, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
18af3cb
to
c9e3f9a
Compare
When switching a service from SingleStack to DualStack, its ClusterIPs are updated. This change to ClusterIPs will now be detected and ovnkube will write IPv6 DNAT rules to the host. Conflicts: go-controller/pkg/node/gateway_shared_intf.go Signed-off-by: Andreas Karis <ak.karis@gmail.com> (cherry picked from commit aa63b98)
c9e3f9a
to
0b633ff
Compare
/retest |
/bugzilla refresh |
@andreaskaris: This pull request references Bugzilla bug 2028812, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/retest-required |
/bugzilla refresh |
@andreaskaris: This pull request references Bugzilla bug 2028812, which is valid. 6 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/lgtm |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
21 similar comments
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/label qe-approved |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest-required Please review the full test history for this PR and help us cut down flakes. |
@andreaskaris: All pull requests linked via external trackers have merged: Bugzilla bug 2028812 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
- What this PR does and why is it needed
When switching a service from SingleStack to DualStack, its ClusterIPs are updated. This change to ClusterIPs will now be detected and ovnkube will write IPv6 DNAT rules to the host. Before this fix, upgrading IPv4 only services to DualStack via a patch or oc edit would not trigger a service update. In 4.9, this affects both connections from outside the cluster as well as connections which are initialized from the node itself.
- Special notes for reviewers
In 4.10, we have a very similar upstream bug, but due to changes in the code, one can actually curl the node port from the outside and hit the service through the OVN flows.
However, in both 4.9 and 4.10 alike, the ip6tables rules are not created. Thus, even in 4.10 it is not possible to curl the node's node port from the node itself after an upgrade of a service from singlestack to dualstack.
The 4.10 upstream bug is: ovn-org/ovn-kubernetes#2700
- How to verify it
See the lengthy comment below in this pull request.
- Description for the changelog
When switching a service from SingleStack to DualStack, its
ClusterIPs are updated. This change to ClusterIPs will now be detected
and ovnkube will write IPv6 DNAT rules to the host.