changes for code review

apolloconfig · Jan 13, 2025 · 0ce4201 · 0ce4201
1 parent 1cc173e
commit 0ce4201
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 51 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -14,6 +14,7 @@ Apollo Java 2.4.0
 * [Fix monitor arg cause npe](https://github.com/apolloconfig/apollo-java/pull/86)
 * [Fix the concurrent issue in SpringValueRegistry.scanAndClean](https://github.com/apolloconfig/apollo-java/pull/95)
 * [Feature support incremental configuration synchronization client](https://github.com/apolloconfig/apollo-java/pull/90)
+* [Feature reduce conflicts when update configmap in k8](https://github.com/apolloconfig/apollo-java/pull/93)
 
 ------------------
 All issues and pull requests are [here](https://github.com/apolloconfig/apollo-java/milestone/4?closed=1)
diff --git a/apollo-client/src/main/java/com/ctrip/framework/apollo/kubernetes/KubernetesManager.java b/apollo-client/src/main/java/com/ctrip/framework/apollo/kubernetes/KubernetesManager.java
@@ -16,10 +16,8 @@
  */
 package com.ctrip.framework.apollo.kubernetes;
 
-import com.ctrip.framework.apollo.build.ApolloInjector;
 import com.ctrip.framework.apollo.core.utils.StringUtils;
 import com.ctrip.framework.apollo.exceptions.ApolloConfigException;
-import com.ctrip.framework.apollo.util.ConfigUtil;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import io.kubernetes.client.openapi.ApiClient;
@@ -40,21 +38,29 @@
 import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 
+/**
+ * Manages Kubernetes ConfigMap operations.
+ * Required Kubernetes permissions:
+ * - pods: [get, list] - For pod selection and write eligibility
+ * - configmaps: [get, create, update] - For ConfigMap operations
+ */
 @Service
 public class KubernetesManager {
     private static final Logger logger = LoggerFactory.getLogger(KubernetesManager.class);
 
+    private static final String RUNNING_POD_FIELD_SELECTOR = "status.phase=Running";
+
+    private static final int MAX_SEARCH_NUM = 100;
+
     private ApiClient client;
     private CoreV1Api coreV1Api;
-    private int propertyKubernetesMaxWritePods;
+    private int propertyKubernetesMaxWritePods = 3;
     private String localPodName = System.getenv("HOSTNAME");
 
     public KubernetesManager() {
         try {
             client = Config.defaultClient();
             coreV1Api = new CoreV1Api(client);
-            ConfigUtil configUtil = ApolloInjector.getInstance(ConfigUtil.class);
-            propertyKubernetesMaxWritePods = configUtil.getPropertyKubernetesMaxWritePods();
         } catch (Exception e) {
             String errorMessage = "Failed to initialize Kubernetes client: " + e.getMessage();
             logger.error(errorMessage, e);
@@ -244,8 +250,8 @@ private boolean isWritePod(String k8sNamespace) {
             String labelSelector = "app=" + appName;
 
             V1PodList v1PodList = coreV1Api.listNamespacedPod(k8sNamespace, null, null,
-                    null, null, labelSelector,
-                    null, null, null
+                    null, RUNNING_POD_FIELD_SELECTOR, labelSelector,
+                    MAX_SEARCH_NUM, null, null
                     , null, null);
 
             return v1PodList.getItems().stream()

diff --git a/apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java b/apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java
@@ -72,14 +72,12 @@ public class ConfigUtil {
   private boolean propertyFileCacheEnabled = true;
   private boolean overrideSystemProperties = true;
   private boolean propertyKubernetesCacheEnabled = false;
-  private int propertyKubernetesMaxWritePods = 3;
   private boolean clientMonitorEnabled = false;
   private boolean clientMonitorJmxEnabled = false;
   private String monitorExternalType = "";
   private long monitorExternalExportPeriod = 10;
   private int monitorExceptionQueueSize = 25;
 
-
   public ConfigUtil() {
     warnLogRateLimiter = RateLimiter.create(0.017); // 1 warning log output per minute
     initRefreshInterval();
@@ -95,7 +93,6 @@ public ConfigUtil() {
     initPropertyFileCacheEnabled();
     initOverrideSystemProperties();
     initPropertyKubernetesCacheEnabled();
-    initPropertyKubernetesMaxWritePods();
     initClientMonitorEnabled();
     initClientMonitorJmxEnabled();
     initClientMonitorExternalType();
@@ -393,44 +390,31 @@ private String getDeprecatedCustomizedCacheRoot() {
   }
 
   public String getK8sNamespace() {
-    return getK8sConfigProperties(ApolloClientSystemConsts.APOLLO_CACHE_KUBERNETES_NAMESPACE,
-            ApolloClientSystemConsts.APOLLO_CACHE_KUBERNETES_NAMESPACE_ENVIRONMENT_VARIABLES,
-            ConfigConsts.KUBERNETES_CACHE_CONFIG_MAP_NAMESPACE_DEFAULT);
-  }
+    String k8sNamespace = getCacheKubernetesNamespace();
 
-  private void initPropertyKubernetesMaxWritePods() {
-    String propertyKubernetesMaxWritePodsStr = getK8sConfigProperties(ApolloClientSystemConsts.APOLLO_CACHE_KUBERNETES_MAX_WRITE_PODS,
-            ApolloClientSystemConsts.APOLLO_CACHE_KUBERNETES_MAX_WRITE_PODS_ENVIRONMENT_VARIABLES,
-            String.valueOf(propertyKubernetesMaxWritePods));
-    if (!Strings.isNullOrEmpty(propertyKubernetesMaxWritePodsStr)) {
-      try {
-        propertyKubernetesMaxWritePods = Integer.parseInt(propertyKubernetesMaxWritePodsStr);
-      } catch (Throwable ex) {
-        logger.error("Config for {} is invalid: {}",
-                ApolloClientSystemConsts.APOLLO_CACHE_KUBERNETES_NAMESPACE, propertyKubernetesMaxWritePodsStr);
-      }
+    if (!Strings.isNullOrEmpty(k8sNamespace)) {
+      return k8sNamespace;
     }
+
+    return ConfigConsts.KUBERNETES_CACHE_CONFIG_MAP_NAMESPACE_DEFAULT;
   }
 
-  private String getK8sConfigProperties(String key, String environmentKey, String defaultValue) {
+  private String getCacheKubernetesNamespace() {
     // 1. Get from System Property
-    String k8sNamespace = System.getProperty(key);
+    String k8sNamespace = System.getProperty(ApolloClientSystemConsts.APOLLO_CACHE_KUBERNETES_NAMESPACE);
     if (Strings.isNullOrEmpty(k8sNamespace)) {
       // 2. Get from OS environment variable
-      k8sNamespace = System.getenv(environmentKey);
+      k8sNamespace = System.getenv(ApolloClientSystemConsts.APOLLO_CACHE_KUBERNETES_NAMESPACE_ENVIRONMENT_VARIABLES);
     }
     if (Strings.isNullOrEmpty(k8sNamespace)) {
       // 3. Get from server.properties
-      k8sNamespace = Foundation.server().getProperty(key, null);
+      k8sNamespace = Foundation.server().getProperty(ApolloClientSystemConsts.APOLLO_CACHE_KUBERNETES_NAMESPACE, null);
     }
     if (Strings.isNullOrEmpty(k8sNamespace)) {
       // 4. Get from app.properties
-      k8sNamespace = Foundation.app().getProperty(key, null);
-    }
-    if (!Strings.isNullOrEmpty(k8sNamespace)) {
-      return k8sNamespace;
+      k8sNamespace = Foundation.app().getProperty(ApolloClientSystemConsts.APOLLO_CACHE_KUBERNETES_NAMESPACE, null);
     }
-    return defaultValue;
+    return k8sNamespace;
   }
 
   public boolean isInLocalMode() {
@@ -540,10 +524,6 @@ public boolean isPropertyKubernetesCacheEnabled() {
     return propertyKubernetesCacheEnabled;
   }
 
-  public int getPropertyKubernetesMaxWritePods() {
-    return propertyKubernetesMaxWritePods;
-  }
-
   public boolean isOverrideSystemProperties() {
     return overrideSystemProperties;
   }
@@ -637,7 +617,7 @@ private void initClientMonitorExceptionQueueSize() {
   public int getMonitorExceptionQueueSize() {
     return monitorExceptionQueueSize;
   }
-  
+
   private boolean getPropertyBoolean(String propertyName, String envName, boolean defaultVal) {
     String enablePropertyNamesCache = System.getProperty(propertyName);
     if (Strings.isNullOrEmpty(enablePropertyNamesCache)) {

diff --git a/apollo-client/src/test/java/com/ctrip/framework/apollo/kubernetes/KubernetesManagerTest.java b/apollo-client/src/test/java/com/ctrip/framework/apollo/kubernetes/KubernetesManagerTest.java
@@ -182,8 +182,8 @@ public void testUpdateConfigMapSuccess() throws Exception {
         // assert
         assertTrue(success);
         Mockito.verify(coreV1Api, Mockito.times(1)).listNamespacedPod(namespace, null, null,
-                null, null, "app=app",
-                null, null, null
+                null, "status.phase=Running", "app=app",
+                100, null, null
                 , null, null);
     }
 

diff --git a/apollo-core/src/main/java/com/ctrip/framework/apollo/core/ApolloClientSystemConsts.java b/apollo-core/src/main/java/com/ctrip/framework/apollo/core/ApolloClientSystemConsts.java
@@ -83,16 +83,6 @@ public class ApolloClientSystemConsts {
    */
   public static final String APOLLO_CACHE_KUBERNETES_NAMESPACE_ENVIRONMENT_VARIABLES = "APOLLO_CACHE_KUBERNETES_NAMESPACE";
 
-  /**
-   * max number of pods that can write the configmap cache in Kubernetes
-   */
-  public static final String APOLLO_CACHE_KUBERNETES_MAX_WRITE_PODS = "apollo.cache.kubernetes.max-write-pods";
-
-  /**
-   * max number of pods that can write the configmap cache in Kubernetes environment variables
-   */
-  public static final String APOLLO_CACHE_KUBERNETES_MAX_WRITE_PODS_ENVIRONMENT_VARIABLES = "APOLLO_CACHE_KUBERNETES_MAX_WRITE_PODS";
-
   /**
    * apollo client access key
    */