OCPBUGS-64697: [release-4.20] Referencing pod named ports within a service results in bad DNAT rules containing tcp/0 target port by ricky-rav · Pull Request #2844 · openshift/ovn-kubernetes

ricky-rav · 2025-11-05T18:49:01Z

upstream PR: OCPBUGS-59552: Referencing pod named ports within a service results in bad DNAT rules containing tcp/0 target port ovn-kubernetes/ovn-kubernetes#5583
downstream PR (4.21): OCPBUGS-61865, OCPBUGS-62636, OCPBUGS-59552: DownStream Merge [10-19-2025] #2817

Two conflicts:

Conflict 1

In downstream ovn-k makeNodeSwitchTargetIPs takes service *corev1.Service as its first input argument. This is a downstream hack for openshift.

$ git cherry-pick -x 06515935075a54a58191d32f95cc8911d7ab86fb 282b01ec669fa0311e46acbb22a48bc687a99a1d 651759c59efbf391956779d5b080654782659f9a 2fff3669b65ed60dc13b28e655ac93561029303e cd70830024d82f03ae1e9c3c8b9b2560a0869fd5
Auto-merging go-controller/pkg/node/gateway_shared_intf.go
Auto-merging go-controller/pkg/ovn/controller/services/lb_config.go
CONFLICT (content): Merge conflict in go-controller/pkg/ovn/controller/services/lb_config.go
Auto-merging go-controller/pkg/ovn/controller/services/lb_config_test.go
error: could not apply 065159350... Fix named port handling in externalTrafficPolicy Local services
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"


$ git diff
diff --cc go-controller/pkg/ovn/controller/services/lb_config.go
index b6bbd833b,c58b278e4..000000000
--- a/go-controller/pkg/ovn/controller/services/lb_config.go
+++ b/go-controller/pkg/ovn/controller/services/lb_config.go
@@@ -44,13 -43,7 +44,17 @@@ type lbConfig struct 
        hasNodePort bool
  }
  
++<<<<<<< HEAD
 +type lbEndpoints struct {
 +      Port  int32
 +      V4IPs []string
 +      V6IPs []string
 +}
 +
 +func makeNodeSwitchTargetIPs(service *corev1.Service, node string, c *lbConfig) (targetIPsV4, targetIPsV6 []string, v4Changed, v6Changed bool) {
++=======
+ func makeNodeSwitchTargetIPs(node string, c *lbConfig) (targetIPsV4, targetIPsV6 []string, v4Changed, v6Changed bool) {
++>>>>>>> 065159350 (Fix named port handling in externalTrafficPolicy Local services)
        targetIPsV4 = c.clusterEndpoints.V4IPs
        targetIPsV6 = c.clusterEndpoints.V6IPs

Conflict 2

Three functions in test/e2e/service.go needed to be added manually: WaitForServingAndReadyServiceEndpointsNum,
countServingAndReadyEndpointsSlicesNum, getServingAndReadyEndpointSliceAddresses.

$ git cherry-pick --continue
[OCPBUGS-64697test d6896892d] Fix named port handling in externalTrafficPolicy Local services
 Author: Andreas Karis <ak.karis@gmail.com>
 Date: Tue Sep 23 20:26:50 2025 +0200
 10 files changed, 742 insertions(+), 398 deletions(-)
Auto-merging go-controller/pkg/ovn/controller/services/lb_config_test.go
[OCPBUGS-64697test 8ed6bdd79] Extends unit test coverage for named port handling in ETP local
 Author: Andreas Karis <ak.karis@gmail.com>
 Date: Tue Sep 23 20:27:27 2025 +0200
 3 files changed, 438 insertions(+), 3 deletions(-)
Auto-merging test/e2e/service.go
Auto-merging test/e2e/util.go
[OCPBUGS-64697test df516cdd8] E2E service: Move checkNumberOf.. to util.go
 Author: Andreas Karis <ak.karis@gmail.com>
 Date: Fri Sep 26 17:51:49 2025 +0200
 2 files changed, 33 insertions(+), 24 deletions(-)
Auto-merging test/e2e/service.go
[OCPBUGS-64697test 64e1ed922] E2E service: Fix potential flake in conn to an ext IP using src port
 Author: Andreas Karis <ak.karis@gmail.com>
 Date: Fri Sep 26 11:27:32 2025 +0200
 1 file changed, 17 insertions(+), 6 deletions(-)
Auto-merging test/e2e/service.go
CONFLICT (content): Merge conflict in test/e2e/service.go
Auto-merging test/e2e/util.go
error: could not apply cd7083002... E2E service: Add test for named port handling with ETP=Local services

$ git diff
diff --cc test/e2e/service.go
index 0d734d05d,069dd6794..000000000
--- a/test/e2e/service.go
+++ b/test/e2e/service.go
[...]
++<<<<<<< HEAD
++=======
+ 
+ // WaitForServingAndReadyServiceEndpointsNum waits until there are EndpointSlices for serviceName
+ // containing a total of expectNum endpoints which are in both serving and ready state.
+ // (If the service is dual-stack, expectNum must count the endpoints of both IP families.)
+ func WaitForServingAndReadyServiceEndpointsNum(ctx context.Context, c clientset.Interface, namespace, serviceName string, expectNum int, interval, timeout time.Duration) error {
+       return wait.PollUntilContextTimeout(ctx, interval, timeout, false, func(ctx context.Context) (bool, error) {
+               framework.Logf("Waiting for amount of service:%s endpoints to be %d", serviceName, expectNum)
+               esList, err := c.DiscoveryV1().EndpointSlices(namespace).List(ctx, metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s", discoveryv1.LabelServiceName, serviceName)})
+               if err != nil {
+                       framework.Logf("Unexpected error trying to get EndpointSlices for %s : %v", serviceName, err)
+                       return false, nil
+               }
+ 
+               if len(esList.Items) == 0 {
+                       if expectNum == 0 {
+                               return true, nil
+                       }
+                       framework.Logf("Waiting for at least 1 EndpointSlice to exist")
+                       return false, nil
+               }
+ 
+               ready := countServingAndReadyEndpointsSlicesNum(esList)
+               if ready != expectNum {
+                       framework.Logf("Unexpected number of Serving And Ready Endpoints on Slices, got %d, expected %d", ready, expectNum)
+                       return false, nil
+               }
+               return true, nil
+       })
+ }
+ 
+ func countServingAndReadyEndpointsSlicesNum(epList *discoveryv1.EndpointSliceList) int {
+       // Only count unique addresses that are Ready and not Terminating
+       addresses := sets.New[string]()
+       for _, epSlice := range epList.Items {
+               addresses = addresses.Union(getServingAndReadyEndpointSliceAddresses(epSlice))
+       }
+       return addresses.Len()
+ }
+ 
+ func getServingAndReadyEndpointSliceAddresses(epSlice discoveryv1.EndpointSlice) sets.Set[string] {
+       addresses := sets.New[string]()
+       for _, ep := range epSlice.Endpoints {
+               cond := ep.Conditions
+               ready := cond.Ready == nil || *cond.Ready
+               serving := cond.Serving == nil || *cond.Serving
+               terminating := cond.Terminating != nil && *cond.Terminating
+               if !ready || !serving || terminating {
+                       continue
+               }
+               if len(ep.Addresses) == 0 || ep.Addresses[0] == "" {
+                       continue
+               }
+               addresses.Insert(ep.Addresses[0])
+       }
+       return addresses
+ }
++>>>>>>> cd7083002 (E2E service: Add test for named port handling with ETP=Local services)

Addresses incorrect DNAT rules with <proto>/0 target port when using services with externalTrafficPolicy: Local and named ports. The issue occurred when allocateLoadBalancerNodePorts was false and services referenced pod named ports. The previous implementation used svcPort.TargetPort.IntValue() which returns 0 for named ports, causing invalid DNAT rules. This refactoring introduces/uses structured endpoint types that properly handle port mapping from endpoint slices, ensuring the actual pod port numbers are used instead of attempting to convert named ports to integers. This change unifies endpoint processing logic by having both the services controller and nodePortWatcher use the same GetEndpointsForService function. This ensures consistent endpoint resolution and port mapping behavior across all service-related components, preventing divergence in logic and similar unnoticed port handling issues in the future. Signed-off-by: Andreas Karis <ak.karis@gmail.com> (cherry picked from commit 0651593)

Adds tests for loadBalancer services with named ports and AllocateLoadBalancerNodePorts=False. Add new test cases in Test_getEndpointsForService. Signed-off-by: Andreas Karis <ak.karis@gmail.com> (cherry picked from commit 282b01e)

Signed-off-by: Andreas Karis <ak.karis@gmail.com> (cherry picked from commit 651759c)

E2E test "Allow connection to an external IP using a source port that is equal to a node port" might flake if a service is already created with the same nodePort number. Give it a chance to recover by selecting a different port. Signed-off-by: Andreas Karis <ak.karis@gmail.com> (cherry picked from commit 2fff366)

The test validates LoadBalancer services with: - Named targetPorts (http/udp) instead of numeric ports - AllocateLoadBalancerNodePorts=false configuration - ExternalTrafficPolicy=Local behavior Signed-off-by: Andreas Karis <ak.karis@gmail.com> (cherry picked from commit cd70830)

openshift-ci-robot · 2025-11-05T18:49:08Z

@ricky-rav: This pull request references Jira Issue OCPBUGS-64697, which is invalid:

release note text must be set and not match the template OR release note type must be set to "Release Note Not Required". For more information you can reference the OpenShift Bug Process.
expected dependent Jira Issue OCPBUGS-59552 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but it is ON_QA instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Two conflicts:

Conflict 1

In downstream ovn-k makeNodeSwitchTargetIPs takes service *corev1.Service as its first input argument. This is a downstream hack for openshift.

$ git cherry-pick -x 06515935075a54a58191d32f95cc8911d7ab86fb 282b01ec669fa0311e46acbb22a48bc687a99a1d 651759c59efbf391956779d5b080654782659f9a 2fff3669b65ed60dc13b28e655ac93561029303e cd70830024d82f03ae1e9c3c8b9b2560a0869fd5
Auto-merging go-controller/pkg/node/gateway_shared_intf.go
Auto-merging go-controller/pkg/ovn/controller/services/lb_config.go
CONFLICT (content): Merge conflict in go-controller/pkg/ovn/controller/services/lb_config.go
Auto-merging go-controller/pkg/ovn/controller/services/lb_config_test.go
error: could not apply 065159350... Fix named port handling in externalTrafficPolicy Local services
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"


$ git diff
diff --cc go-controller/pkg/ovn/controller/services/lb_config.go
index b6bbd833b,c58b278e4..000000000
--- a/go-controller/pkg/ovn/controller/services/lb_config.go
+++ b/go-controller/pkg/ovn/controller/services/lb_config.go
@@@ -44,13 -43,7 +44,17 @@@ type lbConfig struct 
       hasNodePort bool
 }
 
++<<<<<<< HEAD
+type lbEndpoints struct {
+      Port  int32
+      V4IPs []string
+      V6IPs []string
+}
+
+func makeNodeSwitchTargetIPs(service *corev1.Service, node string, c *lbConfig) (targetIPsV4, targetIPsV6 []string, v4Changed, v6Changed bool) {
++=======
+ func makeNodeSwitchTargetIPs(node string, c *lbConfig) (targetIPsV4, targetIPsV6 []string, v4Changed, v6Changed bool) {
++>>>>>>> 065159350 (Fix named port handling in externalTrafficPolicy Local services)
       targetIPsV4 = c.clusterEndpoints.V4IPs
       targetIPsV6 = c.clusterEndpoints.V6IPs

Conflict 2

Three functions in test/e2e/service.go needed to be added manually: WaitForServingAndReadyServiceEndpointsNum,
countServingAndReadyEndpointsSlicesNum, getServingAndReadyEndpointSliceAddresses.

$ git cherry-pick --continue
[OCPBUGS-64697test d6896892d] Fix named port handling in externalTrafficPolicy Local services
Author: Andreas Karis <ak.karis@gmail.com>
Date: Tue Sep 23 20:26:50 2025 +0200
10 files changed, 742 insertions(+), 398 deletions(-)
Auto-merging go-controller/pkg/ovn/controller/services/lb_config_test.go
[OCPBUGS-64697test 8ed6bdd79] Extends unit test coverage for named port handling in ETP local
Author: Andreas Karis <ak.karis@gmail.com>
Date: Tue Sep 23 20:27:27 2025 +0200
3 files changed, 438 insertions(+), 3 deletions(-)
Auto-merging test/e2e/service.go
Auto-merging test/e2e/util.go
[OCPBUGS-64697test df516cdd8] E2E service: Move checkNumberOf.. to util.go
Author: Andreas Karis <ak.karis@gmail.com>
Date: Fri Sep 26 17:51:49 2025 +0200
2 files changed, 33 insertions(+), 24 deletions(-)
Auto-merging test/e2e/service.go
[OCPBUGS-64697test 64e1ed922] E2E service: Fix potential flake in conn to an ext IP using src port
Author: Andreas Karis <ak.karis@gmail.com>
Date: Fri Sep 26 11:27:32 2025 +0200
1 file changed, 17 insertions(+), 6 deletions(-)
Auto-merging test/e2e/service.go
CONFLICT (content): Merge conflict in test/e2e/service.go
Auto-merging test/e2e/util.go
error: could not apply cd7083002... E2E service: Add test for named port handling with ETP=Local services

$ git diff
diff --cc test/e2e/service.go
index 0d734d05d,069dd6794..000000000
--- a/test/e2e/service.go
+++ b/test/e2e/service.go
[...]
++<<<<<<< HEAD
++=======
+ 
+ // WaitForServingAndReadyServiceEndpointsNum waits until there are EndpointSlices for serviceName
+ // containing a total of expectNum endpoints which are in both serving and ready state.
+ // (If the service is dual-stack, expectNum must count the endpoints of both IP families.)
+ func WaitForServingAndReadyServiceEndpointsNum(ctx context.Context, c clientset.Interface, namespace, serviceName string, expectNum int, interval, timeout time.Duration) error {
+       return wait.PollUntilContextTimeout(ctx, interval, timeout, false, func(ctx context.Context) (bool, error) {
+               framework.Logf("Waiting for amount of service:%s endpoints to be %d", serviceName, expectNum)
+               esList, err := c.DiscoveryV1().EndpointSlices(namespace).List(ctx, metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s", discoveryv1.LabelServiceName, serviceName)})
+               if err != nil {
+                       framework.Logf("Unexpected error trying to get EndpointSlices for %s : %v", serviceName, err)
+                       return false, nil
+               }
+ 
+               if len(esList.Items) == 0 {
+                       if expectNum == 0 {
+                               return true, nil
+                       }
+                       framework.Logf("Waiting for at least 1 EndpointSlice to exist")
+                       return false, nil
+               }
+ 
+               ready := countServingAndReadyEndpointsSlicesNum(esList)
+               if ready != expectNum {
+                       framework.Logf("Unexpected number of Serving And Ready Endpoints on Slices, got %d, expected %d", ready, expectNum)
+                       return false, nil
+               }
+               return true, nil
+       })
+ }
+ 
+ func countServingAndReadyEndpointsSlicesNum(epList *discoveryv1.EndpointSliceList) int {
+       // Only count unique addresses that are Ready and not Terminating
+       addresses := sets.New[string]()
+       for _, epSlice := range epList.Items {
+               addresses = addresses.Union(getServingAndReadyEndpointSliceAddresses(epSlice))
+       }
+       return addresses.Len()
+ }
+ 
+ func getServingAndReadyEndpointSliceAddresses(epSlice discoveryv1.EndpointSlice) sets.Set[string] {
+       addresses := sets.New[string]()
+       for _, ep := range epSlice.Endpoints {
+               cond := ep.Conditions
+               ready := cond.Ready == nil || *cond.Ready
+               serving := cond.Serving == nil || *cond.Serving
+               terminating := cond.Terminating != nil && *cond.Terminating
+               if !ready || !serving || terminating {
+                       continue
+               }
+               if len(ep.Addresses) == 0 || ep.Addresses[0] == "" {
+                       continue
+               }
+               addresses.Insert(ep.Addresses[0])
+       }
+       return addresses
+ }
++>>>>>>> cd7083002 (E2E service: Add test for named port handling with ETP=Local services)

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.

ricky-rav · 2025-11-06T08:46:30Z

/jira refresh

openshift-ci-robot · 2025-11-06T08:46:38Z

@ricky-rav: This pull request references Jira Issue OCPBUGS-64697, which is valid. The bug has been moved to the POST state.

7 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.20.z) matches configured target version for branch (4.20.z)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)
release note text is set and does not match the template
dependent bug Jira Issue OCPBUGS-59552 is in the state Verified, which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA))
dependent Jira Issue OCPBUGS-59552 targets the "4.21.0" version, which is one of the valid target versions: 4.21.0
bug has dependents

Requesting review from QA contact:
/cc @Meina-rh

Details

In response to this:

/jira refresh

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.

ricky-rav · 2025-11-06T08:46:51Z

/retest-required

ricky-rav · 2025-11-06T14:20:30Z

/retest-required

Meina-rh · 2025-11-07T01:22:10Z

/retest-required

Meina-rh · 2025-11-07T01:23:07Z

/verified by @Meina-rh

openshift-ci-robot · 2025-11-07T01:23:18Z

@Meina-rh: This PR has been marked as verified by @Meina-rh.

Details

In response to this:

/verified by @Meina-rh

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.

ricky-rav · 2025-11-07T08:01:03Z

/retest-required

ricky-rav · 2025-11-07T15:40:30Z

/retest-required

ricky-rav · 2025-11-07T23:20:58Z

/payload 4.20 ci blocking
/payload 4.20 nightly blocking

openshift-ci · 2025-11-07T23:21:03Z

@ricky-rav: trigger 5 job(s) of type blocking for the ci release of OCP 4.20

periodic-ci-openshift-release-master-ci-4.20-upgrade-from-stable-4.19-e2e-aws-ovn-upgrade
periodic-ci-openshift-release-master-ci-4.20-upgrade-from-stable-4.19-e2e-azure-ovn-upgrade
periodic-ci-openshift-release-master-ci-4.20-e2e-gcp-ovn-upgrade
periodic-ci-openshift-hypershift-release-4.20-periodics-e2e-aks
periodic-ci-openshift-hypershift-release-4.20-periodics-e2e-aws-ovn

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6bacbc60-bc30-11f0-9dfe-136ed4410777-0

trigger 13 job(s) of type blocking for the nightly release of OCP 4.20

periodic-ci-openshift-release-master-nightly-4.20-e2e-aws-ovn-serial-1of2
periodic-ci-openshift-release-master-nightly-4.20-e2e-aws-ovn-serial-2of2
periodic-ci-openshift-release-master-ci-4.20-e2e-aws-upgrade-ovn-single-node
periodic-ci-openshift-release-master-ci-4.20-e2e-aws-ovn-techpreview
periodic-ci-openshift-release-master-ci-4.20-e2e-aws-ovn-techpreview-serial-1of3
periodic-ci-openshift-release-master-ci-4.20-e2e-aws-ovn-techpreview-serial-2of3
periodic-ci-openshift-release-master-ci-4.20-e2e-aws-ovn-techpreview-serial-3of3
periodic-ci-openshift-release-master-nightly-4.20-e2e-aws-ovn-upgrade-fips
periodic-ci-openshift-release-master-ci-4.20-e2e-azure-ovn-upgrade
periodic-ci-openshift-release-master-ci-4.20-upgrade-from-stable-4.19-e2e-gcp-ovn-rt-upgrade
periodic-ci-openshift-hypershift-release-4.20-periodics-e2e-aws-ovn-conformance
periodic-ci-openshift-release-master-nightly-4.20-e2e-metal-ipi-ovn-bm
periodic-ci-openshift-release-master-nightly-4.20-e2e-metal-ipi-ovn-ipv6

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6bacbc60-bc30-11f0-9dfe-136ed4410777-1

jluhrsen · 2025-11-10T21:47:57Z

/payload-job periodic-ci-openshift-release-master-ci-4.20-e2e-azure-ovn-upgrade
/payload-job periodic-ci-openshift-hypershift-release-4.20-periodics-e2e-aws-ovn-conformance

openshift-ci · 2025-11-10T21:48:03Z

@jluhrsen: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-ci-4.20-e2e-azure-ovn-upgrade
periodic-ci-openshift-hypershift-release-4.20-periodics-e2e-aws-ovn-conformance

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/ec37c500-be7e-11f0-9d6a-220802a21fa5-0

ricky-rav · 2025-11-12T08:26:07Z

/payload-job periodic-ci-openshift-release-master-ci-4.20-e2e-gcp-ovn-upgrade
/payload-job periodic-ci-openshift-hypershift-release-4.20-periodics-e2e-aks

openshift-ci · 2025-11-12T08:26:13Z

@ricky-rav: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-ci-4.20-e2e-gcp-ovn-upgrade
periodic-ci-openshift-hypershift-release-4.20-periodics-e2e-aks

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/3d10dac0-bfa1-11f0-84b9-2440655d8703-0

ricky-rav · 2025-11-13T13:52:15Z

/assign @jcaamano

jcaamano · 2025-11-13T14:44:11Z

/lgtm
/approve
/label backport-risk-assessed
/override ci/prow/lint

openshift-ci · 2025-11-13T14:47:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcaamano, ricky-rav

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [jcaamano]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2025-11-13T14:48:50Z

@jcaamano: Overrode contexts on behalf of jcaamano: ci/prow/lint

Details

In response to this:

/lgtm
/approve
/label backport-risk-assessed
/override ci/prow/lint

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.

openshift-ci-robot · 2025-11-13T20:00:18Z

/retest-required

Remaining retests: 0 against base HEAD 27e34ff and 2 for PR HEAD a3a4827 in total

ricky-rav · 2025-11-14T08:54:15Z

/retest-required

openshift-ci · 2025-11-14T11:42:15Z

@ricky-rav: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/security	`a3a4827`	link	false	`/test security`

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

ricky-rav · 2025-11-14T11:44:02Z

/retest-required

openshift-ci-robot · 2025-11-14T16:16:17Z

@ricky-rav: Jira Issue Verification Checks: Jira Issue OCPBUGS-64697
✔️ This pull request was pre-merge verified.
✔️ All associated pull requests have merged.
✔️ All associated, merged pull requests were pre-merge verified.

Jira Issue OCPBUGS-64697 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓

Details

In response to this:

upstream PR: OCPBUGS-59552: Referencing pod named ports within a service results in bad DNAT rules containing tcp/0 target port ovn-kubernetes/ovn-kubernetes#5583
downstream PR (4.21): OCPBUGS-61865, OCPBUGS-62636, OCPBUGS-59552: DownStream Merge [10-19-2025] #2817

Two conflicts:

Conflict 1

In downstream ovn-k makeNodeSwitchTargetIPs takes service *corev1.Service as its first input argument. This is a downstream hack for openshift.

$ git cherry-pick -x 06515935075a54a58191d32f95cc8911d7ab86fb 282b01ec669fa0311e46acbb22a48bc687a99a1d 651759c59efbf391956779d5b080654782659f9a 2fff3669b65ed60dc13b28e655ac93561029303e cd70830024d82f03ae1e9c3c8b9b2560a0869fd5
Auto-merging go-controller/pkg/node/gateway_shared_intf.go
Auto-merging go-controller/pkg/ovn/controller/services/lb_config.go
CONFLICT (content): Merge conflict in go-controller/pkg/ovn/controller/services/lb_config.go
Auto-merging go-controller/pkg/ovn/controller/services/lb_config_test.go
error: could not apply 065159350... Fix named port handling in externalTrafficPolicy Local services
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"


$ git diff
diff --cc go-controller/pkg/ovn/controller/services/lb_config.go
index b6bbd833b,c58b278e4..000000000
--- a/go-controller/pkg/ovn/controller/services/lb_config.go
+++ b/go-controller/pkg/ovn/controller/services/lb_config.go
@@@ -44,13 -43,7 +44,17 @@@ type lbConfig struct 
       hasNodePort bool
 }
 
++<<<<<<< HEAD
+type lbEndpoints struct {
+      Port  int32
+      V4IPs []string
+      V6IPs []string
+}
+
+func makeNodeSwitchTargetIPs(service *corev1.Service, node string, c *lbConfig) (targetIPsV4, targetIPsV6 []string, v4Changed, v6Changed bool) {
++=======
+ func makeNodeSwitchTargetIPs(node string, c *lbConfig) (targetIPsV4, targetIPsV6 []string, v4Changed, v6Changed bool) {
++>>>>>>> 065159350 (Fix named port handling in externalTrafficPolicy Local services)
       targetIPsV4 = c.clusterEndpoints.V4IPs
       targetIPsV6 = c.clusterEndpoints.V6IPs

Conflict 2

Three functions in test/e2e/service.go needed to be added manually: WaitForServingAndReadyServiceEndpointsNum,
countServingAndReadyEndpointsSlicesNum, getServingAndReadyEndpointSliceAddresses.

$ git cherry-pick --continue
[OCPBUGS-64697test d6896892d] Fix named port handling in externalTrafficPolicy Local services
Author: Andreas Karis <ak.karis@gmail.com>
Date: Tue Sep 23 20:26:50 2025 +0200
10 files changed, 742 insertions(+), 398 deletions(-)
Auto-merging go-controller/pkg/ovn/controller/services/lb_config_test.go
[OCPBUGS-64697test 8ed6bdd79] Extends unit test coverage for named port handling in ETP local
Author: Andreas Karis <ak.karis@gmail.com>
Date: Tue Sep 23 20:27:27 2025 +0200
3 files changed, 438 insertions(+), 3 deletions(-)
Auto-merging test/e2e/service.go
Auto-merging test/e2e/util.go
[OCPBUGS-64697test df516cdd8] E2E service: Move checkNumberOf.. to util.go
Author: Andreas Karis <ak.karis@gmail.com>
Date: Fri Sep 26 17:51:49 2025 +0200
2 files changed, 33 insertions(+), 24 deletions(-)
Auto-merging test/e2e/service.go
[OCPBUGS-64697test 64e1ed922] E2E service: Fix potential flake in conn to an ext IP using src port
Author: Andreas Karis <ak.karis@gmail.com>
Date: Fri Sep 26 11:27:32 2025 +0200
1 file changed, 17 insertions(+), 6 deletions(-)
Auto-merging test/e2e/service.go
CONFLICT (content): Merge conflict in test/e2e/service.go
Auto-merging test/e2e/util.go
error: could not apply cd7083002... E2E service: Add test for named port handling with ETP=Local services

$ git diff
diff --cc test/e2e/service.go
index 0d734d05d,069dd6794..000000000
--- a/test/e2e/service.go
+++ b/test/e2e/service.go
[...]
++<<<<<<< HEAD
++=======
+ 
+ // WaitForServingAndReadyServiceEndpointsNum waits until there are EndpointSlices for serviceName
+ // containing a total of expectNum endpoints which are in both serving and ready state.
+ // (If the service is dual-stack, expectNum must count the endpoints of both IP families.)
+ func WaitForServingAndReadyServiceEndpointsNum(ctx context.Context, c clientset.Interface, namespace, serviceName string, expectNum int, interval, timeout time.Duration) error {
+       return wait.PollUntilContextTimeout(ctx, interval, timeout, false, func(ctx context.Context) (bool, error) {
+               framework.Logf("Waiting for amount of service:%s endpoints to be %d", serviceName, expectNum)
+               esList, err := c.DiscoveryV1().EndpointSlices(namespace).List(ctx, metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s", discoveryv1.LabelServiceName, serviceName)})
+               if err != nil {
+                       framework.Logf("Unexpected error trying to get EndpointSlices for %s : %v", serviceName, err)
+                       return false, nil
+               }
+ 
+               if len(esList.Items) == 0 {
+                       if expectNum == 0 {
+                               return true, nil
+                       }
+                       framework.Logf("Waiting for at least 1 EndpointSlice to exist")
+                       return false, nil
+               }
+ 
+               ready := countServingAndReadyEndpointsSlicesNum(esList)
+               if ready != expectNum {
+                       framework.Logf("Unexpected number of Serving And Ready Endpoints on Slices, got %d, expected %d", ready, expectNum)
+                       return false, nil
+               }
+               return true, nil
+       })
+ }
+ 
+ func countServingAndReadyEndpointsSlicesNum(epList *discoveryv1.EndpointSliceList) int {
+       // Only count unique addresses that are Ready and not Terminating
+       addresses := sets.New[string]()
+       for _, epSlice := range epList.Items {
+               addresses = addresses.Union(getServingAndReadyEndpointSliceAddresses(epSlice))
+       }
+       return addresses.Len()
+ }
+ 
+ func getServingAndReadyEndpointSliceAddresses(epSlice discoveryv1.EndpointSlice) sets.Set[string] {
+       addresses := sets.New[string]()
+       for _, ep := range epSlice.Endpoints {
+               cond := ep.Conditions
+               ready := cond.Ready == nil || *cond.Ready
+               serving := cond.Serving == nil || *cond.Serving
+               terminating := cond.Terminating != nil && *cond.Terminating
+               if !ready || !serving || terminating {
+                       continue
+               }
+               if len(ep.Addresses) == 0 || ep.Addresses[0] == "" {
+                       continue
+               }
+               addresses.Insert(ep.Addresses[0])
+       }
+       return addresses
+ }
++>>>>>>> cd7083002 (E2E service: Add test for named port handling with ETP=Local services)

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.

openshift-merge-robot · 2025-11-16T02:32:44Z

Fix included in accepted release 4.20.0-0.nightly-2025-11-15-175712

Relax linter CI job by add missing bits of cherry-pick conflicts introduced by openshift#2844 Signed-off-by: Or Mergi <ormergi@redhat.com>

Relax linter CI job by adding the missing bits of cherry-pick conflicts introduced by openshift#2844 Signed-off-by: Or Mergi <ormergi@redhat.com>

OCPBUGS-65951: [release-4.20]: Fix linter issues, add missing cheryy-pick bits of #2844

andreaskaris added 5 commits November 5, 2025 19:32

E2E service: Move checkNumberOf.. to util.go

87b946b

Signed-off-by: Andreas Karis <ak.karis@gmail.com> (cherry picked from commit 651759c)

openshift-ci bot requested review from jcaamano and martinkennelly November 5, 2025 18:49

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Nov 6, 2025

openshift-ci bot requested a review from Meina-rh November 6, 2025 08:46

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Nov 7, 2025

openshift-ci bot assigned jcaamano Nov 13, 2025

openshift-ci bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Nov 13, 2025

openshift-ci bot assigned anuragthehatter, asood-rh, barbora137, huiran0826, jechen0648, Meina-rh, mffiedler, qiowang721 and rbbratta Nov 13, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 13, 2025

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 13, 2025

openshift-merge-bot bot merged commit 798404b into openshift:release-4.20 Nov 14, 2025
29 of 30 checks passed

ormergi added a commit to ormergi/ocp-ovn-kubernetes that referenced this pull request Nov 19, 2025

Fix linter issues

ef663d4

Relax linter CI job by add missing bits of cherry-pick conflicts introduced by openshift#2844 Signed-off-by: Or Mergi <ormergi@redhat.com>

ormergi mentioned this pull request Nov 19, 2025

OCPBUGS-65951: [release-4.20]: Fix linter issues, add missing cheryy-pick bits of #2844 #2866

Merged

5 tasks

ricky-rav mentioned this pull request Nov 19, 2025

NO-JIRA: [release-4.20] CARRY: Fix service LB unit tests for OCP hack #2867

Closed

ormergi added a commit to ormergi/ocp-ovn-kubernetes that referenced this pull request Nov 20, 2025

CARRY: Fix linter issues

85fa1bb

Relax linter CI job by adding the missing bits of cherry-pick conflicts introduced by openshift#2844 Signed-off-by: Or Mergi <ormergi@redhat.com>

openshift-merge-bot bot added a commit that referenced this pull request Nov 25, 2025

Merge pull request #2866 from ormergi/fix-cp-conflict-pr2844

ecb7aac

OCPBUGS-65951: [release-4.20]: Fix linter issues, add missing cheryy-pick bits of #2844

Conversation

ricky-rav commented Nov 5, 2025

Conflict 1

Conflict 2

Uh oh!

openshift-ci-robot commented Nov 5, 2025

Conflict 1

Conflict 2

Uh oh!

ricky-rav commented Nov 6, 2025

Uh oh!

openshift-ci-robot commented Nov 6, 2025

Uh oh!

ricky-rav commented Nov 6, 2025

Uh oh!

ricky-rav commented Nov 6, 2025

Uh oh!

Meina-rh commented Nov 7, 2025

Uh oh!

Meina-rh commented Nov 7, 2025

Uh oh!

openshift-ci-robot commented Nov 7, 2025

Uh oh!

ricky-rav commented Nov 7, 2025

Uh oh!

ricky-rav commented Nov 7, 2025

Uh oh!

ricky-rav commented Nov 7, 2025

Uh oh!

openshift-ci bot commented Nov 7, 2025

Uh oh!

jluhrsen commented Nov 10, 2025

Uh oh!

openshift-ci bot commented Nov 10, 2025

Uh oh!

ricky-rav commented Nov 12, 2025

Uh oh!

openshift-ci bot commented Nov 12, 2025

Uh oh!

ricky-rav commented Nov 13, 2025

Uh oh!

jcaamano commented Nov 13, 2025

Uh oh!

openshift-ci bot commented Nov 13, 2025

Uh oh!

openshift-ci bot commented Nov 13, 2025

Uh oh!

openshift-ci-robot commented Nov 13, 2025

Uh oh!

ricky-rav commented Nov 14, 2025

Uh oh!

openshift-ci bot commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ricky-rav commented Nov 14, 2025

Uh oh!

Uh oh!

openshift-ci-robot commented Nov 14, 2025

Conflict 1

Conflict 2

Uh oh!

openshift-merge-robot commented Nov 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

Comments

openshift-ci bot commented Nov 14, 2025 •

edited

Loading