Skip to content

Redis health indicators report that Redis is up when the cluster's state is fail #27300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Redis health indicators report that Redis is up when the cluster's state is fail #27300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions ...pring-boot-actuator/src/main/java/org/springframework/boot/actuate/redis/RedisHealth.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,16 @@ static Builder up(Health.Builder builder, Properties info) {
return builder.up();
}

static Builder up(Health.Builder builder, ClusterInfo clusterInfo) {
static Builder info(Health.Builder builder, ClusterInfo clusterInfo) {
builder.withDetail("cluster_size", clusterInfo.getClusterSize());
builder.withDetail("slots_up", clusterInfo.getSlotsOk());
builder.withDetail("slots_fail", clusterInfo.getSlotsFail());
return builder.up();

if ("fail".equalsIgnoreCase(clusterInfo.getState())) {
return builder.down();
} else {
return builder.up();
}
}

}
2 changes: 1 addition & 1 deletion ...t-actuator/src/main/java/org/springframework/boot/actuate/redis/RedisHealthIndicator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ protected void doHealthCheck(Health.Builder builder) throws Exception {

private void doHealthCheck(Health.Builder builder, RedisConnection connection) {
if (connection instanceof RedisClusterConnection) {
RedisHealth.up(builder, ((RedisClusterConnection) connection).clusterGetClusterInfo());
RedisHealth.info(builder, ((RedisClusterConnection) connection).clusterGetClusterInfo());
}
else {
RedisHealth.up(builder, connection.info("server"));
Expand Down
6 changes: 3 additions & 3 deletions ...or/src/main/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private Mono<Health> doHealthCheck(Health.Builder builder, ReactiveRedisConnecti
private Mono<Health> getHealth(Health.Builder builder, ReactiveRedisConnection connection) {
if (connection instanceof ReactiveRedisClusterConnection) {
return ((ReactiveRedisClusterConnection) connection).clusterGetClusterInfo()
.map((info) -> up(builder, info));
.map(info -> info(builder, info));
}
return connection.serverCommands().info("server").map((info) -> up(builder, info));
}
Expand All @@ -74,8 +74,8 @@ private Health up(Health.Builder builder, Properties info) {
return RedisHealth.up(builder, info).build();
}

private Health up(Health.Builder builder, ClusterInfo clusterInfo) {
return RedisHealth.up(builder, clusterInfo).build();
private Health info(Health.Builder builder, ClusterInfo clusterInfo) {
return RedisHealth.info(builder, clusterInfo).build();
}

}
55 changes: 44 additions & 11 deletions ...uator/src/test/java/org/springframework/boot/actuate/redis/RedisHealthIndicatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,25 +75,58 @@ private RedisHealthIndicator createHealthIndicator(RedisConnection redisConnecti
}

@Test
void redisClusterIsUp() {
Properties clusterProperties = new Properties();
void redisClusterIsUpWithoutState() {
final Properties clusterProperties = new Properties();
clusterProperties.setProperty("cluster_size", "4");
clusterProperties.setProperty("cluster_slots_ok", "4");
clusterProperties.setProperty("cluster_slots_fail", "0");
List<RedisClusterNode> redisMasterNodes = Arrays.asList(new RedisClusterNode("127.0.0.1", 7001),
new RedisClusterNode("127.0.0.2", 7001));
RedisClusterConnection redisConnection = mock(RedisClusterConnection.class);
given(redisConnection.clusterGetNodes()).willReturn(redisMasterNodes);
given(redisConnection.clusterGetClusterInfo()).willReturn(new ClusterInfo(clusterProperties));
RedisConnectionFactory redisConnectionFactory = mock(RedisConnectionFactory.class);
given(redisConnectionFactory.getConnection()).willReturn(redisConnection);
RedisHealthIndicator healthIndicator = new RedisHealthIndicator(redisConnectionFactory);
Health health = healthIndicator.health();
final RedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);

final RedisHealthIndicator healthIndicator = new RedisHealthIndicator(redisConnectionFactory);
final Health health = healthIndicator.health();
assertThat(health.getStatus()).isEqualTo(Status.UP);
assertThat(health.getDetails().get("cluster_size")).isEqualTo(4L);
assertThat(health.getDetails().get("slots_up")).isEqualTo(4L);
assertThat(health.getDetails().get("slots_fail")).isEqualTo(0L);
verify(redisConnectionFactory, atLeastOnce()).getConnection();
}

@Test
void redisClusterIsUpWithState() {
final Properties clusterProperties = new Properties();
clusterProperties.setProperty("cluster_state", "ok");
clusterProperties.setProperty("cluster_size", "4");
clusterProperties.setProperty("cluster_slots_ok", "4");
clusterProperties.setProperty("cluster_slots_fail", "0");
final RedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);

final RedisHealthIndicator healthIndicator = new RedisHealthIndicator(redisConnectionFactory);
final Health health = healthIndicator.health();
assertThat(health.getStatus()).isEqualTo(Status.UP);
}

@Test
void redisClusterIsDown() {
final Properties clusterProperties = new Properties();
clusterProperties.setProperty("cluster_state", "fail");
clusterProperties.setProperty("cluster_size", "3");
clusterProperties.setProperty("cluster_slots_ok", "4");
clusterProperties.setProperty("cluster_slots_fail", "0");
final RedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);

final RedisHealthIndicator healthIndicator = new RedisHealthIndicator(redisConnectionFactory);
final Health health = healthIndicator.health();
assertThat(health.getStatus()).isEqualTo(Status.DOWN);
}

private static RedisConnectionFactory mockRedisClusterConnectionFactory(Properties clusterProperties) {
final List<RedisClusterNode> redisMasterNodes = Arrays.asList(new RedisClusterNode("127.0.0.1", 7001),
new RedisClusterNode("127.0.0.2", 7001));
final RedisClusterConnection redisConnection = mock(RedisClusterConnection.class);
given(redisConnection.clusterGetNodes()).willReturn(redisMasterNodes);
given(redisConnection.clusterGetClusterInfo()).willReturn(new ClusterInfo(clusterProperties));
final RedisConnectionFactory redisConnectionFactory = mock(RedisConnectionFactory.class);
given(redisConnectionFactory.getConnection()).willReturn(redisConnection);
return redisConnectionFactory;
}
}
75 changes: 56 additions & 19 deletions ...c/test/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@

package org.springframework.boot.actuate.redis;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

import java.util.Properties;

import io.lettuce.core.RedisConnectionException;
import org.junit.jupiter.api.Test;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;

import org.springframework.boot.actuate.health.Health;
import org.springframework.boot.actuate.health.Status;
import org.springframework.data.redis.RedisConnectionFailureException;
Expand All @@ -32,10 +33,9 @@
import org.springframework.data.redis.connection.ReactiveRedisConnectionFactory;
import org.springframework.data.redis.connection.ReactiveServerCommands;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import io.lettuce.core.RedisConnectionException;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;

/**
* Tests for {@link RedisReactiveHealthIndicator}.
Expand Down Expand Up @@ -67,25 +67,54 @@ void redisIsUp() {
}

@Test
void redisClusterIsUp() {
Properties clusterProperties = new Properties();
void redisClusterIsUpWithoutState() {
final Properties clusterProperties = new Properties();
clusterProperties.setProperty("cluster_size", "4");
clusterProperties.setProperty("cluster_slots_ok", "4");
clusterProperties.setProperty("cluster_slots_fail", "0");
ReactiveRedisClusterConnection redisConnection = mock(ReactiveRedisClusterConnection.class);
given(redisConnection.closeLater()).willReturn(Mono.empty());
given(redisConnection.clusterGetClusterInfo()).willReturn(Mono.just(new ClusterInfo(clusterProperties)));
ReactiveRedisConnectionFactory redisConnectionFactory = mock(ReactiveRedisConnectionFactory.class);
given(redisConnectionFactory.getReactiveConnection()).willReturn(redisConnection);
RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory);
Mono<Health> health = healthIndicator.health();
StepVerifier.create(health).consumeNextWith((h) -> {
final ReactiveRedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);

final RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory);
final Mono<Health> health = healthIndicator.health();
StepVerifier.create(health).consumeNextWith(h -> {
assertThat(h.getStatus()).isEqualTo(Status.UP);
assertThat(h.getDetails().get("cluster_size")).isEqualTo(4L);
assertThat(h.getDetails().get("slots_up")).isEqualTo(4L);
assertThat(h.getDetails().get("slots_fail")).isEqualTo(0L);
}).verifyComplete();
verify(redisConnection).closeLater();
verify(redisConnectionFactory.getReactiveConnection()).closeLater();
}

@Test
void redisClusterIsUpWithState() {
final Properties clusterProperties = new Properties();
clusterProperties.setProperty("cluster_state", "ok");
clusterProperties.setProperty("cluster_size", "4");
clusterProperties.setProperty("cluster_slots_ok", "4");
clusterProperties.setProperty("cluster_slots_fail", "0");
final ReactiveRedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);

final RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory);
final Mono<Health> health = healthIndicator.health();
StepVerifier.create(health).consumeNextWith(h -> {
assertThat(h.getStatus()).isEqualTo(Status.UP);
}).verifyComplete();
}

@Test
void redisClusterIsDown() {
final Properties clusterProperties = new Properties();
clusterProperties.setProperty("cluster_state", "fail");
clusterProperties.setProperty("cluster_size", "3");
clusterProperties.setProperty("cluster_slots_ok", "4");
clusterProperties.setProperty("cluster_slots_fail", "0");
final ReactiveRedisConnectionFactory redisConnectionFactory = mockRedisClusterConnectionFactory(clusterProperties);

final RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory);
final Mono<Health> health = healthIndicator.health();
StepVerifier.create(health).consumeNextWith(h -> {
assertThat(h.getStatus()).isEqualTo(Status.DOWN);
}).verifyComplete();
}

@Test
Expand Down Expand Up @@ -121,4 +150,12 @@ private RedisReactiveHealthIndicator createHealthIndicator(ReactiveRedisConnecti
return new RedisReactiveHealthIndicator(redisConnectionFactory);
}

private static ReactiveRedisConnectionFactory mockRedisClusterConnectionFactory(Properties clusterProperties) {
final ReactiveRedisClusterConnection redisConnection = mock(ReactiveRedisClusterConnection.class);
given(redisConnection.closeLater()).willReturn(Mono.empty());
given(redisConnection.clusterGetClusterInfo()).willReturn(Mono.just(new ClusterInfo(clusterProperties)));
final ReactiveRedisConnectionFactory redisConnectionFactory = mock(ReactiveRedisConnectionFactory.class);
given(redisConnectionFactory.getReactiveConnection()).willReturn(redisConnection);
return redisConnectionFactory;
}
}