[release-4.19] OCPBUGS-81478: node: fix serviceUpdateNotNeeded nil pointer comparison#3100
Conversation
serviceUpdateNotNeeded() used explicit nil guards before dereferencing InternalTrafficPolicy and AllocateLoadBalancerNodePorts. When both old and new are nil (all non-LoadBalancer services), (nil != nil && ...) evaluates to false, incorrectly indicating an update is needed. Use reflect.DeepEqual on the pointer directly, which handles nil == nil. Signed-off-by: Peng Liu <pliu@redhat.com> (cherry picked from commit 40caf4c) (cherry picked from commit 581bfde) (cherry picked from commit 2a5bf31)
|
@pliurh: This pull request references Jira Issue OCPBUGS-81478, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note
|
|
/retest-required |
Signed-off-by: Ying Wang <yingwang@rehat.com>
|
/verified by pre-merge testing |
|
@yingwang-0320: This PR has been marked as verified by DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pliurh, tssurya The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@pliurh: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
|
Scale test details Verified the scale and additional scenarios on the cluster with fix ; - The reflect.DeepEqual() fix in OVN-Kubernetes successfully prevents TCP RST storms No RST packets generated during OVN pod restarts (trigger1) additionally conducted Service Resilience Validation Test which is API Restart Service Preservation Test 1>All 16 external IPs maintained service preservation |
|
/verified by @SachinNinganure |
|
@SachinNinganure: This PR has been marked as verified by DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/retest-required |
1 similar comment
|
/retest-required |
|
/payload 4.19 ci blocking |
|
@arkadeepsen: trigger 5 job(s) of type blocking for the ci release of OCP 4.19
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/5f80edc0-37fb-11f1-9993-c707d1a40497-0 trigger 9 job(s) of type blocking for the nightly release of OCP 4.19
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/5f80edc0-37fb-11f1-9993-c707d1a40497-1 |
|
/retest-required |
1 similar comment
|
/retest-required |
|
/payload-job periodic-ci-openshift-hypershift-release-4.19-periodics-e2e-aks |
|
@arkadeepsen: An error was encountered. No known errors were detected, please see the full error message for details. Full error message.
could not create PullRequestPayloadQualificationRun: rpc error: code = Unavailable desc = keepalive ping failed to receive ACK within timeout
Please contact an administrator to resolve this issue. |
|
/payload-job periodic-ci-openshift-hypershift-release-4.19-periodics-e2e-aks |
|
@arkadeepsen: trigger 3 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/241b65c0-3815-11f1-81f0-e0b12b1226b3-0 |
|
/payload-job periodic-ci-openshift-hypershift-release-4.19-periodics-e2e-aks |
|
@jluhrsen: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d0d02820-385e-11f1-89c5-67f891a6cace-0 |
|
/test e2e-aws-ovn-edge-zones is this supposed to be good now? I do see that it passed a few times recently, but looks pretty bad. |
|
/payload-job periodic-ci-openshift-release-main-ci-4.19-upgrade-from-stable-4.18-e2e-azure-ovn-upgrade |
|
@arkadeepsen: trigger 3 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6ba88ec0-3894-11f1-9829-a6a8506b4fbf-0 |
|
/jira refresh |
|
@arkadeepsen: This pull request references Jira Issue OCPBUGS-81478, which is invalid:
Comment DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@arkadeepsen: This pull request references Jira Issue OCPBUGS-81478, which is valid. The bug has been moved to the POST state. 7 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
cf0c520
into
openshift:release-4.19
|
@pliurh: Jira Issue Verification Checks: Jira Issue OCPBUGS-81478 Jira Issue OCPBUGS-81478 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
serviceUpdateNotNeeded() used explicit nil guards before dereferencing InternalTrafficPolicy and AllocateLoadBalancerNodePorts. When both old and new are nil (all non-LoadBalancer services), (nil != nil && ...) evaluates to false, incorrectly indicating an update is needed.
Use reflect.DeepEqual on the pointer directly, which handles nil == nil.
(cherry picked from commit 40caf4c) (cherry picked from commit 581bfde) (cherry picked from commit 2a5bf31)
📑 Description
Fixes #
Additional Information for reviewers
✅ Checks
How to verify it