Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 8 additions & 2 deletions apiml/src/main/java/org/zowe/apiml/GatewayHealthIndicator.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.actuate.health.AbstractHealthIndicator;
import org.springframework.boot.actuate.health.Health.Builder;
Expand Down Expand Up @@ -46,7 +47,7 @@
@Component
@RequiredArgsConstructor
@Slf4j
public class GatewayHealthIndicator extends AbstractHealthIndicator {
public class GatewayHealthIndicator extends AbstractHealthIndicator implements InitializingBean {

private final ApplicationContext applicationContext;
private final ServiceStartupEventHandler serviceStartupEventHandler;
Expand All @@ -63,6 +64,11 @@ public class GatewayHealthIndicator extends AbstractHealthIndicator {

private AtomicBoolean startedInformationPublished = new AtomicBoolean(false);

@Override
public void afterPropertiesSet() throws Exception {
// load ZWE_ discovery services list environment variable as a proxy to know how many instances were defined?
}

@Override
protected void doHealthCheck(Builder builder) throws Exception {
var anyCatalogIsAvailable = StringUtils.isNotBlank(apiCatalogServiceId);
Expand Down Expand Up @@ -90,7 +96,7 @@ protected void doHealthCheck(Builder builder) throws Exception {
builder.withDetail(CoreService.API_CATALOG.getServiceId(), toStatus(catalogAvailable.get()).getCode());
}

if (isFullyUp()) {
if (isFullyUp()) { // check number of instances (non-modulith)
onFullyUp();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@

package org.zowe.apiml.gateway.config;

import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.math.NumberUtils;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.actuate.health.AbstractHealthIndicator;
import org.springframework.boot.actuate.health.Health;
Expand All @@ -25,6 +28,8 @@
import org.zowe.apiml.product.constants.CoreService;
import org.zowe.apiml.product.logging.annotations.InjectApimlLogger;

import java.util.Arrays;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;

import static org.springframework.boot.actuate.health.Status.DOWN;
Expand All @@ -37,8 +42,11 @@
*/
@Component
@ConditionalOnMissingBean(name = "modulithConfig")
@Slf4j
public class GatewayHealthIndicator extends AbstractHealthIndicator {

private static final String ZWE_DISCOVERY_SERVICES_LIST = "ZWE_DISCOVERY_SERVICES_LIST";

protected final DiscoveryClient discoveryClient;
private final String apiCatalogServiceId;
@InjectApimlLogger
Expand All @@ -47,10 +55,13 @@ public class GatewayHealthIndicator extends AbstractHealthIndicator {
private AtomicBoolean startedInformationPublished = new AtomicBoolean(false);
private AtomicBoolean applicationReady = new AtomicBoolean(false);

private final int expectedInstanceCount;

public GatewayHealthIndicator(DiscoveryClient discoveryClient,
@Value("${apiml.catalog.serviceId:}") String apiCatalogServiceId) {
this.discoveryClient = discoveryClient;
this.apiCatalogServiceId = apiCatalogServiceId;
this.expectedInstanceCount = Optional.ofNullable(System.getenv(ZWE_DISCOVERY_SERVICES_LIST)).map(list -> Arrays.stream(list.split(",")).count()).orElse(1L).intValue();
}

@Override
Expand All @@ -64,6 +75,7 @@ protected void doHealthCheck(Health.Builder builder) {
var zaasUp = !this.discoveryClient.getInstances(CoreService.ZAAS.getServiceId()).isEmpty();

var gatewayCount = this.discoveryClient.getInstances(CoreService.GATEWAY.getServiceId()).size();
var discoveryCount = this.discoveryClient.getInstances(CoreService.DISCOVERY.getServiceId()).size();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about rather to get how many instances are in the configuration? The check itself has no exact amount in the validation.

Also, if there is no validation of instance, how do we know that only local instances are up or all in the HA setup?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it makes sense. This is a partial fix for a specific scenario in which the second instance would print the API Mediation Layer ready before Discovery and/or ZAAS are available.
I guess the question is more generic. It's true this does not cover scenarios with 3 or more instances. It's also true that the message in the second instance is correct even if there's only one Discovery and/or ZAAS.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try to refactor it

var zaasCount = this.discoveryClient.getInstances(CoreService.ZAAS.getServiceId()).size();

builder.status(toStatus(discoveryUp))
Expand All @@ -76,7 +88,14 @@ protected void doHealthCheck(Health.Builder builder) {
builder.withDetail(CoreService.API_CATALOG.getServiceId(), toStatus(apiCatalogUp).getCode());
}

// check number of instances (non-modulith)
if (discoveryUp && apiCatalogUp && zaasUp && applicationReady.get()) {
var instancesCount = NumberUtils.max(gatewayCount, zaasCount, discoveryCount);
if (instancesCount > 1 && (gatewayCount != discoveryCount || gatewayCount != zaasCount)) {
log.debug("instancesCount: {}, gatewayCount: {}, zaasCount: {}, discoveryCount: {}", instancesCount, gatewayCount, zaasCount, discoveryCount);
return;
}

onFullyUp();
}
}
Expand All @@ -99,4 +118,5 @@ boolean isStartedInformationPublished() {
private Status toStatus(boolean up) {
return up ? UP : DOWN;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,45 @@

package org.zowe.apiml.gateway.config;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.boot.actuate.health.Health;
import org.springframework.boot.actuate.health.Status;
import org.springframework.boot.context.event.ApplicationReadyEvent;
import org.springframework.cloud.client.DefaultServiceInstance;
import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.discovery.DiscoveryClient;
import org.springframework.test.util.ReflectionTestUtils;
import org.zowe.apiml.message.log.ApimlLogger;
import org.zowe.apiml.product.constants.CoreService;

import java.util.Collections;
import java.util.List;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
class GatewayHealthIndicatorTest {

@Mock
private DiscoveryClient discoveryClient;

private GatewayHealthIndicator healthIndicator;

@BeforeEach
void setUp() {
this.healthIndicator = new GatewayHealthIndicator(discoveryClient, CoreService.API_CATALOG.getServiceId());
}

private DefaultServiceInstance getDefaultServiceInstance(String serviceId, String hostname, int port) {
return new DefaultServiceInstance(
hostname + ":" + serviceId + ":" + port,
Expand All @@ -40,13 +60,13 @@ private DefaultServiceInstance getDefaultServiceInstance(String serviceId, Strin
class WhenCatalogAndDiscoveryAreAvailable {
@Test
void testStatusIsUp() {
DiscoveryClient discoveryClient = mock(DiscoveryClient.class);
when(discoveryClient.getInstances(CoreService.API_CATALOG.getServiceId())).thenReturn(
Collections.singletonList(getDefaultServiceInstance(CoreService.API_CATALOG.getServiceId(), "host", 10014)));
when(discoveryClient.getInstances(CoreService.DISCOVERY.getServiceId())).thenReturn(
Collections.singletonList(getDefaultServiceInstance(CoreService.DISCOVERY.getServiceId(), "host", 10011)));
when(discoveryClient.getInstances(CoreService.ZAAS.getServiceId())).thenReturn(
Collections.singletonList(getDefaultServiceInstance(CoreService.ZAAS.getServiceId(), "host", 10011)));

GatewayHealthIndicator healthIndicator = new GatewayHealthIndicator(discoveryClient, CoreService.API_CATALOG.getServiceId());
Health.Builder builder = new Health.Builder();
healthIndicator.doHealthCheck(builder);
assertEquals(Status.UP, builder.build().getStatus());
Expand All @@ -57,12 +77,10 @@ void testStatusIsUp() {
class WhenDiscoveryIsNotAreAvailable {
@Test
void testStatusIsDown() {
DiscoveryClient discoveryClient = mock(DiscoveryClient.class);
when(discoveryClient.getInstances(CoreService.API_CATALOG.getServiceId())).thenReturn(
Collections.singletonList(getDefaultServiceInstance(CoreService.API_CATALOG.getServiceId(), "host", 10014)));
when(discoveryClient.getInstances(CoreService.DISCOVERY.getServiceId())).thenReturn(Collections.emptyList());

GatewayHealthIndicator healthIndicator = new GatewayHealthIndicator(discoveryClient, CoreService.API_CATALOG.getServiceId());
Health.Builder builder = new Health.Builder();
healthIndicator.doHealthCheck(builder);
assertEquals(Status.DOWN, builder.build().getStatus());
Expand All @@ -71,40 +89,39 @@ void testStatusIsDown() {

@Nested
class GivenCustomCatalogProvider {

@Test
void whenHealthIsRequested_thenStatusIsUp() {
String customCatalogServiceId = "customCatalog";

DiscoveryClient discoveryClient = mock(DiscoveryClient.class);
when(discoveryClient.getInstances(customCatalogServiceId)).thenReturn(
Collections.singletonList(getDefaultServiceInstance(customCatalogServiceId, "host", 10014)));
when(discoveryClient.getInstances(CoreService.DISCOVERY.getServiceId())).thenReturn(
Collections.singletonList(getDefaultServiceInstance(CoreService.DISCOVERY.getServiceId(), "host", 10011)));

GatewayHealthIndicator healthIndicator = new GatewayHealthIndicator(discoveryClient, customCatalogServiceId);
var healthIndicator = new GatewayHealthIndicator(discoveryClient, customCatalogServiceId);

Health.Builder builder = new Health.Builder();
healthIndicator.doHealthCheck(builder);

String code = (String) builder.build().getDetails().get(CoreService.API_CATALOG.getServiceId());
assertThat(code, is("UP"));
}

}

@Nested
class GivenEverythingIsHealthy {

@Test
void whenHealthRequested_onceLogMessageAboutStartup() {

DiscoveryClient discoveryClient = mock(DiscoveryClient.class);
when(discoveryClient.getInstances(CoreService.API_CATALOG.getServiceId())).thenReturn(
Collections.singletonList(getDefaultServiceInstance(CoreService.API_CATALOG.getServiceId(), "host", 10014)));
when(discoveryClient.getInstances(CoreService.DISCOVERY.getServiceId())).thenReturn(
Collections.singletonList(getDefaultServiceInstance(CoreService.DISCOVERY.getServiceId(), "host", 10011)));
when(discoveryClient.getInstances(CoreService.ZAAS.getServiceId())).thenReturn(
Collections.singletonList(getDefaultServiceInstance(CoreService.ZAAS.getServiceId(), "host", 10023)));

GatewayHealthIndicator healthIndicator = new GatewayHealthIndicator(discoveryClient, CoreService.API_CATALOG.getServiceId());
Health.Builder builder = new Health.Builder();
healthIndicator.onApplicationEvent(mock(ApplicationReadyEvent.class));

Expand All @@ -115,4 +132,31 @@ void whenHealthRequested_onceLogMessageAboutStartup() {

}

@Nested
class WhenHAIsNotComplete {

@Mock
private ApimlLogger apimlLogger;

@BeforeEach
void setUp() {
ReflectionTestUtils.setField(healthIndicator, "apimlLog", apimlLogger);
}

@Test
void whenHealthRequested_skipLog() {
when(discoveryClient.getInstances(CoreService.GATEWAY.getServiceId())).thenReturn(List.of(mock(ServiceInstance.class), mock(ServiceInstance.class)));
when(discoveryClient.getInstances(CoreService.DISCOVERY.getServiceId())).thenReturn(List.of(mock(ServiceInstance.class)));
when(discoveryClient.getInstances(CoreService.ZAAS.getServiceId())).thenReturn(List.of(mock(ServiceInstance.class)));
when(discoveryClient.getInstances(CoreService.API_CATALOG.getServiceId())).thenReturn(List.of(mock(ServiceInstance.class)));
healthIndicator.onApplicationEvent(mock(ApplicationReadyEvent.class));

var builder = new Health.Builder();
healthIndicator.doHealthCheck(builder);

verifyNoInteractions(apimlLogger);
}

}

}
Loading