[release-4.20] OCPBUGS-81477: node: fix serviceUpdateNotNeeded nil pointer comparison#3099
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)
|
@pliurh: This pull request references Jira Issue OCPBUGS-81477, 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 |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcaamano, pliurh 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 |
|
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. |
|
/jira refresh |
|
@arkadeepsen: This pull request references Jira Issue OCPBUGS-81477, 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 |
|
/retest-required |
1 similar comment
|
/retest-required |
|
/test e2e-aws-ovn-serial |
|
/retest-required |
|
/payload 4.20 ci blocking |
|
@arkadeepsen: trigger 5 job(s) of type blocking for the ci release of OCP 4.20
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/fb2e4020-37fa-11f1-8b6f-25ef4e435303-0 trigger 9 job(s) of type blocking for the nightly release of OCP 4.20
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/fb2e4020-37fa-11f1-8b6f-25ef4e435303-1 |
|
/retest-required |
|
/payload-job periodic-ci-openshift-hypershift-release-4.20-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/6e677e90-380e-11f1-9f56-3b4ac4f47013-0 |
|
e2e-metal-ipi-ovn-dualstack-bgp failed on the step to release the lease after the e2e tests passed. Retrying the test one more time to get a green signal. /test e2e-metal-ipi-ovn-dualstack-bgp |
|
/test qe-perfscale-payload-control-plane-6nodes |
|
/test e2e-aws-ovn-serial |
|
/test qe-perfscale-payload-control-plane-6nodes |
|
/payload-job periodic-ci-openshift-hypershift-release-4.20-periodics-e2e-aks |
|
@arkadeepsen: 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/9f0af7b0-3819-11f1-8c9b-cdc0e73c43b9-0 |
|
/retest-required |
|
/payload-job periodic-ci-openshift-hypershift-release-4.20-periodics-e2e-aks |
|
@jluhrsen: trigger 1 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/0ca4bc40-385e-11f1-9eea-2ff3583c91f6-0 |
|
/retest-required |
|
@pliurh: The following tests 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. |
|
/payload-job periodic-ci-openshift-release-main-ci-4.20-e2e-aws-upgrade-ovn-single-node |
|
@arkadeepsen: trigger 4 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/3f0c9fb0-387f-11f1-9605-ee11d7e1b93b-0 |
|
/payload-job periodic-ci-openshift-release-main-ci-4.20-e2e-azure-ovn-upgrade |
|
@arkadeepsen: trigger 1 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/d028b930-388d-11f1-99b0-4601a89ccd18-0 |
|
/test qe-perfscale-payload-control-plane-6nodes |
|
qe-perfscale-payload-control-plane-6nodes is failing due to auth error as there is network lock-down on the account. Slack thread: https://redhat-internal.slack.com/archives/CH76YSYSC/p1776234777981109?thread_ts=1776229952.898689&cid=CH76YSYSC |
|
/override ci/prow/qe-perfscale-payload-control-plane-6nodes |
|
@jcaamano: Overrode contexts on behalf of jcaamano: ci/prow/qe-perfscale-payload-control-plane-6nodes 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 kubernetes-sigs/prow repository. |
b8b321e
into
openshift:release-4.20
|
@pliurh: Jira Issue Verification Checks: Jira Issue OCPBUGS-81477 Jira Issue OCPBUGS-81477 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)
This is a clean manual cherry-pick.
📑 Description
Fixes #
Additional Information for reviewers
✅ Checks
How to verify it