This is an automated email from the ASF dual-hosted git repository.

Aias00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git


The following commit(s) were added to refs/heads/master by this push:
     new 04a3286e48 fix(k8s): make ServiceIngressCache iteration race-free and 
pin the multi-port limitation (#7392)
04a3286e48 is described below

commit 04a3286e48a19f2b40d6c61a356239de6d54247c
Author: wy471x <[email protected]>
AuthorDate: Thu Oct 8 15:35:27 2026 +0800

    fix(k8s): make ServiceIngressCache iteration race-free and pin the 
multi-port limitation (#7392)
    
    * fix: keep the service port selected by an ingress on endpoint updates 
(#6493)
    
    EndpointsReconciler rebuilt one upstream handle per service, taken from the
    first TCP port of every endpoint subset, and applied it to all divide and
    websocket selectors of the ingresses that reference the service. The service
    to ingress cache only stored the ingress names, so a service that exposes
    several TCP ports, or several ingresses that select different ports of the
    same service, ended up routing to the port of the first endpoint port. The
    build failed with a ShenyuException as well when a subset did not expose a
    TCP port.
    
    - ServiceIngressCache: store ServiceIngressRelation values, which keep the
      service port each ingress selected. A relation of the same ingress is
      replaced on every put so a changed backend port does not leave a stale
      relation behind.
    - IngressBackendPort: new value type of the service port selected by an
      ingress backend (port name or number), it matches the endpoint port that
      serves it and falls back to the first TCP port of the subset, because a
      service may map the selected port to a different target port.
    - IngressReconciler: parseServiceFromIngress() now returns the backend
      service names mapped to the selected service port, the relation is stored
      in the cache and the upstream refresh matches the endpoint port of the
      ingress instead of the first one.
    - EndpointsReconciler: build the upstream handle per ingress relation with
      the endpoint port selected by that ingress, so updating endpoints no 
longer
      rewrites selectors to the port of another ingress.
    
    * fix(k8s): hand out copy-on-write relations from ServiceIngressCache
    
    getIngressName() returned the list instance stored in the cache, while
    putIngressName() (removeIf + add inside compute) and
    removeSpecifiedIngressName() mutate that same instance. Both controllers
    are built with withWorkerCount(2), so EndpointsReconciler iterating the
    returned list races with IngressReconciler updating the cache and fails
    with ConcurrentModificationException - or with NoSuchElementException when
    a removal shrinks the list - which aborts the endpoints reconcile.
    
    Store the relations of a service in a CopyOnWriteArrayList so a reader
    iterates a snapshot while another reconcile updates the cache.
    
    * docs(k8s): pin the single ingress multi-port limitation
    
    parseServiceFromIngress() keys its result by service name, so an ingress
    that routes several paths to the same service with different service ports
    keeps only the port of the first path that references the service, and an
    endpoint update applies that upstream handle to every selector of the
    ingress. State the limitation in the javadoc and add a test that pins the
    current behaviour.
---
 .../shenyu/k8s/cache/ServiceIngressCache.java      |   4 +-
 .../shenyu/k8s/reconciler/IngressReconciler.java   |   5 +
 .../k8s/IngressReconcilerMultiPortPathsTest.java   | 133 +++++++++++++++++++++
 3 files changed, 140 insertions(+), 2 deletions(-)

diff --git 
a/shenyu-kubernetes-controller/src/main/java/org/apache/shenyu/k8s/cache/ServiceIngressCache.java
 
b/shenyu-kubernetes-controller/src/main/java/org/apache/shenyu/k8s/cache/ServiceIngressCache.java
index 5722ae0935..52a652b7ed 100644
--- 
a/shenyu-kubernetes-controller/src/main/java/org/apache/shenyu/k8s/cache/ServiceIngressCache.java
+++ 
b/shenyu-kubernetes-controller/src/main/java/org/apache/shenyu/k8s/cache/ServiceIngressCache.java
@@ -20,11 +20,11 @@ package org.apache.shenyu.k8s.cache;
 import com.google.common.collect.Maps;
 import org.apache.shenyu.k8s.common.ServiceIngressRelation;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
  * The cache for mapping service name to the ingress relations that reference 
the service.
@@ -69,7 +69,7 @@ public final class ServiceIngressCache {
      */
     public void putIngressName(final String namespace, final String 
serviceName, final ServiceIngressRelation relation) {
         INGRESS_MAP.compute(getKey(namespace, serviceName), (key, relations) 
-> {
-            List<ServiceIngressRelation> res = Objects.isNull(relations) ? new 
ArrayList<>() : relations;
+            List<ServiceIngressRelation> res = Objects.isNull(relations) ? new 
CopyOnWriteArrayList<>() : relations;
             res.removeIf(item -> 
item.isSameIngress(relation.getIngressNamespace(), relation.getIngressName()));
             res.add(relation);
             return res;
diff --git 
a/shenyu-kubernetes-controller/src/main/java/org/apache/shenyu/k8s/reconciler/IngressReconciler.java
 
b/shenyu-kubernetes-controller/src/main/java/org/apache/shenyu/k8s/reconciler/IngressReconciler.java
index dec42bec88..7d2a91bec8 100644
--- 
a/shenyu-kubernetes-controller/src/main/java/org/apache/shenyu/k8s/reconciler/IngressReconciler.java
+++ 
b/shenyu-kubernetes-controller/src/main/java/org/apache/shenyu/k8s/reconciler/IngressReconciler.java
@@ -392,6 +392,11 @@ public class IngressReconciler implements Reconciler {
     /**
      * Parse the backend services referenced by the ingress, mapped to the 
service port selected by the ingress.
      *
+     * <p>The result is keyed by service name, so when an ingress routes 
several paths to the same service
+     * with different service ports, only the port of the first path that 
references the service is kept.
+     * The relation cached for the ingress is therefore per service rather 
than per path, and an endpoint
+     * update rebuilds the upstream handle of that single port for every 
selector of the ingress.
+     *
      * @param ingress ingress resource
      * @return the backend service names mapped to the service port selected 
by the ingress
      */
diff --git 
a/shenyu-kubernetes-controller/src/test/java/org/apache/shenyu/k8s/IngressReconcilerMultiPortPathsTest.java
 
b/shenyu-kubernetes-controller/src/test/java/org/apache/shenyu/k8s/IngressReconcilerMultiPortPathsTest.java
new file mode 100644
index 0000000000..81b85fcb67
--- /dev/null
+++ 
b/shenyu-kubernetes-controller/src/test/java/org/apache/shenyu/k8s/IngressReconcilerMultiPortPathsTest.java
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shenyu.k8s;
+
+import io.kubernetes.client.extended.controller.reconciler.Request;
+import io.kubernetes.client.extended.controller.reconciler.Result;
+import io.kubernetes.client.informer.SharedIndexInformer;
+import io.kubernetes.client.informer.cache.Indexer;
+import io.kubernetes.client.openapi.ApiClient;
+import io.kubernetes.client.openapi.models.CoreV1EndpointPort;
+import io.kubernetes.client.openapi.models.V1EndpointAddress;
+import io.kubernetes.client.openapi.models.V1EndpointSubsetBuilder;
+import io.kubernetes.client.openapi.models.V1Endpoints;
+import io.kubernetes.client.openapi.models.V1EndpointsBuilder;
+import io.kubernetes.client.openapi.models.V1HTTPIngressPath;
+import io.kubernetes.client.openapi.models.V1HTTPIngressPathBuilder;
+import io.kubernetes.client.openapi.models.V1Ingress;
+import io.kubernetes.client.openapi.models.V1IngressBuilder;
+import io.kubernetes.client.openapi.models.V1IngressRule;
+import io.kubernetes.client.openapi.models.V1IngressRuleBuilder;
+import io.kubernetes.client.openapi.models.V1Secret;
+import io.kubernetes.client.openapi.models.V1Service;
+import org.apache.shenyu.common.config.ssl.ShenyuSniAsyncMapping;
+import org.apache.shenyu.common.enums.PluginEnum;
+import org.apache.shenyu.k8s.cache.IngressSelectorCache;
+import org.apache.shenyu.k8s.cache.ServiceIngressCache;
+import org.apache.shenyu.k8s.common.ServiceIngressRelation;
+import org.apache.shenyu.k8s.parser.IngressParser;
+import org.apache.shenyu.k8s.reconciler.IngressReconciler;
+import org.apache.shenyu.k8s.repository.ShenyuCacheRepository;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Test for an ingress that routes several paths to the same service while the 
paths select
+ * different service ports.
+ */
+public final class IngressReconcilerMultiPortPathsTest {
+
+    private static final String NAMESPACE = "multi-port-paths-ns";
+
+    private static final String INGRESS_NAME = "multi-port-paths-ingress";
+
+    private static final String SERVICE_NAME = "multi-port-paths-service";
+
+    private static final int FIRST_PORT = 8001;
+
+    private static final int SECOND_PORT = 8002;
+
+    private IngressReconciler ingressReconciler;
+
+    @BeforeEach
+    public void init() {
+        final SharedIndexInformer<V1Ingress> ingressInformer = 
mock(SharedIndexInformer.class);
+        final SharedIndexInformer<V1Secret> secretInformer = 
mock(SharedIndexInformer.class);
+        final SharedIndexInformer<V1Service> serviceInformer = 
mock(SharedIndexInformer.class);
+        final SharedIndexInformer<V1Endpoints> endpointsInformer = 
mock(SharedIndexInformer.class);
+        final ShenyuCacheRepository shenyuCacheRepository = 
mock(ShenyuCacheRepository.class);
+
+        Indexer<V1Ingress> ingressIndexer = mock(Indexer.class);
+        V1IngressRule mockedRule = new 
V1IngressRuleBuilder().withNewHttp().withPaths(
+                        path("/first-api", FIRST_PORT),
+                        path("/second-api", SECOND_PORT))
+                .endHttp().build();
+        V1Ingress mockedIngress = new 
V1IngressBuilder().withNewMetadata().withName(INGRESS_NAME).withNamespace(NAMESPACE).endMetadata()
+                
.withNewSpec().withIngressClassName("shenyu").withRules(mockedRule).endSpec()
+                .withKind("Ingress").build();
+        when(ingressIndexer.getByKey(NAMESPACE + "/" + 
INGRESS_NAME)).thenReturn(mockedIngress);
+        when(ingressInformer.getIndexer()).thenReturn(ingressIndexer);
+
+        Indexer<V1Endpoints> endpointsIndexer = mock(Indexer.class);
+        V1Endpoints mockedEndpoints = new 
V1EndpointsBuilder().withKind("Endpoints")
+                
.withNewMetadata().withNamespace(NAMESPACE).withName(SERVICE_NAME).endMetadata()
+                .withSubsets(new V1EndpointSubsetBuilder()
+                        .withAddresses(new V1EndpointAddress().ip("127.0.0.1"))
+                        .withPorts(new 
CoreV1EndpointPort().port(FIRST_PORT).protocol("TCP"),
+                                new 
CoreV1EndpointPort().port(SECOND_PORT).protocol("TCP"))
+                        .build())
+                .build();
+        when(endpointsIndexer.getByKey(NAMESPACE + "/" + 
SERVICE_NAME)).thenReturn(mockedEndpoints);
+        when(endpointsInformer.getIndexer()).thenReturn(endpointsIndexer);
+
+        IngressParser ingressParser = new IngressParser(serviceInformer, 
endpointsInformer);
+        ingressReconciler = new IngressReconciler(ingressInformer, 
secretInformer, shenyuCacheRepository,
+                new ShenyuSniAsyncMapping(), ingressParser, 
mock(ApiClient.class));
+    }
+
+    /**
+     * test that the paths of an ingress are cached as a single service 
relation that keeps the port of
+     * the first path, the limitation documented on 
IngressReconciler#parseServiceFromIngress.
+     */
+    @Test
+    public void testOnlyThePortOfTheFirstPathIsCached() {
+        Result result = Assertions.assertDoesNotThrow(
+                () -> ingressReconciler.reconcile(new Request(NAMESPACE, 
INGRESS_NAME)));
+
+        Assertions.assertEquals(new Result(false), result);
+        List<String> selectorIds = 
IngressSelectorCache.getInstance().get(NAMESPACE, INGRESS_NAME, 
PluginEnum.DIVIDE.getName());
+        Assertions.assertEquals(2, selectorIds.size());
+        List<ServiceIngressRelation> relations = 
ServiceIngressCache.getInstance().getIngressName(NAMESPACE, SERVICE_NAME);
+        Assertions.assertEquals(1, relations.size());
+        Assertions.assertEquals(INGRESS_NAME, 
relations.get(0).getIngressName());
+        Assertions.assertEquals(Integer.valueOf(FIRST_PORT), 
relations.get(0).getPort().getNumber());
+    }
+
+    private static V1HTTPIngressPath path(final String path, final int port) {
+        return new 
V1HTTPIngressPathBuilder().withPath(path).withPathType("ImplementationSpecific")
+                .withNewBackend()
+                    
.withNewService().withName(SERVICE_NAME).withNewPort().withNumber(port).endPort().endService()
+                .endBackend().build();
+    }
+}

Reply via email to