Fix Zookeeper health endpoint bean registration (#185)

There was a bug in the evaluation order for ZookeeperHealthIndicator bean where its conditions for creation,
particularly whether there was a CuratorFramework bean, were evaluated too early causing it to always fail.

This commit moves the inner ZookeeperAutoConfiguration.ZookeeperHealthConfig class into its own public class
ZookeeperHealthAutoConfiguration and makes sure its configured as auto configuration after ZookeeperAutoConfiguration.

This seems to resolve the order problem in testing.

Unit tests added for conditions on ZookeeperHealthIndicator bean.

Minor change to ZookeeperHealthIndicator to only call curator getState method once rather than twice.
This commit is contained in:
Tom Gianos
2018-12-07 11:00:08 -08:00
committed by Spencer Gibb
parent ecfb4aa08d
commit df6d748cd3
5 changed files with 131 additions and 19 deletions

View File

@@ -24,10 +24,6 @@ import org.apache.curator.framework.CuratorFramework;
import org.apache.curator.framework.CuratorFrameworkFactory;
import org.apache.curator.retry.ExponentialBackoffRetry;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.actuate.autoconfigure.health.ConditionalOnEnabledHealthIndicator;
import org.springframework.boot.actuate.endpoint.annotation.Endpoint;
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Bean;
@@ -81,16 +77,4 @@ public class ZookeeperAutoConfiguration {
properties.getMaxRetries(),
properties.getMaxSleepMs());
}
@Configuration
@ConditionalOnClass(Endpoint.class)
protected static class ZookeeperHealthConfig {
@Bean
@ConditionalOnMissingBean
@ConditionalOnBean(CuratorFramework.class)
@ConditionalOnEnabledHealthIndicator("zookeeper")
public ZookeeperHealthIndicator zookeeperHealthIndicator(CuratorFramework curator) {
return new ZookeeperHealthIndicator(curator);
}
}
}

View File

@@ -0,0 +1,56 @@
/*
* Copyright 2013-2018 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
*
* http://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.zookeeper;
import org.apache.curator.framework.CuratorFramework;
import org.springframework.boot.actuate.autoconfigure.health.ConditionalOnEnabledHealthIndicator;
import org.springframework.boot.actuate.endpoint.annotation.Endpoint;
import org.springframework.boot.autoconfigure.AutoConfigureAfter;
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
/**
* Auto {@link Configuration} for adding a Zookeeper health endpoint to actuator if
* required.
*
* @author tgianos
* @since 2.0.1
*/
@Configuration
@ConditionalOnZookeeperEnabled
@ConditionalOnClass(Endpoint.class)
@AutoConfigureAfter({ ZookeeperAutoConfiguration.class })
public class ZookeeperHealthAutoConfiguration {
/**
* If there is an active curator, if the zookeeper health endpoint is enabled and if a
* health indicator hasn't already been added by a user add one.
*
* @param curator The curator connection to zookeeper to use
* @return An instance of {@link ZookeeperHealthIndicator} to add to actuator health
* report
*/
@Bean
@ConditionalOnMissingBean(ZookeeperHealthIndicator.class)
@ConditionalOnBean(CuratorFramework.class)
@ConditionalOnEnabledHealthIndicator("zookeeper")
public ZookeeperHealthIndicator zookeeperHealthIndicator(CuratorFramework curator) {
return new ZookeeperHealthIndicator(curator);
}
}

View File

@@ -38,7 +38,8 @@ public class ZookeeperHealthIndicator extends AbstractHealthIndicator {
@Override
protected void doHealthCheck(Health.Builder builder) throws Exception {
try {
if (this.curator.getState() != CuratorFrameworkState.STARTED) {
CuratorFrameworkState state = this.curator.getState();
if (state != CuratorFrameworkState.STARTED) {
builder.down().withDetail("error", "Client not started");
}
else if (this.curator.checkExists().forPath("/") == null) {
@@ -49,7 +50,7 @@ public class ZookeeperHealthIndicator extends AbstractHealthIndicator {
}
builder.withDetail("connectionString",
this.curator.getZookeeperClient().getCurrentConnectionString())
.withDetail("state", this.curator.getState());
.withDetail("state", state);
}
catch (Exception e) {
builder.down(e);

View File

@@ -1,3 +1,4 @@
# Auto Configuration
org.springframework.boot.autoconfigure.EnableAutoConfiguration=\
org.springframework.cloud.zookeeper.ZookeeperAutoConfiguration
org.springframework.cloud.zookeeper.ZookeeperAutoConfiguration,\
org.springframework.cloud.zookeeper.ZookeeperHealthAutoConfiguration

View File

@@ -0,0 +1,70 @@
/*
* Copyright 2013-2018 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
*
* http://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.zookeeper;
import org.apache.curator.framework.CuratorFramework;
import org.assertj.core.api.Assertions;
import org.junit.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.context.annotation.Bean;
/**
* Tests for {@link ZookeeperHealthAutoConfiguration}.
*
* @author tgianos
* @since 2.0.1
*/
public class ZookeeperHealthAutoConfigurationTests {
private final ApplicationContextRunner contextRunner = new ApplicationContextRunner()
.withConfiguration(AutoConfigurations.of(ZookeeperAutoConfiguration.class,
ZookeeperHealthAutoConfiguration.class))
.withUserConfiguration(ZookeeperAutoConfigurationTests.TestConfig.class);
@Test
public void testDefaultPropertiesCreateZookeeperHealthIndicator() {
this.contextRunner.run((context) -> Assertions.assertThat(context)
.hasSingleBean(ZookeeperHealthIndicator.class));
}
@Test
public void testZookeeperHealthIndicatorDisabled() {
this.contextRunner.withPropertyValues("management.health.zookeeper.enabled=false")
.run((context) -> Assertions.assertThat(context)
.doesNotHaveBean(ZookeeperHealthIndicator.class));
}
@Test
public void testZookeeperHealthIndicatorAlreadyAdded() {
this.contextRunner.withUserConfiguration(HealthIndicatorCustomConfig.class)
.run((context) -> {
Assertions.assertThat(context)
.hasSingleBean(ZookeeperHealthIndicator.class);
Assertions.assertThat(context)
.doesNotHaveBean("zookeeperHealthIndicator");
Assertions.assertThat(context)
.hasBean("customZookeeperHealthIndicator");
});
}
static class HealthIndicatorCustomConfig {
@Bean
ZookeeperHealthIndicator customZookeeperHealthIndicator(
CuratorFramework curatorFramework) {
return new ZookeeperHealthIndicator(curatorFramework);
}
}
}