Skip to content

Commit

Permalink
feat: reduce conflicts when update configmap in k8s #89 (#93)
Browse files Browse the repository at this point in the history 
## What's the purpose of this PR

fix #89

## Which issue(s) this PR fixes:
fix #89

## Brief changelog

reduce conflicts when update configmap in k8s

Follow this checklist to help us incorporate your contribution quickly and easily:

- [x] Read the [Contributing Guide](https://github.com/apolloconfig/apollo/blob/master/CONTRIBUTING.md) before making this pull request.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Write necessary unit tests to verify the code.
- [x] Run `mvn clean test` to make sure this pull request doesn't break anything.
- [x] Update the [`CHANGES` log](https://github.com/apolloconfig/apollo-java/blob/master/CHANGES.md).


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

- **New Features**
	- Introduced a mechanism to reduce conflicts when updating ConfigMap in Kubernetes.
	- Enhanced access key secret retrieval for applications.
	- Added pod write permission controls for ConfigMap management.

- **Improvements**
	- Refined configuration utility for better property handling.
	- Improved Kubernetes resource management logic.

- **Testing**
	- Updated test suite to improve coverage of Kubernetes-related functionality.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
@24kpure
24kpure authored Jan 13, 2025
1 parent b476bb8 commit e8b41b6
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
65 changes: 64 additions & 1 deletion apollo-client/src/main/java/com/ctrip/framework/apollo/kubernetes/KubernetesManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,44 @@

import com.ctrip.framework.apollo.core.utils.StringUtils;
import com.ctrip.framework.apollo.exceptions.ApolloConfigException;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import io.kubernetes.client.openapi.ApiClient;
import io.kubernetes.client.openapi.ApiException;
import io.kubernetes.client.openapi.apis.CoreV1Api;
import io.kubernetes.client.openapi.models.V1ConfigMap;
import io.kubernetes.client.openapi.models.V1ObjectMeta;
import io.kubernetes.client.openapi.models.V1Pod;
import io.kubernetes.client.openapi.models.V1PodList;
import io.kubernetes.client.util.Config;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Service;

import java.util.Comparator;
import java.util.HashMap;
import java.util.Map;
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 = 3;
private String localPodName = System.getenv("HOSTNAME");

public KubernetesManager() {
try {
Expand All @@ -51,8 +68,11 @@ public KubernetesManager() {
}
}

public KubernetesManager(CoreV1Api coreV1Api) {
@VisibleForTesting
public KubernetesManager(CoreV1Api coreV1Api, String localPodName, int propertyKubernetesMaxWritePods) {
this.coreV1Api = coreV1Api;
this.localPodName = localPodName;
this.propertyKubernetesMaxWritePods = propertyKubernetesMaxWritePods;
}

private V1ConfigMap buildConfigMap(String name, String namespace, Map<String, String> data) {
Expand Down Expand Up @@ -132,6 +152,10 @@ public boolean updateConfigMap(String k8sNamespace, String name, Map<String, Str
return false;
}

if (!isWritePod(k8sNamespace)) {
return true;
}

int maxRetries = 5;
int retryCount = 0;
long waitTime = 100;
Expand Down Expand Up @@ -205,4 +229,43 @@ public boolean checkConfigMapExist(String k8sNamespace, String configMapName) {
return false;
}
}

/**
* check pod whether pod can write configmap
*
* @param k8sNamespace config map namespace
* @return true if this pod can write configmap, false otherwise
*/
private boolean isWritePod(String k8sNamespace) {
try {
if (Strings.isNullOrEmpty(localPodName)) {
return true;
}
V1Pod localPod = coreV1Api.readNamespacedPod(localPodName, k8sNamespace, null);
V1ObjectMeta localMetadata = localPod.getMetadata();
if (localMetadata == null || localMetadata.getLabels() == null) {
return true;
}
String appName = localMetadata.getLabels().get("app");
String labelSelector = "app=" + appName;

V1PodList v1PodList = coreV1Api.listNamespacedPod(k8sNamespace, null, null,
null, RUNNING_POD_FIELD_SELECTOR, labelSelector,
MAX_SEARCH_NUM, null, null
, null, null);

return v1PodList.getItems().stream()
.map(V1Pod::getMetadata)
.filter(Objects::nonNull)
//Make each node selects the same write nodes by sorting
.filter(metadata -> metadata.getCreationTimestamp() != null)
.sorted(Comparator.comparing(V1ObjectMeta::getCreationTimestamp))
.map(V1ObjectMeta::getName)
.limit(propertyKubernetesMaxWritePods)
.anyMatch(localPodName::equals);
} catch (Exception e) {
logger.info("Error determining write pod eligibility:{}", e.getMessage(), e);
return true;
}
}
}
2 changes: 1 addition & 1 deletion apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -617,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)) {
Expand Down
56 changes: 46 additions & 10 deletions apollo-client/src/test/java/com/ctrip/framework/apollo/kubernetes/KubernetesManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,30 @@
import io.kubernetes.client.openapi.apis.CoreV1Api;
import io.kubernetes.client.openapi.models.V1ConfigMap;
import io.kubernetes.client.openapi.models.V1ObjectMeta;
import io.kubernetes.client.openapi.models.V1Pod;
import io.kubernetes.client.openapi.models.V1PodList;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;

import java.time.OffsetDateTime;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import static org.mockito.Mockito.*;
import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.isNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class KubernetesManagerTest {

Expand All @@ -38,7 +54,7 @@ public class KubernetesManagerTest {
@Before
public void setUp() {
coreV1Api = mock(CoreV1Api.class);
kubernetesManager = new KubernetesManager(coreV1Api);
kubernetesManager = new KubernetesManager(coreV1Api, "localPodName", 3);

MockInjector.setInstance(KubernetesManager.class, kubernetesManager);
MockInjector.setInstance(CoreV1Api.class, coreV1Api);
Expand All @@ -58,13 +74,13 @@ public void testCreateConfigMapSuccess() throws Exception {
.metadata(new V1ObjectMeta().name(name).namespace(namespace))
.data(data);

when(coreV1Api.createNamespacedConfigMap(eq(namespace), eq(configMap), isNull(), isNull(), isNull(),isNull())).thenReturn(configMap);
when(coreV1Api.createNamespacedConfigMap(eq(namespace), eq(configMap), isNull(), isNull(), isNull(), isNull())).thenReturn(configMap);

// act
String result = kubernetesManager.createConfigMap(namespace, name, data);

// assert
verify(coreV1Api, times(1)).createNamespacedConfigMap(eq(namespace), any(V1ConfigMap.class), isNull(), isNull(), isNull(),isNull());
verify(coreV1Api, times(1)).createNamespacedConfigMap(eq(namespace), any(V1ConfigMap.class), isNull(), isNull(), isNull(), isNull());
assert name.equals(result);
}

Expand All @@ -82,7 +98,7 @@ public void testCreateConfigMapNullData() throws Exception {
String result = kubernetesManager.createConfigMap(namespace, name, data);

// assert
verify(coreV1Api, times(1)).createNamespacedConfigMap(eq(namespace), any(V1ConfigMap.class), isNull(), isNull(), isNull(),isNull());
verify(coreV1Api, times(1)).createNamespacedConfigMap(eq(namespace), any(V1ConfigMap.class), isNull(), isNull(), isNull(), isNull());
assert name.equals(result);
}

Expand Down Expand Up @@ -135,20 +151,40 @@ public void testUpdateConfigMapSuccess() throws Exception {
// arrange
String namespace = "default";
String name = "testConfigMap";
Map<String, String> data = new HashMap<>();
data.put("key", "value");

V1Pod pod = new V1Pod()
.metadata(
new V1ObjectMeta()
.name("localPodName")
.creationTimestamp(OffsetDateTime.now())
.labels(Collections.singletonMap("app", "app")));
V1PodList v1PodList = new V1PodList().addItemsItem(new V1Pod().metadata(pod.getMetadata()));

Map<String, String> existData = new HashMap<>();
existData.put("key", "value");
V1ConfigMap configMap = new V1ConfigMap();
configMap.metadata(new V1ObjectMeta().name(name).namespace(namespace));
configMap.data(data);
configMap.data(existData);

when(coreV1Api.readNamespacedPod("localPodName", namespace, null)).thenReturn(pod);
when(coreV1Api.listNamespacedPod(namespace, null, null,
null, null, "app=app",
null, null, null
, null, null)).thenReturn(v1PodList);
when(coreV1Api.readNamespacedConfigMap(name, namespace, null)).thenReturn(configMap);
when(coreV1Api.replaceNamespacedConfigMap(name, namespace, configMap, null, null, null, null)).thenReturn(configMap);

// act
Boolean success = kubernetesManager.updateConfigMap(namespace, name, data);
HashMap<String, String> updateData = new HashMap<>(existData);
updateData.put("newKey","newValue");
boolean success = kubernetesManager.updateConfigMap(namespace, name, updateData);

// assert
assertTrue(success);
Mockito.verify(coreV1Api, Mockito.times(1)).listNamespacedPod(namespace, null, null,
null, "status.phase=Running", "app=app",
100, null, null
, null, null);
}

/**
Expand Down

0 comments on commit e8b41b6

Please sign in to comment.