Revert selecting Ready Pods, remove Terminating Pods in OAP clust…

…er (apache#10988)
panshuling321 · Jun 25, 2023 · 8997796 · 8997796
1 parent d1a6976
commit 8997796
Show file tree

Hide file tree

Showing 4 changed files with 4 additions and 16 deletions.
diff --git a/docs/en/changes/changes.md b/docs/en/changes/changes.md
@@ -8,7 +8,6 @@
 
 * Add Neo4j component ID(112) language: Python.
 * Add Istio ServiceEntry registry to resolve unknown IPs in ALS.
-* Improve Kubernetes coordinator to only select ready OAP Pods to build cluster.
 * Wrap `deleteProperty` API to the BanyanDBStorageClient.
 * [Breaking change] Remove `matchedCounter` from `HttpUriRecognitionService#feedRawData`.
 * Remove patterns from `HttpUriRecognitionService#feedRawData` and add max 10 candidates of raw URIs for each pattern.
@@ -17,6 +16,7 @@
 * Fix `NPE` in metrics query when the metric is not exist.
 * Remove E2E tests for Istio < 1.15, ElasticSearch < 7.16.3, they might still work but are not supported as planed.
 * Scroll all results in ElasticSearch storage and refactor scrolling logics, including Service, Instance, Endpoint, Process, etc.
+* Improve Kubernetes coordinator to remove `Terminating` OAP Pods in cluster.
 
 #### UI
 * Fix metric name `browser_app_error_rate` in `Browser-Root` dashboard.

diff --git a/...ava/org/apache/skywalking/oap/server/cluster/plugin/kubernetes/KubernetesCoordinator.java b/...ava/org/apache/skywalking/oap/server/cluster/plugin/kubernetes/KubernetesCoordinator.java
@@ -23,7 +23,6 @@
 import io.fabric8.kubernetes.api.model.PodStatus;
 import io.fabric8.kubernetes.client.informers.ResourceEventHandler;
 import lombok.extern.slf4j.Slf4j;
-import org.apache.skywalking.library.kubernetes.KubernetesPods;
 import org.apache.skywalking.oap.server.core.CoreModule;
 import org.apache.skywalking.oap.server.core.cluster.ClusterCoordinator;
 import org.apache.skywalking.oap.server.core.cluster.ClusterHealthStatus;
@@ -179,9 +178,9 @@ private void updateRemoteInstances(Pod pod, EventType event) {
                 switch (event) {
                     case ADDED:
                     case MODIFIED:
-                        if (KubernetesPods.isReady(pod)) {
+                        if ("Running".equalsIgnoreCase(pod.getStatus().getPhase())) {
                             remoteInstanceMap.put(remoteInstance.getAddress().toString(), remoteInstance);
-                        } else {
+                        } else if ("Terminating".equalsIgnoreCase(pod.getStatus().getPhase())) {
                             remoteInstanceMap.remove(remoteInstance.getAddress().toString());
                         }
                         break;

diff --git a/...org/apache/skywalking/oap/server/cluster/plugin/kubernetes/NamespacedPodListInformer.java b/...org/apache/skywalking/oap/server/cluster/plugin/kubernetes/NamespacedPodListInformer.java
@@ -23,7 +23,6 @@
 import io.fabric8.kubernetes.client.informers.ResourceEventHandler;
 import io.fabric8.kubernetes.client.informers.cache.Lister;
 import lombok.extern.slf4j.Slf4j;
-import org.apache.skywalking.library.kubernetes.KubernetesPods;
 
 import java.util.List;
 import java.util.Optional;
@@ -68,7 +67,7 @@ public Optional<List<Pod>> listPods() {
         return Optional.ofNullable(podLister.list().size() != 0
             ? podLister.list()
                        .stream()
-                       .filter(KubernetesPods::isReady)
+                       .filter(pod -> "Running".equalsIgnoreCase(pod.getStatus().getPhase()))
                        .collect(Collectors.toList())
             : null);
     }

diff --git a/...rnetes-support/src/main/java/org/apache/skywalking/library/kubernetes/KubernetesPods.java b/...rnetes-support/src/main/java/org/apache/skywalking/library/kubernetes/KubernetesPods.java
@@ -72,16 +72,6 @@ public Optional<Pod> load(ObjectID objectID) {
         });
     }
 
-    public static boolean isReady(Pod pod) {
-        return "Running".equalsIgnoreCase(pod.getStatus().getPhase()) &&
-            pod.getStatus()
-               .getConditions()
-               .stream()
-               .anyMatch(condition ->
-                   "Ready".equalsIgnoreCase(condition.getType()) &&
-                       "True".equalsIgnoreCase(condition.getStatus()));
-    }
-
     @SneakyThrows
     public Optional<Pod> findByIP(final String ip) {
         return podByIP.get(ip);