fix: 修复昨晚讨论的问题

apolloconfig · Oct 30, 2024 · f7ff6d9 · f7ff6d9
1 parent 0c66c59
commit f7ff6d9
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 62 deletions.
diff --git a/...ient/src/main/java/com/ctrip/framework/apollo/internals/K8sConfigMapConfigRepository.java b/...ient/src/main/java/com/ctrip/framework/apollo/internals/K8sConfigMapConfigRepository.java
@@ -27,7 +27,7 @@
 import com.ctrip.framework.apollo.tracer.spi.Transaction;
 import com.ctrip.framework.apollo.util.ConfigUtil;
 import com.ctrip.framework.apollo.util.ExceptionUtil;
-import com.google.common.base.Joiner;
+import com.ctrip.framework.apollo.util.escape.EscapeUtil;
 import com.google.common.base.Preconditions;
 import com.google.gson.Gson;
 import com.google.gson.reflect.TypeToken;
@@ -67,24 +67,16 @@ public K8sConfigMapConfigRepository(String namespace, ConfigRepository upstream)
         this.setUpstreamRepository(upstream);
     }
 
-    String getConfigMapKey() {
-        return configMapKey;
-    }
-
-    String getConfigMapName() {
-        return configMapName;
-    }
-
-    void setConfigMapKey(String cluster, String namespace) {
+    private void setConfigMapKey(String cluster, String namespace) {
         // cluster: User Definition >idc>default
         if (StringUtils.isBlank(cluster)) {
-            configMapKey = Joiner.on(ConfigConsts.CONFIGMAP_KEY_SEPARATOR).join("default", namespace);
+            configMapKey = EscapeUtil.createConfigMapKey("default", namespace);
             return;
         }
-        configMapKey = Joiner.on(ConfigConsts.CONFIGMAP_KEY_SEPARATOR).join(cluster, namespace);
+        configMapKey = EscapeUtil.createConfigMapKey(cluster, namespace);
     }
 
-    void setConfigMapName(String appId, boolean syncImmediately) {
+    private void setConfigMapName(String appId, boolean syncImmediately) {
         Preconditions.checkNotNull(appId, "AppId cannot be null");
         configMapName = ConfigConsts.APOLLO_CONFIG_CACHE + appId;
         this.checkConfigMapName(configMapName);
@@ -181,11 +173,11 @@ protected void sync() {
         }
     }
 
-    public Properties loadFromK8sConfigMap() {
+    Properties loadFromK8sConfigMap() {
         Preconditions.checkNotNull(configMapName, "ConfigMap name cannot be null");
 
         try {
-            String jsonConfig = loadConfigFromK8sWithRetry();
+            String jsonConfig = kubernetesManager.getValueFromConfigMap(k8sNamespace, configMapName, configMapKey);
 
             // Convert jsonConfig to properties
             Properties properties = propertiesFactory.getPropertiesInstance();
@@ -202,30 +194,7 @@ public Properties loadFromK8sConfigMap() {
         }
     }
 
-    private String loadConfigFromK8sWithRetry() {
-        String jsonConfig = null;
-
-        // Try to load from the specified cluster
-        if (!configMapKey.equals("default")) {
-            jsonConfig = kubernetesManager.getValueFromConfigMap(k8sNamespace, configMapName, configMapKey);
-        }
-
-        // Try to load from the data center cluster
-        if (StringUtils.isBlank(jsonConfig) && !configUtil.getDataCenter().equals(configMapKey)) {
-            String dataCenterKey = Joiner.on(ConfigConsts.CONFIGMAP_KEY_SEPARATOR).join(configUtil.getDataCenter(), namespace);
-            jsonConfig = kubernetesManager.getValueFromConfigMap(k8sNamespace, configMapName, dataCenterKey);
-        }
-
-        // Fallback to the default cluster
-        if (StringUtils.isBlank(jsonConfig)) {
-            String defaultKey = Joiner.on(ConfigConsts.CONFIGMAP_KEY_SEPARATOR).join("default", namespace);
-            jsonConfig = kubernetesManager.getValueFromConfigMap(k8sNamespace, configMapName, defaultKey);
-        }
-
-        return jsonConfig;
-    }
-
-    public boolean trySyncFromUpstream() {
+    private boolean trySyncFromUpstream() {
         if (upstream == null) {
             return false;
         }
@@ -266,7 +235,7 @@ public void onRepositoryChange(String namespace, Properties newProperties) {
         this.fireRepositoryChange(namespace, newProperties);
     }
 
-    public void persistConfigMap(Properties properties) {
+    void persistConfigMap(Properties properties) {
         Transaction transaction = Tracer.newTransaction("Apollo.ConfigService", "persistK8sConfigMap");
         transaction.addData("configMapName", configMapName);
         transaction.addData("k8sNamespace", k8sNamespace);

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
@@ -17,6 +17,7 @@
 package com.ctrip.framework.apollo.kubernetes;
 
 import com.ctrip.framework.apollo.core.utils.StringUtils;
+import com.ctrip.framework.apollo.exceptions.ApolloConfigException;
 import io.kubernetes.client.openapi.ApiClient;
 import io.kubernetes.client.openapi.ApiException;
 import io.kubernetes.client.openapi.apis.CoreV1Api;
@@ -36,7 +37,7 @@ public class KubernetesManager {
     private ApiClient client;
     private CoreV1Api coreV1Api;
 
-    private final Logger logger = LoggerFactory.getLogger(this.getClass());
+    private static final Logger logger = LoggerFactory.getLogger(KubernetesManager.class);
 
     public KubernetesManager() {
         try {
@@ -53,7 +54,7 @@ public KubernetesManager(CoreV1Api coreV1Api) {
         this.coreV1Api = coreV1Api;
     }
 
-    public V1ConfigMap buildConfigMap(String name, String namespace, Map<String, String> data) {
+    private V1ConfigMap buildConfigMap(String name, String namespace, Map<String, String> data) {
         V1ObjectMeta metadata = new V1ObjectMeta()
                 .name(name)
                 .namespace(namespace);
@@ -124,39 +125,57 @@ public String getValueFromConfigMap(String k8sNamespace, String name, String key
      * @return config map name
      */
     // Set the retry times using the client retry mechanism (CAS)
-    public boolean updateConfigMap(String k8sNamespace, String name, Map<String, String> data) {
+    public boolean updateConfigMap(String k8sNamespace, String name, Map<String, String> data) throws ApiException {
         if (StringUtils.isEmpty(k8sNamespace) || StringUtils.isEmpty(name)) {
             logger.error("Parameters can not be null or empty: k8sNamespace={}, name={}", k8sNamespace, name);
             return false;
         }
 
-        // retry
         int maxRetries = 5;
         int retryCount = 0;
         long waitTime = 100;
 
         while (retryCount < maxRetries) {
             try {
                 V1ConfigMap configmap = coreV1Api.readNamespacedConfigMap(name, k8sNamespace, null);
-                configmap.setData(data);
+                Map<String, String> existingData = configmap.getData();
+
+                // Determine if the data contains its own kv and de-weight it
+                boolean containsEntry = data.entrySet().stream()
+                        .allMatch(entry -> entry.getValue().equals(existingData.get(entry.getKey())));
+
+                if (containsEntry) {
+                    logger.info("Data is identical or already contains the entry, no update needed.");
+                    return true;
+                }
+
+                // Add new entries to the existing data
+                existingData.putAll(data);
+                configmap.setData(existingData);
+
                 coreV1Api.replaceNamespacedConfigMap(name, k8sNamespace, configmap, null, null, null, null);
                 return true;
             } catch (ApiException e) {
                 if (e.getCode() == 409) {
                     retryCount++;
                     logger.warn("Conflict occurred, retrying... ({})", retryCount);
                     try {
-                        TimeUnit.MILLISECONDS.sleep(waitTime);
+                        // Scramble the time, so that different machines in the distributed retry time is different
+                        // The random ratio ranges from 0.9 to 1.1
+                        TimeUnit.MILLISECONDS.sleep((long) (waitTime * (0.9 + Math.random() * 0.2)));
                     } catch (InterruptedException ie) {
                         Thread.currentThread().interrupt();
                     }
                     waitTime = Math.min(waitTime * 2, 1000);
                 } else {
                     logger.error("Error updating ConfigMap: {}", e.getMessage(), e);
+                    throw e;
                 }
             }
         }
-        return false;
+        String errorMessage = String.format("Failed to update ConfigMap after %d retries: k8sNamespace=%s, name=%s", maxRetries, k8sNamespace, name);
+        logger.error(errorMessage);
+        throw new ApolloConfigException(errorMessage);
     }
 
     /**

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
@@ -379,7 +379,7 @@ private String getDeprecatedCustomizedCacheRoot() {
   }
 
   public String getK8sNamespace() {
-    String k8sNamespace = getCustomizedConfigMapNamespace();
+    String k8sNamespace = getCacheKubernetesNamespace();
 
     if (!Strings.isNullOrEmpty(k8sNamespace)) {
       return k8sNamespace;
@@ -388,7 +388,7 @@ public String getK8sNamespace() {
     return ConfigConsts.KUBERNETES_CACHE_CONFIG_MAP_NAMESPACE_DEFAULT;
   }
 
-  private String getCustomizedConfigMapNamespace() {
+  private String getCacheKubernetesNamespace() {
     // 1. Get from System Property
     String k8sNamespace = System.getProperty(ApolloClientSystemConsts.APOLLO_CACHE_KUBERNETES_NAMESPACE);
     if (Strings.isNullOrEmpty(k8sNamespace)) {

diff --git a/apollo-client/src/main/java/com/ctrip/framework/apollo/util/escape/EscapeUtil.java b/apollo-client/src/main/java/com/ctrip/framework/apollo/util/escape/EscapeUtil.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright 2022 Apollo Authors
+ *
+ * Licensed 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 com.ctrip.framework.apollo.util.escape;
+
+/**
+ * @author dyx1234
+ */
+public class EscapeUtil {
+
+    // Escapes a single underscore in a namespace
+    public static String escapeNamespace(String namespace) {
+        return namespace.replace("_", "__");
+    }
+
+    // Concatenate the cluster and the escaped namespace, using three underscores as delimiters
+    public static String createConfigMapKey(String cluster, String namespace) {
+        String escapedNamespace = escapeNamespace(namespace);
+        return cluster + "___" + escapedNamespace;
+    }
+
+}
diff --git a/.../src/test/java/com/ctrip/framework/apollo/internals/K8sConfigMapConfigRepositoryTest.java b/.../src/test/java/com/ctrip/framework/apollo/internals/K8sConfigMapConfigRepositoryTest.java
@@ -20,10 +20,13 @@
 import com.ctrip.framework.apollo.enums.ConfigSourceType;
 import com.ctrip.framework.apollo.kubernetes.KubernetesManager;
 import com.ctrip.framework.apollo.util.ConfigUtil;
+import com.ctrip.framework.apollo.util.escape.EscapeUtil;
+import io.kubernetes.client.openapi.ApiException;
 import io.kubernetes.client.openapi.models.V1ConfigMap;
 import io.kubernetes.client.openapi.models.V1ObjectMeta;
 import org.junit.Before;
 import org.junit.Test;
+import org.springframework.test.util.ReflectionTestUtils;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -68,7 +71,7 @@ public void setUp() {
         when(upstreamRepo.getConfig()).thenReturn(someProperties);
         when(upstreamRepo.getSourceType()).thenReturn(someSourceType);
 
-        // make comfigmap
+        // make configmap
         data = new HashMap<>();
         data.put(defaultKey, defaultJsonValue);
         configMap = new V1ConfigMap()
@@ -78,20 +81,40 @@ public void setUp() {
         k8sConfigMapConfigRepository = new K8sConfigMapConfigRepository(someNamespace, upstreamRepo);
     }
 
+    // TODO 直接mock manager中的参数
 
+    /**
+     * 测试setConfigMapKey方法，当cluster和namespace都为正常值时
+     */
     @Test
-    public void testSetConfigMapKey() {
-        when(kubernetesManager.createConfigMap(anyString(), anyString(), any())).thenReturn("someAppId");
-        k8sConfigMapConfigRepository.setConfigMapKey(someCluster, someNamespace);
-        assertEquals(someCluster +"-"+ someNamespace, k8sConfigMapConfigRepository.getConfigMapKey());
-    }
+    public void testSetConfigMapKeyUnderNormalConditions() throws Throwable {
+        // arrange
+        String cluster = "testCluster";
+        String namespace = "test_Namespace_1";
+        String escapedKey = "testCluster___test__Namespace__1";
 
-    @Test
-    public void testSetConfigMapName() {
-        k8sConfigMapConfigRepository.setConfigMapName(someAppId, false);
-        assertEquals(someConfigmapName, k8sConfigMapConfigRepository.getConfigMapName());
+        // act
+        ReflectionTestUtils.invokeMethod(k8sConfigMapConfigRepository, "setConfigMapKey", cluster, namespace);
+
+        // assert
+        String expectedConfigMapKey = EscapeUtil.createConfigMapKey(cluster, namespace);
+        assertEquals(escapedKey, ReflectionTestUtils.getField(k8sConfigMapConfigRepository, "configMapKey"));
+        assertEquals(expectedConfigMapKey, ReflectionTestUtils.getField(k8sConfigMapConfigRepository, "configMapKey"));
     }
 
+//    @Test
+//    public void testSetConfigMapKey() {
+//        when(kubernetesManager.createConfigMap(anyString(), anyString(), any())).thenReturn("someAppId");
+//        k8sConfigMapConfigRepository.setConfigMapKey(someCluster, someNamespace);
+//        assertEquals(someCluster +"-"+ someNamespace, k8sConfigMapConfigRepository.getConfigMapKey());
+//    }
+//
+//    @Test
+//    public void testSetConfigMapName() {
+//        k8sConfigMapConfigRepository.setConfigMapName(someAppId, false);
+//        assertEquals(someConfigmapName, k8sConfigMapConfigRepository.getConfigMapName());
+//    }
+
     /**
      * 测试sync方法成功从上游数据源同步
      */
@@ -140,7 +163,7 @@ public void testGetConfig() {
     }
 
     @Test
-    public void testPersistConfigMap() {
+    public void testPersistConfigMap() throws ApiException {
         // Arrange
         Properties properties = new Properties();
         properties.setProperty(defaultKey, defaultValue);
@@ -151,7 +174,7 @@ public void testPersistConfigMap() {
     }
 
     @Test
-    public void testOnRepositoryChange() {
+    public void testOnRepositoryChange() throws ApiException {
         // Arrange
         Properties newProperties = new Properties();
         newProperties.setProperty(defaultKey, defaultValue);

diff --git a/apollo-client/src/test/java/com/ctrip/framework/apollo/util/ConfigUtilTest.java b/apollo-client/src/test/java/com/ctrip/framework/apollo/util/ConfigUtilTest.java
@@ -48,6 +48,7 @@ public void tearDown() throws Exception {
     System.clearProperty(PropertiesFactory.APOLLO_PROPERTY_ORDER_ENABLE);
     System.clearProperty(ApolloClientSystemConsts.APOLLO_PROPERTY_NAMES_CACHE_ENABLE);
     System.clearProperty(ApolloClientSystemConsts.APOLLO_CACHE_KUBERNETES_NAMESPACE);
+    System.clearProperty(ApolloClientSystemConsts.APOLLO_KUBERNETES_CACHE_ENABLE);
   }
 
   @Test
@@ -262,6 +263,17 @@ public void testK8sNamespaceWithDefault() {
     assertEquals(ConfigConsts.KUBERNETES_CACHE_CONFIG_MAP_NAMESPACE_DEFAULT, configUtil.getK8sNamespace());
   }
 
+  @Test
+  public void testKubernetesCacheEnabledWithSystemProperty() {
+    boolean someKubernetesCacheEnabled = true;
+
+    System.setProperty(ApolloClientSystemConsts.APOLLO_KUBERNETES_CACHE_ENABLE, String.valueOf(someKubernetesCacheEnabled));
+
+    ConfigUtil configUtil = new ConfigUtil();
+
+    assertTrue(configUtil.isPropertyKubernetesCacheEnabled());
+  }
+
   @Test
   public void testCustomizePropertiesOrdered() {
     boolean propertiesOrdered = true;

diff --git a/apollo-core/src/main/java/com/ctrip/framework/apollo/core/ConfigConsts.java b/apollo-core/src/main/java/com/ctrip/framework/apollo/core/ConfigConsts.java
@@ -20,7 +20,6 @@ public interface ConfigConsts {
   String NAMESPACE_APPLICATION = "application";
   String CLUSTER_NAME_DEFAULT = "default";
   String CLUSTER_NAMESPACE_SEPARATOR = "+";
-  String CONFIGMAP_KEY_SEPARATOR = "-";
   String APOLLO_CONFIG_CACHE = "apollo-configcache-";
   String APOLLO_CLUSTER_KEY = "apollo.cluster";
   String APOLLO_META_KEY = "apollo.meta";