Reload functionality clean-up part 1 (#1030)

This commit is contained in:
erabii
2022-06-17 02:47:17 +03:00
committed by GitHub
parent bdb1cccef0
commit e3181b01fc
9 changed files with 277 additions and 223 deletions

View File

@@ -59,9 +59,9 @@ public final class Constants {
public static final String APPLICATION_PROPERTIES = "application.properties";
/**
* prefix of the configMap.
* reload mode spring property.
*/
public static final String PREFIX = "configmap";
public static final String RELOAD_MODE = "spring.cloud.kubernetes.reload.mode";
private Constants() {
}

View File

@@ -17,7 +17,6 @@
package org.springframework.cloud.kubernetes.commons.config.reload;
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -52,44 +51,26 @@ public abstract class ConfigurationChangeDetector {
public ConfigurationChangeDetector(ConfigurableEnvironment environment, ConfigReloadProperties properties,
ConfigurationUpdateStrategy strategy) {
this.environment = environment;
this.properties = properties;
this.strategy = strategy;
this.environment = Objects.requireNonNull(environment);
this.properties = Objects.requireNonNull(properties);
this.strategy = Objects.requireNonNull(strategy);
}
public void reloadProperties() {
this.log.info("Reloading using strategy: " + this.strategy.getName());
log.info("Reloading using strategy: " + this.strategy.getName());
this.strategy.reload();
}
/**
* Determines if two property sources are different.
* @param left left map property sources
* @param right right map property sources
* @return {@code true} if source has changed
*/
public boolean changed(MapPropertySource left, MapPropertySource right) {
if (left == right) {
return false;
}
if (left == null || right == null) {
return true;
}
Map<String, Object> leftMap = left.getSource();
Map<String, Object> rightMap = right.getSource();
return !Objects.equals(leftMap, rightMap);
}
public boolean changed(List<? extends MapPropertySource> left, List<? extends MapPropertySource> right) {
if (left.size() != right.size()) {
this.log.warn("The current number of ConfigMap PropertySources does not match "
log.warn("The current number of ConfigMap PropertySources does not match "
+ "the ones loaded from the Kubernetes - No reload will take place");
if (log.isDebugEnabled()) {
this.log.debug(String.format("source 1: %d", left.size()));
log.debug("left size: " + left.size());
left.forEach(item -> log.debug(item));
this.log.debug(String.format("source 2: %d", right.size()));
log.debug("right size: " + right.size());
right.forEach(item -> log.debug(item));
}
return false;
@@ -103,41 +84,22 @@ public abstract class ConfigurationChangeDetector {
return false;
}
/**
* Finds one registered property source of the given type, logging a warning if
* multiple property sources of that type are available.
* @param <S> property source type
* @param sourceClass class for which property sources will be searched for
* @return matched property source
*/
protected <S extends PropertySource<?>> S findPropertySource(Class<S> sourceClass) {
List<S> sources = findPropertySources(sourceClass);
if (sources.size() == 0) {
return null;
}
if (sources.size() > 1) {
this.log.warn("Found more than one property source of type " + sourceClass);
}
return sources.get(0);
}
/**
* @param <S> property source type
* @param sourceClass class for which property sources will be found
* @return finds all registered property sources of the given type
*/
public <S extends PropertySource<?>> List<S> findPropertySources(Class<S> sourceClass) {
List<S> managedSources = new LinkedList<>();
List<S> managedSources = new ArrayList<>();
LinkedList<PropertySource<?>> sources = toLinkedList(this.environment.getPropertySources());
this.log.debug("findPropertySources");
this.log.debug(String.format("environment: %s", this.environment));
this.log.debug(String.format("environment sources: %s", sources));
List<PropertySource<?>> sources = environment.getPropertySources().stream()
.collect(Collectors.toCollection(ArrayList::new));
log.debug("environment: " + environment);
log.debug("environment sources: " + sources);
while (!sources.isEmpty()) {
PropertySource<?> source = sources.pop();
if (source instanceof CompositePropertySource) {
CompositePropertySource comp = (CompositePropertySource) source;
PropertySource<?> source = sources.remove(0);
if (source instanceof CompositePropertySource comp) {
sources.addAll(comp.getPropertySources());
}
else if (sourceClass.isInstance(source)) {
@@ -154,14 +116,6 @@ public abstract class ConfigurationChangeDetector {
return managedSources;
}
private <E> LinkedList<E> toLinkedList(Iterable<E> it) {
LinkedList<E> list = new LinkedList<>();
for (E e : it) {
list.add(e);
}
return list;
}
/**
* Returns a list of MapPropertySource that correspond to the current state of the
* system. This only handles the PropertySource objects that are returned.
@@ -178,20 +132,39 @@ public abstract class ConfigurationChangeDetector {
if (propertySource instanceof MapPropertySource) {
result.add((MapPropertySource) propertySource);
}
else if (propertySource instanceof CompositePropertySource) {
result.addAll(((CompositePropertySource) propertySource).getPropertySources().stream()
.filter(p -> p instanceof MapPropertySource).map(p -> (MapPropertySource) p)
.collect(Collectors.toList()));
else if (propertySource instanceof CompositePropertySource source) {
List<MapPropertySource> list = source.getPropertySources().stream()
.filter(p -> p instanceof MapPropertySource).map(x -> (MapPropertySource) x)
.collect(Collectors.toList());
result.addAll(list);
}
else {
this.log.debug("Found property source that cannot be handled: " + propertySource.getClass());
log.debug("Found property source that cannot be handled: " + propertySource.getClass());
}
this.log.debug("locateMapPropertySources");
this.log.debug(String.format("environment: %s", environment));
this.log.debug(String.format("sources: %s", result));
log.debug("environment: " + environment);
log.debug("sources: " + result);
return result;
}
/**
* Determines if two property sources are different.
* @param left left map property sources
* @param right right map property sources
* @return {@code true} if source has changed
*/
boolean changed(MapPropertySource left, MapPropertySource right) {
if (left == right) {
return false;
}
if (left == null || right == null) {
return true;
}
Map<String, Object> leftMap = left.getSource();
Map<String, Object> rightMap = right.getSource();
return !Objects.equals(leftMap, rightMap);
}
}

View File

@@ -30,7 +30,7 @@ import org.springframework.scheduling.TaskScheduler;
import org.springframework.scheduling.support.PeriodicTrigger;
/**
* A change detector that periodically retrieves secrets and fire a reload when something
* A change detector that periodically retrieves secrets and fires a reload when something
* changes.
*
* @author Nicola Ferraro

View File

@@ -16,6 +16,7 @@
package org.springframework.cloud.kubernetes.commons.config.reload.condition;
import org.springframework.cloud.kubernetes.commons.config.Constants;
import org.springframework.cloud.kubernetes.commons.config.reload.ConfigReloadProperties;
import org.springframework.context.annotation.Condition;
import org.springframework.context.annotation.ConditionContext;
@@ -28,16 +29,16 @@ import org.springframework.core.type.AnnotatedTypeMetadata;
* @author Kris Iyer
*
*/
public class EventReloadDetectionMode implements Condition {
public final class EventReloadDetectionMode implements Condition {
@Override
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
Environment environment = context.getEnvironment();
if (!environment.containsProperty("spring.cloud.kubernetes.reload.mode")) {
if (!environment.containsProperty(Constants.RELOAD_MODE)) {
return true;
}
return ConfigReloadProperties.ReloadDetectionMode.EVENT.name()
.equalsIgnoreCase(context.getEnvironment().getProperty("spring.cloud.kubernetes.reload.mode"));
.equalsIgnoreCase(environment.getProperty(Constants.RELOAD_MODE));
}
}

View File

@@ -16,6 +16,7 @@
package org.springframework.cloud.kubernetes.commons.config.reload.condition;
import org.springframework.cloud.kubernetes.commons.config.Constants;
import org.springframework.cloud.kubernetes.commons.config.reload.ConfigReloadProperties;
import org.springframework.context.annotation.Condition;
import org.springframework.context.annotation.ConditionContext;
@@ -28,16 +29,16 @@ import org.springframework.core.type.AnnotatedTypeMetadata;
* @author Kris Iyer
*
*/
public class PollingReloadDetectionMode implements Condition {
public final class PollingReloadDetectionMode implements Condition {
@Override
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
Environment environment = context.getEnvironment();
if (!environment.containsProperty("spring.cloud.kubernetes.reload.mode")) {
if (!environment.containsProperty(Constants.RELOAD_MODE)) {
return false;
}
return ConfigReloadProperties.ReloadDetectionMode.POLLING.name()
.equalsIgnoreCase(context.getEnvironment().getProperty("spring.cloud.kubernetes.reload.mode"));
.equalsIgnoreCase(environment.getProperty(Constants.RELOAD_MODE));
}
}

View File

@@ -0,0 +1,213 @@
/*
* Copyright 2013-2022 the original author or 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
*
* https://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 org.springframework.cloud.kubernetes.commons.config.reload;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.springframework.cloud.bootstrap.config.BootstrapPropertySource;
import org.springframework.core.env.CompositePropertySource;
import org.springframework.core.env.EnumerablePropertySource;
import org.springframework.core.env.MapPropertySource;
import org.springframework.core.env.MutablePropertySources;
import org.springframework.core.env.PropertySource;
import org.springframework.mock.env.MockEnvironment;
import static org.assertj.core.api.Assertions.assertThat;
/**
* @author wind57
*/
class ConfigurationChangeDetectorTests {
private final ConfigurationChangeDetector changeDetector = new ConfigurationChangeDetector(new MockEnvironment(),
new ConfigReloadProperties(), new ConfigurationUpdateStrategy("some", () -> {
})) {
};
@Test
void testChangedTwoNulls() {
boolean changed = changeDetector.changed(null, (MapPropertySource) null);
assertThat(changed).isFalse();
}
@Test
void testChangedLeftNullRightNonNull() {
MapPropertySource right = new MapPropertySource("rightNonNull", Collections.emptyMap());
boolean changed = changeDetector.changed(null, right);
assertThat(changed).isTrue();
}
@Test
void testChangedLeftNonNullRightNull() {
MapPropertySource left = new MapPropertySource("leftNonNull", Collections.emptyMap());
boolean changed = changeDetector.changed(left, null);
assertThat(changed).isTrue();
}
@Test
void testChangedEqualMaps() {
Object value = new Object();
Map<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
Map<String, Object> rightMap = new HashMap<>();
rightMap.put("key", value);
MapPropertySource left = new MapPropertySource("left", leftMap);
MapPropertySource right = new MapPropertySource("right", rightMap);
boolean changed = changeDetector.changed(left, right);
assertThat(changed).isFalse();
}
@Test
void testChangedNonEqualMaps() {
Object value = new Object();
Map<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
leftMap.put("anotherKey", value);
Map<String, Object> rightMap = new HashMap<>();
rightMap.put("key", value);
MapPropertySource left = new MapPropertySource("left", leftMap);
MapPropertySource right = new MapPropertySource("right", rightMap);
boolean changed = changeDetector.changed(left, right);
assertThat(changed).isTrue();
}
@Test
void testChangedListsDifferentSizes() {
List<MapPropertySource> left = Collections.singletonList(new MapPropertySource("one", Collections.emptyMap()));
List<MapPropertySource> right = Collections.emptyList();
boolean changed = changeDetector.changed(left, right);
assertThat(changed).isFalse();
}
@Test
void testChangedListSameSizesButNotEqual() {
Object value = new Object();
Map<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
Map<String, Object> rightMap = new HashMap<>();
leftMap.put("anotherKey", value);
List<MapPropertySource> left = Collections.singletonList(new MapPropertySource("one", leftMap));
List<MapPropertySource> right = Collections.singletonList(new MapPropertySource("two", rightMap));
boolean changed = changeDetector.changed(left, right);
assertThat(changed).isTrue();
}
@Test
void testChangedListSameSizesEqual() {
Object value = new Object();
Map<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
Map<String, Object> rightMap = new HashMap<>();
leftMap.put("key", value);
List<MapPropertySource> left = Collections.singletonList(new MapPropertySource("one", leftMap));
List<MapPropertySource> right = Collections.singletonList(new MapPropertySource("two", rightMap));
boolean changed = changeDetector.changed(left, right);
assertThat(changed).isTrue();
}
@Test
void testFindPropertySources() {
MockEnvironment environment = new MockEnvironment();
ConfigurationChangeDetector detector = new ConfigurationChangeDetector(environment,
new ConfigReloadProperties(), new ConfigurationUpdateStrategy("some", () -> {
})) {
};
MutablePropertySources propertySources = environment.getPropertySources();
propertySources.addFirst(new OneComposite());
propertySources.addFirst(new PlainPropertySource("plain"));
propertySources.addFirst(new OneBootstrap(new EnumerablePropertySource<>("enumerable") {
@Override
public String[] getPropertyNames() {
return new String[0];
}
@Override
public Object getProperty(String name) {
return null;
}
}));
List<PlainPropertySource> result = detector.findPropertySources(PlainPropertySource.class);
Assertions.assertEquals(3, result.size());
Assertions.assertEquals("plain", result.get(0).getProperty(""));
Assertions.assertEquals("from-bootstrap", result.get(1).getProperty(""));
Assertions.assertEquals("from-inner-two-composite", result.get(2).getProperty(""));
}
private static final class OneComposite extends CompositePropertySource {
private OneComposite() {
super("one");
}
@Override
public Collection<PropertySource<?>> getPropertySources() {
return List.of(new TwoComposite());
}
}
private static final class TwoComposite extends CompositePropertySource {
private TwoComposite() {
super("two");
}
@Override
public Collection<PropertySource<?>> getPropertySources() {
return List.of(new PlainPropertySource("from-inner-two-composite"));
}
}
private static final class PlainPropertySource extends PropertySource<String> {
private PlainPropertySource(String name) {
super(name);
}
@Override
public Object getProperty(String name) {
return this.name;
}
}
private static final class OneBootstrap extends BootstrapPropertySource<String> {
private OneBootstrap(EnumerablePropertySource<String> delegate) {
super(delegate);
}
@Override
public PropertySource<String> getDelegate() {
return new PlainPropertySource("from-bootstrap");
}
}
}

View File

@@ -31,7 +31,7 @@ import org.springframework.core.type.AnnotatedTypeMetadata;
* @author wind57
*/
@ExtendWith(MockitoExtension.class)
public class EventReloadDetectionModeTest {
class EventReloadDetectionModeTest {
private static final String RELOAD_PROPERTY = "spring.cloud.kubernetes.reload.mode";
@@ -50,7 +50,7 @@ public class EventReloadDetectionModeTest {
// returns a "null".
// I am leaving it here just to make sure nothing breaks in branching
@Test
public void testNull() {
void testNull() {
Mockito.when(context.getEnvironment()).thenReturn(environment);
Mockito.when(environment.containsProperty(RELOAD_PROPERTY)).thenReturn(true);
Mockito.when(environment.getProperty(RELOAD_PROPERTY)).thenReturn(null);
@@ -60,7 +60,7 @@ public class EventReloadDetectionModeTest {
// lack of this property being set, means a match.
@Test
public void testDoesNotContain() {
void testDoesNotContain() {
Mockito.when(context.getEnvironment()).thenReturn(environment);
Mockito.when(environment.containsProperty(RELOAD_PROPERTY)).thenReturn(false);
boolean matches = underTest.matches(context, metadata);
@@ -68,7 +68,7 @@ public class EventReloadDetectionModeTest {
}
@Test
public void testMatchesCase() {
void testMatchesCase() {
Mockito.when(context.getEnvironment()).thenReturn(environment);
Mockito.when(environment.containsProperty(RELOAD_PROPERTY)).thenReturn(true);
Mockito.when(environment.getProperty(RELOAD_PROPERTY)).thenReturn("EVENT");
@@ -77,7 +77,7 @@ public class EventReloadDetectionModeTest {
}
@Test
public void testMatchesIgnoreCase() {
void testMatchesIgnoreCase() {
Mockito.when(context.getEnvironment()).thenReturn(environment);
Mockito.when(environment.containsProperty(RELOAD_PROPERTY)).thenReturn(true);
Mockito.when(environment.getProperty(RELOAD_PROPERTY)).thenReturn("eVeNt");
@@ -86,7 +86,7 @@ public class EventReloadDetectionModeTest {
}
@Test
public void testNoMatch() {
void testNoMatch() {
Mockito.when(context.getEnvironment()).thenReturn(environment);
Mockito.when(environment.containsProperty(RELOAD_PROPERTY)).thenReturn(true);
Mockito.when(environment.getProperty(RELOAD_PROPERTY)).thenReturn("not-eVeNt");

View File

@@ -31,7 +31,7 @@ import org.springframework.core.type.AnnotatedTypeMetadata;
* @author wind57
*/
@ExtendWith(MockitoExtension.class)
public class PollingReloadDetectionModeTest {
class PollingReloadDetectionModeTest {
private static final String RELOAD_PROPERTY = "spring.cloud.kubernetes.reload.mode";
@@ -50,7 +50,7 @@ public class PollingReloadDetectionModeTest {
// returns a "null".
// I am leaving it here just to make sure nothing breaks in branching
@Test
public void testNull() {
void testNull() {
Mockito.when(context.getEnvironment()).thenReturn(environment);
Mockito.when(environment.containsProperty(RELOAD_PROPERTY)).thenReturn(true);
Mockito.when(environment.getProperty(RELOAD_PROPERTY)).thenReturn(null);
@@ -60,7 +60,7 @@ public class PollingReloadDetectionModeTest {
// lack of this property being set, means a NO match (unlike EventReloadDetectionMode)
@Test
public void testDoesNotContain() {
void testDoesNotContain() {
Mockito.when(context.getEnvironment()).thenReturn(environment);
Mockito.when(environment.containsProperty(RELOAD_PROPERTY)).thenReturn(false);
boolean matches = underTest.matches(context, metadata);
@@ -68,7 +68,7 @@ public class PollingReloadDetectionModeTest {
}
@Test
public void testMatchesCase() {
void testMatchesCase() {
Mockito.when(context.getEnvironment()).thenReturn(environment);
Mockito.when(environment.containsProperty(RELOAD_PROPERTY)).thenReturn(true);
Mockito.when(environment.getProperty(RELOAD_PROPERTY)).thenReturn("POLLING");
@@ -77,7 +77,7 @@ public class PollingReloadDetectionModeTest {
}
@Test
public void testMatchesIgnoreCase() {
void testMatchesIgnoreCase() {
Mockito.when(context.getEnvironment()).thenReturn(environment);
Mockito.when(environment.containsProperty(RELOAD_PROPERTY)).thenReturn(true);
Mockito.when(environment.getProperty(RELOAD_PROPERTY)).thenReturn("PoLLiNG");
@@ -86,7 +86,7 @@ public class PollingReloadDetectionModeTest {
}
@Test
public void testNoMatch() {
void testNoMatch() {
Mockito.when(context.getEnvironment()).thenReturn(environment);
Mockito.when(environment.containsProperty(RELOAD_PROPERTY)).thenReturn(true);
Mockito.when(environment.getProperty(RELOAD_PROPERTY)).thenReturn("not-POLLING");

View File

@@ -1,134 +0,0 @@
/*
* Copyright 2013-2020 the original author or 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
*
* https://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 org.springframework.cloud.kubernetes.fabric8.config.reload;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.Test;
import org.springframework.cloud.kubernetes.commons.config.reload.ConfigReloadProperties;
import org.springframework.cloud.kubernetes.commons.config.reload.ConfigurationChangeDetector;
import org.springframework.cloud.kubernetes.commons.config.reload.ConfigurationUpdateStrategy;
import org.springframework.core.env.ConfigurableEnvironment;
import org.springframework.core.env.MapPropertySource;
import static org.assertj.core.api.Assertions.assertThat;
/**
* @author wind57
*/
public class ConfigurationChangeDetectorTest {
private final ConfigurationChangeDetectorStub stub = new ConfigurationChangeDetectorStub(null, null, null);
@Test
public void testChangedTwoNulls() {
boolean changed = stub.changed(null, (MapPropertySource) null);
assertThat(changed).isFalse();
}
@Test
public void testChangedLeftNullRightNonNull() {
MapPropertySource right = new MapPropertySource("rightNonNull", Collections.emptyMap());
boolean changed = stub.changed(null, right);
assertThat(changed).isTrue();
}
@Test
public void testChangedLeftNonNullRightNull() {
MapPropertySource left = new MapPropertySource("leftNonNull", Collections.emptyMap());
boolean changed = stub.changed(left, null);
assertThat(changed).isTrue();
}
@Test
public void testChangedEqualMaps() {
Object value = new Object();
Map<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
Map<String, Object> rightMap = new HashMap<>();
rightMap.put("key", value);
MapPropertySource left = new MapPropertySource("left", leftMap);
MapPropertySource right = new MapPropertySource("right", rightMap);
boolean changed = stub.changed(left, right);
assertThat(changed).isFalse();
}
@Test
public void testChangedNonEqualMaps() {
Object value = new Object();
Map<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
leftMap.put("anotherKey", value);
Map<String, Object> rightMap = new HashMap<>();
rightMap.put("key", value);
MapPropertySource left = new MapPropertySource("left", leftMap);
MapPropertySource right = new MapPropertySource("right", rightMap);
boolean changed = stub.changed(left, right);
assertThat(changed).isTrue();
}
@Test
public void testChangedListsDifferentSizes() {
List<MapPropertySource> left = Collections.singletonList(new MapPropertySource("one", Collections.emptyMap()));
List<MapPropertySource> right = Collections.emptyList();
boolean changed = stub.changed(left, right);
assertThat(changed).isFalse();
}
@Test
public void testChangedListSameSizesButNotEqual() {
Object value = new Object();
Map<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
Map<String, Object> rightMap = new HashMap<>();
leftMap.put("anotherKey", value);
List<MapPropertySource> left = Collections.singletonList(new MapPropertySource("one", leftMap));
List<MapPropertySource> right = Collections.singletonList(new MapPropertySource("two", rightMap));
boolean changed = stub.changed(left, right);
assertThat(changed).isTrue();
}
@Test
public void testChangedListSameSizesEqual() {
Object value = new Object();
Map<String, Object> leftMap = new HashMap<>();
leftMap.put("key", value);
Map<String, Object> rightMap = new HashMap<>();
leftMap.put("key", value);
List<MapPropertySource> left = Collections.singletonList(new MapPropertySource("one", leftMap));
List<MapPropertySource> right = Collections.singletonList(new MapPropertySource("two", rightMap));
boolean changed = stub.changed(left, right);
assertThat(changed).isTrue();
}
/**
* only needed to test some protected methods it defines
*/
private static final class ConfigurationChangeDetectorStub extends ConfigurationChangeDetector {
private ConfigurationChangeDetectorStub(ConfigurableEnvironment environment, ConfigReloadProperties properties,
ConfigurationUpdateStrategy strategy) {
super(environment, properties, strategy);
}
}
}