From 5db4993d623d1fe16333473d2f38cd72781a1cd7 Mon Sep 17 00:00:00 2001 From: 24kpure <1033237526@qq.com> Date: Tue, 24 Dec 2024 18:00:45 +0800 Subject: [PATCH] changes for code review --- CHANGES.md | 1 + .../apollo/kubernetes/KubernetesManager.java | 20 ++++++--- .../framework/apollo/util/ConfigUtil.java | 44 +++++-------------- .../kubernetes/KubernetesManagerTest.java | 4 +- .../apollo/core/ApolloClientSystemConsts.java | 10 ----- 5 files changed, 28 insertions(+), 51 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index c2f6ac72..10017ab8 100644 --- 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 index 76824e08..67163c5c 100644 --- 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 = 1000; + 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 index 237952bf..09e0ba2f 100644 --- 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 index e701aac3..286b16ad 100644 --- 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", + 1000, 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 index 0f3a118d..c199c1a2 100644 --- 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 */