diff --git a/spring-cloud-kubernetes-lock/src/main/java/org/springframework/cloud/kubernetes/lock/ConfigMapLockRepository.java b/spring-cloud-kubernetes-lock/src/main/java/org/springframework/cloud/kubernetes/lock/ConfigMapLockRepository.java index 6717ab2c..426783dc 100644 --- a/spring-cloud-kubernetes-lock/src/main/java/org/springframework/cloud/kubernetes/lock/ConfigMapLockRepository.java +++ b/spring-cloud-kubernetes-lock/src/main/java/org/springframework/cloud/kubernetes/lock/ConfigMapLockRepository.java @@ -16,8 +16,7 @@ package org.springframework.cloud.kubernetes.lock; -import java.util.List; -import java.util.stream.Collectors; +import java.util.Optional; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; @@ -53,22 +52,26 @@ public class ConfigMapLockRepository { this.namespace = namespace; } - public ConfigMap get(String name) { - return kubernetesClient.configMaps() + public Optional get(String name) { + String configMapName = getConfigMapName(name); + ConfigMap configMap = kubernetesClient.configMaps() .inNamespace(namespace) - .withName(getConfigMapName(name)) + .withName(configMapName) .get(); + + return Optional.ofNullable(configMap); } public boolean create(String name, String holder, long expiration) { + String configMapName = getConfigMapName(name); + String expirationString = String.valueOf(expiration); ConfigMap configMap = new ConfigMapBuilder().withNewMetadata() - .withName(getConfigMapName(name)) + .withName(configMapName) .addToLabels(PROVIDER_LABEL, PROVIDER_LABEL_VALUE) .addToLabels(KIND_LABEL, KIND_LABEL_VALUE) .endMetadata() .addToData(HOLDER_KEY, holder) - .addToData(EXPIRATION_KEY, String.valueOf(expiration)) - // TODO add information about the creator + .addToData(EXPIRATION_KEY, expirationString) .build(); try { @@ -83,30 +86,23 @@ public class ConfigMapLockRepository { return true; } - public void deleteAll() { + public void delete(String name) { + // TODO make sure that only creator can delete the lock kubernetesClient.configMaps() .inNamespace(namespace) - .withLabel(PROVIDER_LABEL, PROVIDER_LABEL_VALUE) - .withLabel(KIND_LABEL, KIND_LABEL_VALUE) + .withName(getConfigMapName(name)) .delete(); } - public void deleteExpired() { - long now = System.currentTimeMillis(); - // TODO check that it was created by this process - List configMaps = kubernetesClient.configMaps() - .inNamespace(namespace) - .withLabel(PROVIDER_LABEL, PROVIDER_LABEL_VALUE) - .withLabel(KIND_LABEL, KIND_LABEL_VALUE) - .list() - .getItems() - .stream() - .filter(c -> Long.valueOf(c.getData().get("expiration")) < now) - .collect(Collectors.toList()); + public void deleteIfExpired(String name) { + get(name) + .filter(this::isExpired) + .ifPresent(c -> delete(name)); + } - kubernetesClient.configMaps() - .inNamespace(namespace) - .delete(configMaps); + private boolean isExpired(ConfigMap configMap) { + String expirationString = configMap.getData().get(EXPIRATION_KEY); + return Long.valueOf(expirationString) < System.currentTimeMillis(); } private String getConfigMapName(String name) { diff --git a/spring-cloud-kubernetes-lock/src/test/java/org/springframework/cloud/kubernetes/lock/ConfigMapLockRepositoryIT.java b/spring-cloud-kubernetes-lock/src/test/java/org/springframework/cloud/kubernetes/lock/ConfigMapLockRepositoryIT.java index 8d7d9044..b18e6b62 100644 --- a/spring-cloud-kubernetes-lock/src/test/java/org/springframework/cloud/kubernetes/lock/ConfigMapLockRepositoryIT.java +++ b/spring-cloud-kubernetes-lock/src/test/java/org/springframework/cloud/kubernetes/lock/ConfigMapLockRepositoryIT.java @@ -1,6 +1,7 @@ package org.springframework.cloud.kubernetes.lock; import java.util.Map; +import java.util.Optional; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.client.KubernetesClient; @@ -14,11 +15,17 @@ import org.junit.Test; import org.junit.runner.RunWith; import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.cloud.kubernetes.lock.ConfigMapLockRepository.EXPIRATION_KEY; +import static org.springframework.cloud.kubernetes.lock.ConfigMapLockRepository.HOLDER_KEY; @RunWith(Arquillian.class) @RequiresKubernetes public class ConfigMapLockRepositoryIT { + private static final String NAME = "test-name"; + + private static final String HOLDER = "test-holder"; + @ArquillianResource private KubernetesClient kubernetesClient; @@ -34,54 +41,46 @@ public class ConfigMapLockRepositoryIT { @After public void after() { - deleteConfigMap("test-name"); - deleteConfigMap("test-name-2"); + repository.delete(NAME); } @Test - public void shouldCreateConfigMap() { - long expiration = System.currentTimeMillis(); - boolean result = repository.create("test-name", "test-holder", expiration); - assertThat(result).isTrue(); + public void shouldCreate() { + assertThat(repository.create(NAME, HOLDER, 1000)).isTrue(); - ConfigMap configMap = repository.get("test-name"); - Map data = configMap.getData(); - assertThat(data).containsEntry(ConfigMapLockRepository.HOLDER_KEY, "test-holder"); - assertThat(data).containsEntry(ConfigMapLockRepository.EXPIRATION_KEY, String.valueOf(expiration)); + Optional optionalConfigMap = repository.get(NAME); + assertThat(optionalConfigMap.isPresent()).isTrue(); + + Map data = optionalConfigMap.get().getData(); + assertThat(data).containsEntry(HOLDER_KEY, HOLDER); + assertThat(data).containsEntry(EXPIRATION_KEY, String.valueOf(1000)); } @Test - public void shouldNotOverwriteConfigMap() { - boolean firstResult = repository.create("test-name", "test-holder", 0); - assertThat(firstResult).isTrue(); - - boolean secondResult = repository.create("test-name", "test-holder", 0); - assertThat(secondResult).isFalse(); + public void shouldNotOverwrite() { + assertThat(repository.create(NAME, HOLDER, 0)).isTrue(); + assertThat(repository.create(NAME, HOLDER, 0)).isFalse(); } @Test - public void shouldDeleteAllConfigMaps() { - repository.create("test-name", "test-holder", 0); - repository.create("test-name-2", "test-holder-2", 0); - repository.deleteAll(); - assertThat(repository.get("test-name")).isNull(); - assertThat(repository.get("test-name-2")).isNull(); + public void shouldDelete() { + repository.create(NAME, HOLDER, System.currentTimeMillis() + 10000); + repository.delete(NAME); + assertThat(repository.get(NAME).isPresent()).isFalse(); } @Test - public void shouldDeleteExpiredConfigMaps() { - repository.create("test-name", "test-holder", System.currentTimeMillis() - 1); - repository.create("test-name-2", "test-holder-2", System.currentTimeMillis() + 10000); - repository.deleteExpired(); - assertThat(repository.get("test-name")).isNull(); - assertThat(repository.get("test-name-2")).isNotNull(); + public void shouldDeleteExpired() { + repository.create(NAME, HOLDER, System.currentTimeMillis() - 1); + repository.deleteIfExpired(NAME); + assertThat(repository.get(NAME).isPresent()).isFalse(); } - private void deleteConfigMap(String name) { - kubernetesClient.configMaps() - .inNamespace(session.getNamespace()) - .withName(String.format("%s-%s", ConfigMapLockRepository.CONFIG_MAP_PREFIX, name)) - .delete(); + @Test + public void shouldKeepNotExpired() { + repository.create(NAME, HOLDER, System.currentTimeMillis() + 10000); + repository.deleteIfExpired(NAME); + assertThat(repository.get(NAME).isPresent()).isTrue(); } }