Skip to content

Commit 49baacb

Browse files
wilkinsonawilkinsona
committed
Polish "Mark Redis as down when cluster_state is fail"
See gh-27300
1 parent f31141d commit 49baacb

File tree

5 files changed

+93
-94
lines changed

5 files changed

+93
-94
lines changed

‎spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/redis/RedisHealth.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -38,14 +38,15 @@ static Builder up(Health.Builder builder, Properties info) {
3838
return builder.up();
3939
}
4040

41-
static Builder info(Health.Builder builder, ClusterInfo clusterInfo) {
41+
static Builder fromClusterInfo(Health.Builder builder, ClusterInfo clusterInfo) {
4242
builder.withDetail("cluster_size", clusterInfo.getClusterSize());
4343
builder.withDetail("slots_up", clusterInfo.getSlotsOk());
4444
builder.withDetail("slots_fail", clusterInfo.getSlotsFail());
4545

4646
if ("fail".equalsIgnoreCase(clusterInfo.getState())) {
4747
return builder.down();
48-
} else {
48+
}
49+
else {
4950
return builder.up();
5051
}
5152
}

‎spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/redis/RedisHealthIndicator.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -57,7 +57,7 @@ protected void doHealthCheck(Health.Builder builder) throws Exception {
5757

5858
private void doHealthCheck(Health.Builder builder, RedisConnection connection) {
5959
if (connection instanceof RedisClusterConnection) {
60-
RedisHealth.info(builder, ((RedisClusterConnection) connection).clusterGetClusterInfo());
60+
RedisHealth.fromClusterInfo(builder, ((RedisClusterConnection) connection).clusterGetClusterInfo());
6161
}
6262
else {
6363
RedisHealth.up(builder, connection.info("server"));

‎spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicator.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -65,7 +65,7 @@ private Mono<Health> doHealthCheck(Health.Builder builder, ReactiveRedisConnecti
6565
private Mono<Health> getHealth(Health.Builder builder, ReactiveRedisConnection connection) {
6666
if (connection instanceof ReactiveRedisClusterConnection) {
6767
return ((ReactiveRedisClusterConnection) connection).clusterGetClusterInfo()
68-
.map(info -> info(builder, info));
68+
.map((info) -> fromClusterInfo(builder, info));
6969
}
7070
return connection.serverCommands().info("server").map((info) -> up(builder, info));
7171
}
@@ -74,8 +74,8 @@ private Health up(Health.Builder builder, Properties info) {
7474
return RedisHealth.up(builder, info).build();
7575
}
7676

77-
private Health info(Health.Builder builder, ClusterInfo clusterInfo) {
78-
return RedisHealth.info(builder, clusterInfo).build();
77+
private Health fromClusterInfo(Health.Builder builder, ClusterInfo clusterInfo) {
78+
return RedisHealth.fromClusterInfo(builder, clusterInfo).build();
7979
}
8080

8181
}

‎spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/redis/RedisHealthIndicatorTests.java

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -68,22 +68,11 @@ void redisIsDown() {
6868
assertThat((String) health.getDetails().get("error")).contains("Connection failed");
6969
}
7070

71-
private RedisHealthIndicator createHealthIndicator(RedisConnection redisConnection) {
72-
RedisConnectionFactory redisConnectionFactory = mock(RedisConnectionFactory.class);
73-
given(redisConnectionFactory.getConnection()).willReturn(redisConnection);
74-
return new RedisHealthIndicator(redisConnectionFactory);
75-
}
76-
7771
@Test
78-
void redisClusterIsUpWithoutState() {
79-
final Properties clusterProperties = new Properties();
80-
clusterProperties.setProperty("cluster_size", "4");
81-
clusterProperties.setProperty("cluster_slots_ok", "4");
82-
clusterProperties.setProperty("cluster_slots_fail", "0");
83-
final RedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);
84-
85-
final RedisHealthIndicator healthIndicator = new RedisHealthIndicator(redisConnectionFactory);
86-
final Health health = healthIndicator.health();
72+
void healthWhenClusterStateIsAbsentShouldBeUp() {
73+
RedisConnectionFactory redisConnectionFactory = createClusterConnectionFactory(null);
74+
RedisHealthIndicator healthIndicator = new RedisHealthIndicator(redisConnectionFactory);
75+
Health health = healthIndicator.health();
8776
assertThat(health.getStatus()).isEqualTo(Status.UP);
8877
assertThat(health.getDetails().get("cluster_size")).isEqualTo(4L);
8978
assertThat(health.getDetails().get("slots_up")).isEqualTo(4L);
@@ -92,41 +81,52 @@ void redisClusterIsUpWithoutState() {
9281
}
9382

9483
@Test
95-
void redisClusterIsUpWithState() {
96-
final Properties clusterProperties = new Properties();
97-
clusterProperties.setProperty("cluster_state", "ok");
98-
clusterProperties.setProperty("cluster_size", "4");
99-
clusterProperties.setProperty("cluster_slots_ok", "4");
100-
clusterProperties.setProperty("cluster_slots_fail", "0");
101-
final RedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);
102-
103-
final RedisHealthIndicator healthIndicator = new RedisHealthIndicator(redisConnectionFactory);
104-
final Health health = healthIndicator.health();
84+
void healthWhenClusterStateIsOkShouldBeUp() {
85+
RedisConnectionFactory redisConnectionFactory = createClusterConnectionFactory("ok");
86+
RedisHealthIndicator healthIndicator = new RedisHealthIndicator(redisConnectionFactory);
87+
Health health = healthIndicator.health();
10588
assertThat(health.getStatus()).isEqualTo(Status.UP);
89+
assertThat(health.getDetails().get("cluster_size")).isEqualTo(4L);
90+
assertThat(health.getDetails().get("slots_up")).isEqualTo(4L);
91+
assertThat(health.getDetails().get("slots_fail")).isEqualTo(0L);
92+
verify(redisConnectionFactory, atLeastOnce()).getConnection();
10693
}
10794

10895
@Test
109-
void redisClusterIsDown() {
110-
final Properties clusterProperties = new Properties();
111-
clusterProperties.setProperty("cluster_state", "fail");
112-
clusterProperties.setProperty("cluster_size", "3");
113-
clusterProperties.setProperty("cluster_slots_ok", "4");
114-
clusterProperties.setProperty("cluster_slots_fail", "0");
115-
final RedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);
116-
117-
final RedisHealthIndicator healthIndicator = new RedisHealthIndicator(redisConnectionFactory);
118-
final Health health = healthIndicator.health();
96+
void healthWhenClusterStateIsFailShouldBeDown() {
97+
RedisConnectionFactory redisConnectionFactory = createClusterConnectionFactory("fail");
98+
RedisHealthIndicator healthIndicator = new RedisHealthIndicator(redisConnectionFactory);
99+
Health health = healthIndicator.health();
119100
assertThat(health.getStatus()).isEqualTo(Status.DOWN);
101+
assertThat(health.getDetails().get("cluster_size")).isEqualTo(4L);
102+
assertThat(health.getDetails().get("slots_up")).isEqualTo(3L);
103+
assertThat(health.getDetails().get("slots_fail")).isEqualTo(1L);
104+
verify(redisConnectionFactory, atLeastOnce()).getConnection();
105+
}
106+
107+
private RedisHealthIndicator createHealthIndicator(RedisConnection redisConnection) {
108+
RedisConnectionFactory redisConnectionFactory = mock(RedisConnectionFactory.class);
109+
given(redisConnectionFactory.getConnection()).willReturn(redisConnection);
110+
return new RedisHealthIndicator(redisConnectionFactory);
120111
}
121112

122-
private static RedisConnectionFactory mockRedisClusterConnectionFactory(Properties clusterProperties) {
123-
final List<RedisClusterNode> redisMasterNodes = Arrays.asList(new RedisClusterNode("127.0.0.1", 7001),
124-
new RedisClusterNode("127.0.0.2", 7001));
125-
final RedisClusterConnection redisConnection = mock(RedisClusterConnection.class);
113+
private RedisConnectionFactory createClusterConnectionFactory(String state) {
114+
Properties clusterProperties = new Properties();
115+
if (state != null) {
116+
clusterProperties.setProperty("cluster_state", state);
117+
}
118+
clusterProperties.setProperty("cluster_size", "4");
119+
boolean failure = "fail".equals(state);
120+
clusterProperties.setProperty("cluster_slots_ok", failure ? "3" : "4");
121+
clusterProperties.setProperty("cluster_slots_fail", failure ? "1" : "0");
122+
List<RedisClusterNode> redisMasterNodes = Arrays.asList(new RedisClusterNode("127.0.0.1", 7001),
123+
new RedisClusterNode("127.0.0.2", 7001));
124+
RedisClusterConnection redisConnection = mock(RedisClusterConnection.class);
126125
given(redisConnection.clusterGetNodes()).willReturn(redisMasterNodes);
127126
given(redisConnection.clusterGetClusterInfo()).willReturn(new ClusterInfo(clusterProperties));
128-
final RedisConnectionFactory redisConnectionFactory = mock(RedisConnectionFactory.class);
127+
RedisConnectionFactory redisConnectionFactory = mock(RedisConnectionFactory.class);
129128
given(redisConnectionFactory.getConnection()).willReturn(redisConnection);
130129
return redisConnectionFactory;
131130
}
131+
132132
}

‎spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicatorTests.java

Lines changed: 42 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,14 +16,13 @@
1616

1717
package org.springframework.boot.actuate.redis;
1818

19-
import static org.assertj.core.api.Assertions.assertThat;
20-
import static org.mockito.BDDMockito.given;
21-
import static org.mockito.Mockito.mock;
22-
import static org.mockito.Mockito.verify;
23-
2419
import java.util.Properties;
2520

21+
import io.lettuce.core.RedisConnectionException;
2622
import org.junit.jupiter.api.Test;
23+
import reactor.core.publisher.Mono;
24+
import reactor.test.StepVerifier;
25+
2726
import org.springframework.boot.actuate.health.Health;
2827
import org.springframework.boot.actuate.health.Status;
2928
import org.springframework.data.redis.RedisConnectionFailureException;
@@ -33,9 +32,10 @@
3332
import org.springframework.data.redis.connection.ReactiveRedisConnectionFactory;
3433
import org.springframework.data.redis.connection.ReactiveServerCommands;
3534

36-
import io.lettuce.core.RedisConnectionException;
37-
import reactor.core.publisher.Mono;
38-
import reactor.test.StepVerifier;
35+
import static org.assertj.core.api.Assertions.assertThat;
36+
import static org.mockito.BDDMockito.given;
37+
import static org.mockito.Mockito.mock;
38+
import static org.mockito.Mockito.verify;
3939

4040
/**
4141
* Tests for {@link RedisReactiveHealthIndicator}.
@@ -67,16 +67,11 @@ void redisIsUp() {
6767
}
6868

6969
@Test
70-
void redisClusterIsUpWithoutState() {
71-
final Properties clusterProperties = new Properties();
72-
clusterProperties.setProperty("cluster_size", "4");
73-
clusterProperties.setProperty("cluster_slots_ok", "4");
74-
clusterProperties.setProperty("cluster_slots_fail", "0");
75-
final ReactiveRedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);
76-
77-
final RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory);
78-
final Mono<Health> health = healthIndicator.health();
79-
StepVerifier.create(health).consumeNextWith(h -> {
70+
void healthWhenClusterStateIsAbsentShouldBeUp() {
71+
ReactiveRedisConnectionFactory redisConnectionFactory = createClusterConnectionFactory(null);
72+
RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory);
73+
Mono<Health> health = healthIndicator.health();
74+
StepVerifier.create(health).consumeNextWith((h) -> {
8075
assertThat(h.getStatus()).isEqualTo(Status.UP);
8176
assertThat(h.getDetails().get("cluster_size")).isEqualTo(4L);
8277
assertThat(h.getDetails().get("slots_up")).isEqualTo(4L);
@@ -86,34 +81,29 @@ void redisClusterIsUpWithoutState() {
8681
}
8782

8883
@Test
89-
void redisClusterIsUpWithState() {
90-
final Properties clusterProperties = new Properties();
91-
clusterProperties.setProperty("cluster_state", "ok");
92-
clusterProperties.setProperty("cluster_size", "4");
93-
clusterProperties.setProperty("cluster_slots_ok", "4");
94-
clusterProperties.setProperty("cluster_slots_fail", "0");
95-
final ReactiveRedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);
96-
97-
final RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory);
98-
final Mono<Health> health = healthIndicator.health();
99-
StepVerifier.create(health).consumeNextWith(h -> {
84+
void healthWhenClusterStateIsOkShouldBeUp() {
85+
ReactiveRedisConnectionFactory redisConnectionFactory = createClusterConnectionFactory("ok");
86+
RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory);
87+
Mono<Health> health = healthIndicator.health();
88+
StepVerifier.create(health).consumeNextWith((h) -> {
10089
assertThat(h.getStatus()).isEqualTo(Status.UP);
90+
assertThat(h.getDetails().get("cluster_size")).isEqualTo(4L);
91+
assertThat(h.getDetails().get("slots_up")).isEqualTo(4L);
92+
assertThat(h.getDetails().get("slots_fail")).isEqualTo(0L);
10193
}).verifyComplete();
10294
}
10395

10496
@Test
105-
void redisClusterIsDown() {
106-
final Properties clusterProperties = new Properties();
97+
void healthWhenClusterStateIsFailShouldBeDown() {
98+
Properties clusterProperties = new Properties();
10799
clusterProperties.setProperty("cluster_state", "fail");
108-
clusterProperties.setProperty("cluster_size", "3");
109-
clusterProperties.setProperty("cluster_slots_ok", "4");
110-
clusterProperties.setProperty("cluster_slots_fail", "0");
111-
final ReactiveRedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);
112-
113-
final RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory);
114-
final Mono<Health> health = healthIndicator.health();
115-
StepVerifier.create(health).consumeNextWith(h -> {
100+
ReactiveRedisConnectionFactory redisConnectionFactory = createClusterConnectionFactory("fail");
101+
RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory);
102+
Mono<Health> health = healthIndicator.health();
103+
StepVerifier.create(health).consumeNextWith((h) -> {
116104
assertThat(h.getStatus()).isEqualTo(Status.DOWN);
105+
assertThat(h.getDetails().get("slots_up")).isEqualTo(3L);
106+
assertThat(h.getDetails().get("slots_fail")).isEqualTo(1L);
117107
}).verifyComplete();
118108
}
119109

@@ -143,19 +133,27 @@ void redisConnectionIsDown() {
143133

144134
private RedisReactiveHealthIndicator createHealthIndicator(ReactiveRedisConnection redisConnection,
145135
ReactiveServerCommands serverCommands) {
146-
147136
ReactiveRedisConnectionFactory redisConnectionFactory = mock(ReactiveRedisConnectionFactory.class);
148137
given(redisConnectionFactory.getReactiveConnection()).willReturn(redisConnection);
149138
given(redisConnection.serverCommands()).willReturn(serverCommands);
150139
return new RedisReactiveHealthIndicator(redisConnectionFactory);
151140
}
152141

153-
private static ReactiveRedisConnectionFactory mockRedisClusterConnectionFactory(Properties clusterProperties) {
154-
final ReactiveRedisClusterConnection redisConnection = mock(ReactiveRedisClusterConnection.class);
142+
private ReactiveRedisConnectionFactory createClusterConnectionFactory(String state) {
143+
Properties clusterProperties = new Properties();
144+
if (state != null) {
145+
clusterProperties.setProperty("cluster_state", state);
146+
}
147+
clusterProperties.setProperty("cluster_size", "4");
148+
boolean failure = "fail".equals(state);
149+
clusterProperties.setProperty("cluster_slots_ok", failure ? "3" : "4");
150+
clusterProperties.setProperty("cluster_slots_fail", failure ? "1" : "0");
151+
ReactiveRedisClusterConnection redisConnection = mock(ReactiveRedisClusterConnection.class);
155152
given(redisConnection.closeLater()).willReturn(Mono.empty());
156153
given(redisConnection.clusterGetClusterInfo()).willReturn(Mono.just(new ClusterInfo(clusterProperties)));
157-
final ReactiveRedisConnectionFactory redisConnectionFactory = mock(ReactiveRedisConnectionFactory.class);
154+
ReactiveRedisConnectionFactory redisConnectionFactory = mock(ReactiveRedisConnectionFactory.class);
158155
given(redisConnectionFactory.getReactiveConnection()).willReturn(redisConnection);
159156
return redisConnectionFactory;
160157
}
158+
161159
}

0 commit comments

Comments
 (0)