From 19de744dcf7785bc5300233d97b49d1dc364fdd1 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Thu, 27 Feb 2020 10:25:15 -0500 Subject: [PATCH 1/2] Take into account BootstrapPropertySource when listening to configuration changes. Fixes #524 --- .../config/reload/ConfigurationChangeDetector.java | 13 ++++++++++--- .../discovery/KubernetesServiceInstanceTests.java | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/ConfigurationChangeDetector.java b/spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/ConfigurationChangeDetector.java index e23c2df302..2b6bace656 100644 --- a/spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/ConfigurationChangeDetector.java +++ b/spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/ConfigurationChangeDetector.java @@ -28,6 +28,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.cloud.bootstrap.config.BootstrapPropertySource; import org.springframework.cloud.bootstrap.config.PropertySourceLocator; import org.springframework.core.env.CompositePropertySource; import org.springframework.core.env.ConfigurableEnvironment; @@ -98,9 +99,8 @@ protected boolean changed(List l1, List l2) { if (l1.size() != l2.size()) { - this.log.debug( - "The current number of Confimap PropertySources does not match " - + "the ones loaded from the Kubernetes - No reload will take place"); + this.log.warn("The current number of Confimap PropertySources does not match " + + "the ones loaded from the Kubernetes - No reload will take place"); return false; } @@ -150,6 +150,13 @@ protected > List findPropertySources( else if (sourceClass.isInstance(source)) { managedSources.add(sourceClass.cast(source)); } + else if (BootstrapPropertySource.class.isInstance(source)) { + PropertySource propertySource = ((BootstrapPropertySource) source) + .getDelegate(); + if (sourceClass.isInstance(propertySource)) { + sources.add(propertySource); + } + } } return managedSources; diff --git a/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesServiceInstanceTests.java b/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesServiceInstanceTests.java index 4ade0af1d8..eb3738e562 100644 --- a/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesServiceInstanceTests.java +++ b/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesServiceInstanceTests.java @@ -37,7 +37,7 @@ private KubernetesServiceInstance assertServiceInstance(boolean secure) { EndpointPort port = new EndpointPort(); port.setPort(8080); KubernetesServiceInstance instance = new KubernetesServiceInstance("123", - "myservice", address, port, Collections.emptyMap(), secure); + "myservice", address, port, Collections.emptyMap(), secure); assertThat(instance.getInstanceId()).isEqualTo("123"); assertThat(instance.getServiceId()).isEqualTo("myservice"); From 912b33d14d81e582fd7e8da7d4e29de10a24ad0c Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Thu, 27 Feb 2020 12:47:25 -0500 Subject: [PATCH 2/2] Adding test --- ...BasedConfigurationChangeDetectorTests.java | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 spring-cloud-kubernetes-config/src/test/java/org/springframework/cloud/kubernetes/config/reload/EventBasedConfigurationChangeDetectorTests.java diff --git a/spring-cloud-kubernetes-config/src/test/java/org/springframework/cloud/kubernetes/config/reload/EventBasedConfigurationChangeDetectorTests.java b/spring-cloud-kubernetes-config/src/test/java/org/springframework/cloud/kubernetes/config/reload/EventBasedConfigurationChangeDetectorTests.java new file mode 100644 index 0000000000..d96962baeb --- /dev/null +++ b/spring-cloud-kubernetes-config/src/test/java/org/springframework/cloud/kubernetes/config/reload/EventBasedConfigurationChangeDetectorTests.java @@ -0,0 +1,83 @@ +/* + * 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.config.reload; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapList; +import io.fabric8.kubernetes.api.model.DoneableConfigMap; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.dsl.MixedOperation; +import io.fabric8.kubernetes.client.dsl.Resource; +import org.junit.Test; + +import org.springframework.cloud.bootstrap.config.BootstrapPropertySource; +import org.springframework.cloud.kubernetes.config.ConfigMapPropertySource; +import org.springframework.cloud.kubernetes.config.ConfigMapPropertySourceLocator; +import org.springframework.cloud.kubernetes.config.SecretsPropertySourceLocator; +import org.springframework.mock.env.MockEnvironment; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * @author Ryan Baxter + */ +public class EventBasedConfigurationChangeDetectorTests { + + @Test + public void verifyConfigChangesAccountsForBootstrapPropertySources() { + ConfigReloadProperties configReloadProperties = new ConfigReloadProperties(); + MockEnvironment env = new MockEnvironment(); + KubernetesClient k8sClient = mock(KubernetesClient.class); + ConfigMap configMap = new ConfigMap(); + Map data = new HashMap(); + data.put("foo", "bar"); + configMap.setData(data); + MixedOperation> mixedOperation = mock( + MixedOperation.class); + Resource resource = mock(Resource.class); + when(resource.get()).thenReturn(configMap); + when(mixedOperation.withName(eq("myconfigmap"))).thenReturn(resource); + when(k8sClient.configMaps()).thenReturn(mixedOperation); + + ConfigMapPropertySource configMapPropertySource = new ConfigMapPropertySource( + k8sClient, "myconfigmap"); + env.getPropertySources() + .addFirst(new BootstrapPropertySource(configMapPropertySource)); + + ConfigurationUpdateStrategy configurationUpdateStrategy = mock( + ConfigurationUpdateStrategy.class); + ConfigMapPropertySourceLocator configMapLocator = mock( + ConfigMapPropertySourceLocator.class); + SecretsPropertySourceLocator secretsLocator = mock( + SecretsPropertySourceLocator.class); + EventBasedConfigurationChangeDetector detector = new EventBasedConfigurationChangeDetector( + env, configReloadProperties, k8sClient, configurationUpdateStrategy, + configMapLocator, secretsLocator); + List sources = detector + .findPropertySources(ConfigMapPropertySource.class); + assertThat(sources.size()).isEqualTo(1); + assertThat(sources.get(0).getProperty("foo")).isEqualTo("bar"); + } + +}