wy471x opened a new pull request, #7287:
URL: https://github.com/apache/shenyu/pull/7287

   Fixes #6493
   
   Make sure that:
   
   - [X] You have read the [contribution 
guidelines](https://shenyu.apache.org/community/contributor-guide).
   - [X] You submit test cases (unit or integration tests) that back your 
changes.
   - [X] Your local test passed `./mvnw clean install 
-Dmaven.javadoc.skip=true`.
   
   ## Summary
   
   `EndpointsReconciler` rebuilt a single upstream handle per service from the 
first TCP endpoint port:
   
   ```java
   CoreV1EndpointPort endpointPort = ports.stream()
           .filter(coreV1EndpointPort -> 
"TCP".equals(coreV1EndpointPort.getProtocol()))
           .findFirst()
           .orElseThrow(...);
   ```
   
   and applied that handle to every divide/websocket selector of every ingress 
that references the service, because `ServiceIngressCache` only stored 
`(ingressNamespace, ingressName)` and dropped the backend service port selected 
by each ingress. For a multi-port service, an endpoint update therefore 
silently rewrote selectors to the port of the first endpoint port, and a subset 
without a TCP port aborted the reconcile with a `ShenyuException`.
   
   ### Changes:
   
   1. `ServiceIngressRelation` (new, `org.apache.shenyu.k8s.common`) — value 
type of a service/ingress relation that also carries the `IngressBackendPort` 
selected by the ingress backend, with `isSameIngress(namespace, name)` used for 
cache lookups.
   2. `IngressBackendPort` (new, `org.apache.shenyu.k8s.common`) — value type 
of the service port selected by an ingress backend, built with 
`from(V1ServiceBackendPort)` from either the port name or the port number. 
`selectEndpointPort(ports, backendPort)` picks the endpoint port that serves it 
(matched by number or by name) and falls back to the first TCP port of the 
subset, because a service may map the selected port to a different 
`targetPort`; it returns `null` instead of throwing when a subset exposes no 
TCP port.
   3. `ServiceIngressCache` (ServiceIngressCache.java:66) — stores 
`ServiceIngressRelation` values per service and `getIngressName()` returns the 
relations of the service. `putIngressName()` replaces the previous relation of 
the same ingress, so a changed backend port does not leave a stale relation 
that would still rewrite the selectors of that ingress.
   4. `IngressReconciler.parseServiceFromIngress()` 
(IngressReconciler.java:390) — returns the backend service names mapped to the 
service port the ingress selects instead of `Pair<namespace, serviceName>`; the 
relation with the port is stored in `ServiceIngressCache` on reconcile and 
removed on delete.
   5. `IngressReconciler.updateUpstreamFromEndpoints()` 
(IngressReconciler.java:589) — resolves the endpoints addresses of the port 
selected by the ingress instead of the first TCP port.
   6. `EndpointsReconciler.reconcile()` / `updateSelectors()` 
(EndpointsReconciler.java:115) — builds one handle per ingress relation, from 
the endpoint addresses of the port that ingress selected, and applies it only 
to the selectors of that ingress.
   
   ### Test Cases:
   
   `EndpointsReconcilerTest`:
   
   - `testUpdateMultiPortSelectors` (new) — a service exposing the TCP endpoint 
ports 8001 and 8002 with two ingresses that select 8001 and 8002; each selector 
keeps its own port and does not receive the port of the other ingress.
   - `testUpdateSelectorWithNamedBackendPort` (new) — an ingress that selects 
the service port by name resolves to the endpoint port with that name.
   - `testUpdateSelectorWithUnmatchedBackendPort` (new) — an ingress whose 
selected service port is not exposed by the endpoints (service port mapped to a 
different target port) falls back to the first TCP port instead of failing.
   - `testUpdateWebSocketSelector` — existing test, updated to the new cache 
API, still asserts the websocket handle is built from the endpoint address.
   
   With the port matching disabled (i.e. the previous first-TCP-port 
behaviour), `testUpdateMultiPortSelectors` and 
`testUpdateSelectorWithNamedBackendPort` fail with `Expected: a string 
containing "127.0.0.1:8002" but: was "...127.0.0.1:8001..."`, which is the 
misrouting reported in the issue.
   
   ## Verification
   
   - `mvn -pl shenyu-kubernetes-controller test` — 23 tests, 0 failures.
   - `mvn -pl shenyu-kubernetes-controller validate` — checkstyle passed.
   
   close #6493


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to