Skip to content

Commit a4ad5eb

Browse files
krisiyekrisiye
authored
Refactor ConfigurationChangeDetector to simplify conditionals. Issue 643 (#644)
* Refactor configuration change detector into configMap and Secret change detectors each for reload detection mode event/polling. Fixes #643 * Add additional tests * Refactor spring-cloud-kubernetes-configuration-watcher to decouple configMap and Secrets for HTTP and Bus implementations * fix logger * fix checkstyle errors.
1 parent 6674497 commit a4ad5eb

21 files changed

+1079
-289
lines changed

‎spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/ConfigReloadAutoConfiguration.java

Lines changed: 63 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.springframework.boot.actuate.autoconfigure.endpoint.EndpointAutoConfiguration;
2525
import org.springframework.boot.actuate.autoconfigure.info.InfoEndpointAutoConfiguration;
2626
import org.springframework.boot.autoconfigure.AutoConfigureAfter;
27-
import org.springframework.boot.autoconfigure.condition.AnyNestedCondition;
2827
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
2928
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
3029
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
@@ -36,6 +35,8 @@
3635
import org.springframework.cloud.context.restart.RestartEndpoint;
3736
import org.springframework.cloud.kubernetes.config.ConfigMapPropertySourceLocator;
3837
import org.springframework.cloud.kubernetes.config.SecretsPropertySourceLocator;
38+
import org.springframework.cloud.kubernetes.config.reload.condition.EventReloadDetectionMode;
39+
import org.springframework.cloud.kubernetes.config.reload.condition.PollingReloadDetectionMode;
3940
import org.springframework.context.ConfigurableApplicationContext;
4041
import org.springframework.context.annotation.Bean;
4142
import org.springframework.context.annotation.Conditional;
@@ -49,14 +50,14 @@
4950
* Definition of beans needed for the automatic reload of configuration.
5051
*
5152
* @author Nicolla Ferraro
53+
* @author Kris Iyer
5254
*/
5355
@Configuration(proxyBeanMethods = false)
5456
@ConditionalOnProperty(value = "spring.cloud.kubernetes.enabled", matchIfMissing = true)
5557
@ConditionalOnClass(EndpointAutoConfiguration.class)
5658
@AutoConfigureAfter({ InfoEndpointAutoConfiguration.class, RefreshEndpointAutoConfiguration.class,
5759
RefreshAutoConfiguration.class })
5860
@EnableConfigurationProperties(ConfigReloadProperties.class)
59-
6061
public class ConfigReloadAutoConfiguration {
6162

6263
/**
@@ -75,25 +76,71 @@ protected static class ConfigReloadAutoConfigurationBeans {
7576
private KubernetesClient kubernetesClient;
7677

7778
/**
79+
* Polling configMap ConfigurationChangeDetector.
7880
* @param properties config reload properties
7981
* @param strategy configuration update strategy
82+
* @param configMapPropertySourceLocator configMap property source locator
8083
* @return a bean that listen to configuration changes and fire a reload.
8184
*/
8285
@Bean
83-
@Conditional(OnConfigEnabledOrSecretsEnabled.class)
84-
public ConfigurationChangeDetector propertyChangeWatcher(ConfigReloadProperties properties,
85-
ConfigurationUpdateStrategy strategy,
86-
@Autowired(required = false) ConfigMapPropertySourceLocator configMapPropertySourceLocator,
87-
@Autowired(required = false) SecretsPropertySourceLocator secretsPropertySourceLocator) {
88-
switch (properties.getMode()) {
89-
case POLLING:
90-
return new PollingConfigurationChangeDetector(this.environment, properties, this.kubernetesClient,
91-
strategy, configMapPropertySourceLocator, secretsPropertySourceLocator);
92-
case EVENT:
93-
return new EventBasedConfigurationChangeDetector(this.environment, properties, this.kubernetesClient,
94-
strategy, configMapPropertySourceLocator, secretsPropertySourceLocator);
95-
}
96-
throw new IllegalStateException("Unsupported configuration reload mode: " + properties.getMode());
86+
@ConditionalOnBean(ConfigMapPropertySourceLocator.class)
87+
@Conditional(PollingReloadDetectionMode.class)
88+
public ConfigurationChangeDetector configMapPropertyChangePollingWatcher(ConfigReloadProperties properties,
89+
ConfigurationUpdateStrategy strategy, ConfigMapPropertySourceLocator configMapPropertySourceLocator) {
90+
91+
return new PollingConfigMapChangeDetector(this.environment, properties, this.kubernetesClient, strategy,
92+
configMapPropertySourceLocator);
93+
}
94+
95+
/**
96+
* Polling secrets ConfigurationChangeDetector.
97+
* @param properties config reload properties
98+
* @param strategy configuration update strategy
99+
* @param secretsPropertySourceLocator secrets property source locator
100+
* @return a bean that listen to configuration changes and fire a reload.
101+
*/
102+
@Bean
103+
@ConditionalOnBean(SecretsPropertySourceLocator.class)
104+
@Conditional(PollingReloadDetectionMode.class)
105+
public ConfigurationChangeDetector secretsPropertyChangePollingWatcher(ConfigReloadProperties properties,
106+
ConfigurationUpdateStrategy strategy, SecretsPropertySourceLocator secretsPropertySourceLocator) {
107+
108+
return new PollingSecretsChangeDetector(this.environment, properties, this.kubernetesClient, strategy,
109+
secretsPropertySourceLocator);
110+
}
111+
112+
/**
113+
* Event Based configMap ConfigurationChangeDetector.
114+
* @param properties config reload properties
115+
* @param strategy configuration update strategy
116+
* @param configMapPropertySourceLocator configMap property source locator
117+
* @return a bean that listen to configMap change events and fire a reload.
118+
*/
119+
@Bean
120+
@ConditionalOnBean(ConfigMapPropertySourceLocator.class)
121+
@Conditional(EventReloadDetectionMode.class)
122+
public ConfigurationChangeDetector configMapPropertyChangeEventWatcher(ConfigReloadProperties properties,
123+
ConfigurationUpdateStrategy strategy, ConfigMapPropertySourceLocator configMapPropertySourceLocator) {
124+
125+
return new EventBasedConfigMapChangeDetector(this.environment, properties, this.kubernetesClient, strategy,
126+
configMapPropertySourceLocator);
127+
}
128+
129+
/**
130+
* Event Based secrets ConfigurationChangeDetector.
131+
* @param properties config reload properties
132+
* @param strategy configuration update strategy
133+
* @param secretsPropertySourceLocator secrets property source locator
134+
* @return a bean that listen to secrets change events and fire a reload.
135+
*/
136+
@Bean
137+
@ConditionalOnBean(SecretsPropertySourceLocator.class)
138+
@Conditional(EventReloadDetectionMode.class)
139+
public ConfigurationChangeDetector secretsPropertyChangeEventWatcher(ConfigReloadProperties properties,
140+
ConfigurationUpdateStrategy strategy, SecretsPropertySourceLocator secretsPropertySourceLocator) {
141+
142+
return new EventBasedSecretsChangeDetector(this.environment, properties, this.kubernetesClient, strategy,
143+
secretsPropertySourceLocator);
97144
}
98145

99146
/**
@@ -135,24 +182,6 @@ private static void wait(ConfigReloadProperties properties) {
135182
}
136183
}
137184

138-
private static class OnConfigEnabledOrSecretsEnabled extends AnyNestedCondition {
139-
140-
OnConfigEnabledOrSecretsEnabled() {
141-
super(ConfigurationPhase.REGISTER_BEAN);
142-
}
143-
144-
@ConditionalOnBean(ConfigMapPropertySourceLocator.class)
145-
static class configEnabled {
146-
147-
}
148-
149-
@ConditionalOnBean(SecretsPropertySourceLocator.class)
150-
static class secretsEnabled {
151-
152-
}
153-
154-
}
155-
156185
}
157186

158187
}

‎spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/ConfigReloadDefaultAutoConfiguration.java

Lines changed: 65 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,25 @@
2121
import io.fabric8.kubernetes.client.KubernetesClient;
2222

2323
import org.springframework.beans.factory.annotation.Autowired;
24+
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
2425
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
2526
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
2627
import org.springframework.boot.context.properties.EnableConfigurationProperties;
2728
import org.springframework.cloud.kubernetes.config.ConfigMapPropertySourceLocator;
2829
import org.springframework.cloud.kubernetes.config.SecretsPropertySourceLocator;
30+
import org.springframework.cloud.kubernetes.config.reload.condition.EventReloadDetectionMode;
31+
import org.springframework.cloud.kubernetes.config.reload.condition.PollingReloadDetectionMode;
2932
import org.springframework.context.ConfigurableApplicationContext;
3033
import org.springframework.context.annotation.Bean;
34+
import org.springframework.context.annotation.Conditional;
3135
import org.springframework.context.annotation.Configuration;
3236
import org.springframework.core.env.AbstractEnvironment;
3337
import org.springframework.scheduling.annotation.EnableAsync;
3438
import org.springframework.scheduling.annotation.EnableScheduling;
3539

3640
/**
3741
* @author Ryan Baxter
42+
* @author Kris Iyer
3843
*/
3944
@Configuration(proxyBeanMethods = false)
4045
@ConditionalOnProperty(value = "spring.cloud.kubernetes.enabled", matchIfMissing = true)
@@ -63,30 +68,76 @@ protected static class ConfigReloadAutoConfigurationBeans {
6368
private SecretsPropertySourceLocator secretsPropertySourceLocator;
6469

6570
/**
71+
* Polling configMap ConfigurationChangeDetector.
6672
* @param properties config reload properties
6773
* @param strategy configuration update strategy
74+
* @param configMapPropertySourceLocator configMap property source locator
6875
* @return a bean that listen to configuration changes and fire a reload.
6976
*/
7077
@Bean
71-
@ConditionalOnMissingBean
72-
public ConfigurationChangeDetector propertyChangeWatcher(ConfigReloadProperties properties,
73-
ConfigurationUpdateStrategy strategy) {
74-
switch (properties.getMode()) {
75-
case POLLING:
76-
return new PollingConfigurationChangeDetector(this.environment, properties, this.kubernetesClient,
77-
strategy, this.configMapPropertySourceLocator, this.secretsPropertySourceLocator);
78-
case EVENT:
79-
return new EventBasedConfigurationChangeDetector(this.environment, properties, this.kubernetesClient,
80-
strategy, this.configMapPropertySourceLocator, this.secretsPropertySourceLocator);
81-
}
82-
throw new IllegalStateException("Unsupported configuration reload mode: " + properties.getMode());
78+
@ConditionalOnBean(ConfigMapPropertySourceLocator.class)
79+
@Conditional(PollingReloadDetectionMode.class)
80+
public ConfigurationChangeDetector configMapPropertyChangePollingWatcher(ConfigReloadProperties properties,
81+
ConfigurationUpdateStrategy strategy, ConfigMapPropertySourceLocator configMapPropertySourceLocator) {
82+
83+
return new PollingConfigMapChangeDetector(this.environment, properties, this.kubernetesClient, strategy,
84+
configMapPropertySourceLocator);
85+
}
86+
87+
/**
88+
* Polling secrets ConfigurationChangeDetector.
89+
* @param properties config reload properties
90+
* @param strategy configuration update strategy
91+
* @param secretsPropertySourceLocator secrets property source locator
92+
* @return a bean that listen to configuration changes and fire a reload.
93+
*/
94+
@Bean
95+
@ConditionalOnBean(SecretsPropertySourceLocator.class)
96+
@Conditional(PollingReloadDetectionMode.class)
97+
public ConfigurationChangeDetector secretsPropertyChangePollingWatcher(ConfigReloadProperties properties,
98+
ConfigurationUpdateStrategy strategy, SecretsPropertySourceLocator secretsPropertySourceLocator) {
99+
100+
return new PollingSecretsChangeDetector(this.environment, properties, this.kubernetesClient, strategy,
101+
secretsPropertySourceLocator);
102+
}
103+
104+
/**
105+
* Event Based configMap ConfigurationChangeDetector.
106+
* @param properties config reload properties
107+
* @param strategy configuration update strategy
108+
* @param configMapPropertySourceLocator configMap property source locator
109+
* @return a bean that listen to configMap change events and fire a reload.
110+
*/
111+
@Bean
112+
@ConditionalOnBean(ConfigMapPropertySourceLocator.class)
113+
@Conditional(EventReloadDetectionMode.class)
114+
public ConfigurationChangeDetector configMapPropertyChangeEventWatcher(ConfigReloadProperties properties,
115+
ConfigurationUpdateStrategy strategy, ConfigMapPropertySourceLocator configMapPropertySourceLocator) {
116+
117+
return new EventBasedConfigMapChangeDetector(this.environment, properties, this.kubernetesClient, strategy,
118+
configMapPropertySourceLocator);
119+
}
120+
121+
/**
122+
* Event Based secrets ConfigurationChangeDetector.
123+
* @param properties config reload properties
124+
* @param strategy configuration update strategy
125+
* @param secretsPropertySourceLocator secrets property source locator
126+
* @return a bean that listen to secrets change events and fire a reload.
127+
*/
128+
@Bean
129+
@ConditionalOnBean(SecretsPropertySourceLocator.class)
130+
@Conditional(EventReloadDetectionMode.class)
131+
public ConfigurationChangeDetector secretsPropertyChangeEventWatcher(ConfigReloadProperties properties,
132+
ConfigurationUpdateStrategy strategy, SecretsPropertySourceLocator secretsPropertySourceLocator) {
133+
134+
return new EventBasedSecretsChangeDetector(this.environment, properties, this.kubernetesClient, strategy,
135+
secretsPropertySourceLocator);
83136
}
84137

85138
/**
86139
* @param properties config reload properties
87140
* @param ctx application context
88-
* @param restarter restart endpoint
89-
* @param refresher context refresher
90141
* @return provides the action to execute when the configuration changes.
91142
*/
92143
@Bean

‎spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/EventBasedConfigurationChangeDetector.java renamed to ‎spring-cloud-kubernetes-config/src/main/java/org/springframework/cloud/kubernetes/config/reload/EventBasedConfigMapChangeDetector.java

Lines changed: 8 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -23,51 +23,45 @@
2323
import javax.annotation.PreDestroy;
2424

2525
import io.fabric8.kubernetes.api.model.ConfigMap;
26-
import io.fabric8.kubernetes.api.model.Secret;
2726
import io.fabric8.kubernetes.client.KubernetesClient;
2827
import io.fabric8.kubernetes.client.KubernetesClientException;
2928
import io.fabric8.kubernetes.client.Watch;
3029
import io.fabric8.kubernetes.client.Watcher;
3130

3231
import org.springframework.cloud.kubernetes.config.ConfigMapPropertySource;
3332
import org.springframework.cloud.kubernetes.config.ConfigMapPropertySourceLocator;
34-
import org.springframework.cloud.kubernetes.config.SecretsPropertySource;
35-
import org.springframework.cloud.kubernetes.config.SecretsPropertySourceLocator;
3633
import org.springframework.core.env.AbstractEnvironment;
3734

3835
/**
39-
* A change detector that subscribes to changes in secrets and configmaps and fire a
36+
* An Event Based change detector that subscribes to changes in configMaps and fire a
4037
* reload when something changes.
4138
*
4239
* @author Nicola Ferraro
4340
* @author Haytham Mohamed
41+
* @author Kris Iyer
4442
*/
45-
public class EventBasedConfigurationChangeDetector extends ConfigurationChangeDetector {
43+
public class EventBasedConfigMapChangeDetector extends ConfigurationChangeDetector {
4644

4745
private final ConfigMapPropertySourceLocator configMapPropertySourceLocator;
4846

49-
private final SecretsPropertySourceLocator secretsPropertySourceLocator;
50-
5147
private final Map<String, Watch> watches;
5248

53-
public EventBasedConfigurationChangeDetector(AbstractEnvironment environment, ConfigReloadProperties properties,
49+
public EventBasedConfigMapChangeDetector(AbstractEnvironment environment, ConfigReloadProperties properties,
5450
KubernetesClient kubernetesClient, ConfigurationUpdateStrategy strategy,
55-
ConfigMapPropertySourceLocator configMapPropertySourceLocator,
56-
SecretsPropertySourceLocator secretsPropertySourceLocator) {
51+
ConfigMapPropertySourceLocator configMapPropertySourceLocator) {
5752
super(environment, properties, kubernetesClient, strategy);
5853

5954
this.configMapPropertySourceLocator = configMapPropertySourceLocator;
60-
this.secretsPropertySourceLocator = secretsPropertySourceLocator;
6155
this.watches = new HashMap<>();
6256
}
6357

6458
@PostConstruct
6559
public void watch() {
6660
boolean activated = false;
6761

68-
if (this.properties.isMonitoringConfigMaps() && this.configMapPropertySourceLocator != null) {
62+
if (this.properties.isMonitoringConfigMaps()) {
6963
try {
70-
String name = "config-maps-watch";
64+
String name = "config-maps-watch-event";
7165
this.watches.put(name, this.kubernetesClient.configMaps().watch(new Watcher<ConfigMap>() {
7266
@Override
7367
public void eventReceived(Action action, ConfigMap configMap) {
@@ -91,34 +85,8 @@ public void onClose(KubernetesClientException e) {
9185
}
9286
}
9387

94-
if (this.properties.isMonitoringSecrets() && this.secretsPropertySourceLocator != null) {
95-
try {
96-
activated = false;
97-
String name = "secrets-watch";
98-
this.watches.put(name, this.kubernetesClient.secrets().watch(new Watcher<Secret>() {
99-
@Override
100-
public void eventReceived(Action action, Secret secret) {
101-
if (log.isDebugEnabled()) {
102-
log.debug(name + " received and event for Secret " + secret.getMetadata().getName());
103-
}
104-
onEvent(secret);
105-
}
106-
107-
@Override
108-
public void onClose(KubernetesClientException e) {
109-
}
110-
}));
111-
activated = true;
112-
this.log.info("Added new Kubernetes watch: " + name);
113-
}
114-
catch (Exception e) {
115-
this.log.error("Error while establishing a connection to watch secrets: configuration may remain stale",
116-
e);
117-
}
118-
}
119-
12088
if (activated) {
121-
this.log.info("Kubernetes event-based configuration change detector activated");
89+
this.log.info("Kubernetes event-based configMap change detector activated");
12290
}
12391
}
12492

@@ -147,13 +115,4 @@ protected void onEvent(ConfigMap configMap) {
147115
}
148116
}
149117

150-
protected void onEvent(Secret secret) {
151-
boolean changed = changed(locateMapPropertySources(this.secretsPropertySourceLocator, this.environment),
152-
findPropertySources(SecretsPropertySource.class));
153-
if (changed) {
154-
this.log.info("Detected change in secrets");
155-
reloadProperties();
156-
}
157-
}
158-
159118
}

0 commit comments

Comments
 (0)