Skip to content

Commit

Permalink
Fix 1310 for ConfigMaps (#1332)
Browse files Browse the repository at this point in the history
  • Loading branch information
wind57 authored May 2, 2023
1 parent 2917504 commit 944c7d7
Show file tree
Hide file tree
Showing 30 changed files with 1,258 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,7 @@ void equalsOne() {
}

/**
* - left is empty map
* - right is null
* - left is empty map - right is null
*
* treat as equal, that is: no change
*/
Expand All @@ -198,8 +197,7 @@ void equalsTwo() {
}

/**
* - left is empty map
* - right is null
* - left is empty map - right is null
*
* treat as equal, that is: no change
*/
Expand All @@ -213,8 +211,7 @@ void equalsThree() {
}

/**
* - left is null
* - right is empty map
* - left is null - right is empty map
*
* treat as equal, that is: no change
*/
Expand All @@ -228,8 +225,7 @@ void equalsFour() {
}

/**
* - left is empty map
* - right is empty map
* - left is empty map - right is empty map
*
* treat as equal, that is: no change
*/
Expand All @@ -243,8 +239,7 @@ void equalsFive() {
}

/**
* - left is empty map
* - right is [1, b]
* - left is empty map - right is [1, b]
*
* treat as non-equal, that is change
*/
Expand All @@ -258,8 +253,7 @@ void equalsSix() {
}

/**
* - left is [1, a]
* - right is [1, b]
* - left is [1, a] - right is [1, b]
*
* treat as non-equal, that is change
*/
Expand All @@ -273,8 +267,7 @@ void equalsSeven() {
}

/**
* - left is [1, a, 2 aa]
* - right is [1, b, 2, aa]
* - left is [1, a, 2 aa] - right is [1, b, 2, aa]
*
* treat as non-equal, that is change
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public Collection<PropertySource<?>> locateCollection(Environment environment) {

private void addPropertySourcesFromPaths(Environment environment, CompositePropertySource composite) {
Set<String> uniquePaths = new LinkedHashSet<>(properties.paths());
LOG.debug("paths property sources : " + uniquePaths);
uniquePaths.stream().map(Paths::get).filter(p -> {
boolean exists = Files.exists(p);
if (!exists) {
Expand Down Expand Up @@ -139,7 +140,8 @@ private void addPropertySourceIfNeeded(Function<String, Map<String, Object>> con
LOG.warn("Property source: " + name + "will be ignored because no properties could be found");
}
else {
composite.addFirstPropertySource(new MapPropertySource(name, map));
LOG.debug("will add file-based property source : " + name);
composite.addFirstPropertySource(new MountConfigMapPropertySource(name, map));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@

import org.springframework.core.env.MapPropertySource;

/**
* @author wind57
*/
public final class MountConfigMapPropertySource extends MapPropertySource {

public MountConfigMapPropertySource(String name, Map<String, Object> source) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import org.springframework.cloud.bootstrap.config.BootstrapPropertySource;
import org.springframework.cloud.bootstrap.config.PropertySourceLocator;
import org.springframework.cloud.kubernetes.commons.config.MountConfigMapPropertySource;
import org.springframework.core.env.CompositePropertySource;
import org.springframework.core.env.ConfigurableEnvironment;
import org.springframework.core.env.MapPropertySource;
Expand Down Expand Up @@ -73,8 +74,8 @@ public static <S extends PropertySource<?>> List<S> findPropertySources(Class<S>

List<PropertySource<?>> sources = environment.getPropertySources().stream()
.collect(Collectors.toCollection(ArrayList::new));
LOG.debug(() -> "environment: " + environment);
LOG.debug(() -> "environment sources: " + sources);
LOG.debug(() -> "environment from findPropertySources: " + environment);
LOG.debug(() -> "environment sources from findPropertySources : " + sources);

while (!sources.isEmpty()) {
PropertySource<?> source = sources.remove(0);
Expand All @@ -84,14 +85,24 @@ public static <S extends PropertySource<?>> List<S> findPropertySources(Class<S>
else if (sourceClass.isInstance(source)) {
managedSources.add(sourceClass.cast(source));
}
else if (source instanceof MountConfigMapPropertySource mountConfigMapPropertySource) {
// we know that the type is correct here
managedSources.add((S) mountConfigMapPropertySource);
}
else if (source instanceof BootstrapPropertySource<?> bootstrapPropertySource) {
PropertySource<?> propertySource = bootstrapPropertySource.getDelegate();
LOG.debug(() -> "bootstrap delegate class : " + propertySource.getClass());
if (sourceClass.isInstance(propertySource)) {
sources.add(propertySource);
}
else if (propertySource instanceof MountConfigMapPropertySource mountConfigMapPropertySource) {
// we know that the type is correct here
managedSources.add((S) mountConfigMapPropertySource);
}
}
}

LOG.debug(() -> "findPropertySources : " + managedSources.stream().map(PropertySource::getName).toList());
return managedSources;
}

Expand Down Expand Up @@ -123,32 +134,35 @@ else if (propertySource instanceof CompositePropertySource source) {
LOG.debug(() -> "Found property source that cannot be handled: " + propertySource.getClass());
}

LOG.debug(() -> "environment: " + environment);
LOG.debug(() -> "sources: " + result);
LOG.debug(() -> "environment from locateMapPropertySources : " + environment);
LOG.debug(() -> "sources from locateMapPropertySources : " + result);

return result;
}

static boolean changed(List<? extends MapPropertySource> left, List<? extends MapPropertySource> right) {
if (left.size() != right.size()) {
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()) {
LOG.debug("left size: " + left.size());
left.forEach(item -> LOG.debug(item.toString()));

LOG.debug("right size: " + right.size());
right.forEach(item -> LOG.debug(item.toString()));
}
LOG.warn(() -> "The current number of ConfigMap PropertySources does not match "
+ "the ones loaded from Kubernetes - No reload will take place");
return false;
}

for (int i = 0; i < left.size(); i++) {
if (changed(left.get(i), right.get(i))) {
MapPropertySource leftPropertySource = left.get(i);
MapPropertySource rightPropertySource = right.get(i);
if (changed(leftPropertySource, rightPropertySource)) {
LOG.debug(() -> "found change in : " + leftPropertySource);
return true;
}
}
LOG.debug(() -> "no changes found, reload will not happen");
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.junit.jupiter.api.Test;

import org.springframework.cloud.bootstrap.config.BootstrapPropertySource;
import org.springframework.cloud.kubernetes.commons.config.MountConfigMapPropertySource;
import org.springframework.core.env.CompositePropertySource;
import org.springframework.core.env.EnumerablePropertySource;
import org.springframework.core.env.MapPropertySource;
Expand Down Expand Up @@ -138,12 +139,15 @@ public Object getProperty(String name) {
return null;
}
}));

List<PlainPropertySource> result = ConfigReloadUtil.findPropertySources(PlainPropertySource.class, environment);
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(""));
propertySources.addFirst(new MountConfigMapPropertySource("mounted", Map.of("a", "b")));

List<? extends PropertySource> result = ConfigReloadUtil.findPropertySources(PlainPropertySource.class,
environment);
Assertions.assertEquals(4, result.size());
Assertions.assertEquals("b", result.get(0).getProperty("a"));
Assertions.assertEquals("plain", result.get(1).getProperty(""));
Assertions.assertEquals("from-bootstrap", result.get(2).getProperty(""));
Assertions.assertEquals("from-inner-two-composite", result.get(3).getProperty(""));
}

private static final class OneComposite extends CompositePropertySource {
Expand Down
3 changes: 2 additions & 1 deletion spring-cloud-kubernetes-integration-tests/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,6 @@
<module>spring-cloud-kubernetes-client-catalog-watcher</module>
<module>spring-cloud-kubernetes-client-discovery-it</module>
<module>spring-cloud-kubernetes-fabric8-client-discovery-with-bootstrap</module>
</modules>
<module>spring-cloud-kubernetes-client-configmap-polling-reload</module>
</modules>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-kubernetes-integration-tests</artifactId>
<version>3.0.3-SNAPSHOT</version>
</parent>

<artifactId>spring-cloud-kubernetes-client-configmap-polling-reload</artifactId>

<dependencies>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-starter-kubernetes-client-config</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-webflux</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-actuator</artifactId>
</dependency>

<!-- not bootstrap starter, because we want to be able to disable bootstrap per-test -->
<!-- and ConditionalOnBootstrapEnabled has a @ConditionalOnClass on a Marker -->
<!-- that is present in bootstrap-starter, but not in this one -->
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-starter</artifactId>
</dependency>

<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-kubernetes-test-support</artifactId>
</dependency>

</dependencies>

<build>
<resources>
<resource>
<directory>../src/main/resources</directory>
<filtering>true</filtering>
</resource>
<resource>
<directory>src/main/resources</directory>
<filtering>true</filtering>
</resource>
</resources>

<plugins>
<!-- build image in the 'package' phase, and ignore plain tests -->
<!-- via maven-surefire-plugin::skipTests -->
<plugin>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-maven-plugin</artifactId>
<configuration>
<imageName>docker.io/springcloud/${project.artifactId}:${project.version}</imageName>
<imageBuilder>paketobuildpacks/builder</imageBuilder>
</configuration>
<executions>
<execution>
<id>build-image</id>
<configuration>
<skip>${skip.build.image}</skip>
</configuration>
<phase>package</phase>
<goals>
<goal>build-image</goal>
</goals>
</execution>
<execution>
<id>repackage</id>
<phase>package</phase>
<goals>
<goal>repackage</goal>
</goals>
</execution>
</executions>
</plugin>

<!-- ignore plain tests (in the 'test' phase), so that we could build the image first, see above -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<skipTests>true</skipTests>
</configuration>
</plugin>

<!-- run tests in the 'integration-tests' phase, one that is after 'package' (where we build the image) -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-failsafe-plugin</artifactId>
<executions>
<execution>
<goals>
<goal>integration-test</goal>
</goals>
</execution>
</executions>
<configuration>
<includes>
<include>${testsToRun}</include>
</includes>
</configuration>
</plugin>
</plugins>

</build>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright 2013-2023 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.client.configmap.polling.reload;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.context.properties.EnableConfigurationProperties;

/**
* @author wind57
*/

@SpringBootApplication
@EnableConfigurationProperties(ConfigMapProperties.class)
public class ConfigMapApp {

public static void main(String[] args) {
SpringApplication.run(ConfigMapApp.class, args);
}

}
Loading

0 comments on commit 944c7d7

Please sign in to comment.