Skip to content

Add health checks again #1685

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

Merged
Yavor16 merged 1 commit into from
Aug 28, 2025
Merged

Add health checks again #1685

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
19 changes: 17 additions & 2 deletions ...in/java/org/cloudfoundry/multiapps/controller/client/facade/adapters/RawCloudProcess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
import org.cloudfoundry.client.v3.processes.Data;
import org.cloudfoundry.client.v3.processes.HealthCheck;
import org.cloudfoundry.client.v3.processes.Process;
import org.immutables.value.Value;

import org.cloudfoundry.client.v3.processes.ReadinessHealthCheck;
import org.cloudfoundry.multiapps.controller.client.facade.domain.CloudProcess;
import org.cloudfoundry.multiapps.controller.client.facade.domain.HealthCheckType;
import org.cloudfoundry.multiapps.controller.client.facade.domain.ImmutableCloudProcess;
import org.immutables.value.Value;

@Value.Immutable
public abstract class RawCloudProcess extends RawCloudEntity<CloudProcess> {
Expand All @@ -19,6 +19,7 @@ public abstract class RawCloudProcess extends RawCloudEntity<CloudProcess> {
public CloudProcess derive() {
Process process = getProcess();
HealthCheck healthCheck = process.getHealthCheck();
ReadinessHealthCheck readinessHealthCheckType = process.getReadinessHealthCheck();
Integer healthCheckTimeout = null;
String healthCheckHttpEndpoint = null;
Integer healthCheckInvocationTimeout = null;
Expand All @@ -28,6 +29,15 @@ public CloudProcess derive() {
healthCheckInvocationTimeout = healthCheckData.getInvocationTimeout();
healthCheckHttpEndpoint = healthCheckData.getEndpoint();
}
Integer readinessHealthCheckInvocationTimeout = null;
String readinessHealthCheckHttpEndpoint = null;
Integer readinessHealthCheckInterval = null;
if (readinessHealthCheckType.getData() != null) {
Copy link
Contributor

@IvanBorislavovDimitrov IvanBorislavovDimitrov Aug 26, 2025

Choose a reason for hiding this comment

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

can readinessHealthCheckType be null and throw npe?

Copy link
Contributor Author

@Yavor16 Yavor16 Aug 28, 2025

Choose a reason for hiding this comment

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

I think the type can't be null because it has default value. In the documentation is said that the default value is process

Data readinessHealthCheckData = readinessHealthCheckType.getData();
readinessHealthCheckInvocationTimeout = readinessHealthCheckData.getInvocationTimeout();
readinessHealthCheckHttpEndpoint = readinessHealthCheckData.getEndpoint();
readinessHealthCheckInterval = readinessHealthCheckData.getInterval();
}
return ImmutableCloudProcess.builder()
.command(process.getCommand())
.instances(process.getInstances())
Expand All @@ -39,6 +49,11 @@ public CloudProcess derive() {
.healthCheckHttpEndpoint(healthCheckHttpEndpoint)
.healthCheckTimeout(healthCheckTimeout)
.healthCheckInvocationTimeout(healthCheckInvocationTimeout)
.readinessHealthCheckType(readinessHealthCheckType.getType()
.getValue())
.readinessHealthCheckHttpEndpoint(readinessHealthCheckHttpEndpoint)
.readinessHealthCheckInvocationTimeout(readinessHealthCheckInvocationTimeout)
.readinessHealthCheckInterval(readinessHealthCheckInterval)
.build();
}
}
5 changes: 2 additions & 3 deletions ...n/java/org/cloudfoundry/multiapps/controller/client/facade/adapters/RawInstancesInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@

import org.cloudfoundry.client.v3.applications.GetApplicationProcessStatisticsResponse;
import org.cloudfoundry.client.v3.processes.ProcessStatisticsResource;
import org.immutables.value.Value;

import org.cloudfoundry.multiapps.controller.client.facade.domain.ImmutableInstanceInfo;
import org.cloudfoundry.multiapps.controller.client.facade.domain.ImmutableInstancesInfo;
import org.cloudfoundry.multiapps.controller.client.facade.domain.InstanceInfo;
import org.cloudfoundry.multiapps.controller.client.facade.domain.InstanceState;
import org.cloudfoundry.multiapps.controller.client.facade.domain.InstancesInfo;
import org.immutables.value.Value;

@Value.Immutable
public abstract class RawInstancesInfo extends RawCloudEntity<InstancesInfo> {
Expand Down Expand Up @@ -41,7 +40,7 @@ private static InstanceInfo parseProcessStatistic(ProcessStatisticsResource stat
return ImmutableInstanceInfo.builder()
.index(statsResource.getIndex())
.state(InstanceState.valueOfWithDefault(statsResource.getState()))
.isRoutable(Boolean.parseBoolean(statsResource.getRoutable()))
.build();
}

}
15 changes: 13 additions & 2 deletions ...rc/main/java/org/cloudfoundry/multiapps/controller/client/facade/domain/CloudProcess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package org.cloudfoundry.multiapps.controller.client.facade.domain;

import org.immutables.value.Value;

import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import org.cloudfoundry.multiapps.controller.client.facade.Nullable;
import org.immutables.value.Value;

@Value.Immutable
@JsonSerialize(as = ImmutableCloudProcess.class)
Expand All @@ -30,6 +29,18 @@ public abstract class CloudProcess extends CloudEntity implements Derivable<Clou
@Nullable
public abstract Integer getHealthCheckTimeout();

@Nullable
public abstract Integer getReadinessHealthCheckInterval();

@Nullable
public abstract Integer getReadinessHealthCheckInvocationTimeout();

@Nullable
public abstract String getReadinessHealthCheckType();

@Nullable
public abstract String getReadinessHealthCheckHttpEndpoint();

@Override
public CloudProcess derive() {
return this;
Expand Down
6 changes: 4 additions & 2 deletions ...rc/main/java/org/cloudfoundry/multiapps/controller/client/facade/domain/InstanceInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package org.cloudfoundry.multiapps.controller.client.facade.domain;

import org.immutables.value.Value;

import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import org.cloudfoundry.multiapps.controller.client.facade.Nullable;
import org.immutables.value.Value;

@Value.Immutable
@JsonSerialize(as = ImmutableInstanceInfo.class)
Expand All @@ -14,4 +14,6 @@ public interface InstanceInfo {

InstanceState getState();

@Nullable
Boolean isRoutable();
}
24 changes: 24 additions & 0 deletions ...ent/src/main/java/org/cloudfoundry/multiapps/controller/client/facade/domain/Staging.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,30 @@ public interface Staging {
@Nullable
String getHealthCheckHttpEndpoint();

/**
* @return readiness health check interval
*/
@Nullable
Integer getReadinessHealthCheckInterval();

/**
* @return readiness health check timeout
*/
@Nullable
Integer getReadinessHealthCheckInvocationTimeout();

/**
* @return readiness health check type
*/
@Nullable
String getReadinessHealthCheckType();

/**
* @return readiness health check http endpoint
*/
@Nullable
String getReadinessHealthCheckHttpEndpoint();

/**
* @return boolean value to see if ssh is enabled
*/
Expand Down
20 changes: 20 additions & 0 deletions ...g/cloudfoundry/multiapps/controller/client/facade/rest/CloudControllerRestClientImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@
import org.cloudfoundry.client.v3.processes.Data;
import org.cloudfoundry.client.v3.processes.HealthCheck;
import org.cloudfoundry.client.v3.processes.HealthCheckType;
import org.cloudfoundry.client.v3.processes.ReadinessHealthCheck;
import org.cloudfoundry.client.v3.processes.ReadinessHealthCheckType;
import org.cloudfoundry.client.v3.processes.UpdateProcessRequest;
import org.cloudfoundry.client.v3.roles.ListRolesRequest;
import org.cloudfoundry.client.v3.roles.RoleResource;
Expand Down Expand Up @@ -393,11 +395,29 @@ private void updateApplicationProcess(UUID applicationGuid, Staging staging) {
if (staging.getHealthCheckType() != null) {
updateProcessRequestBuilder.healthCheck(buildHealthCheck(staging));
}
if (staging.getReadinessHealthCheckType() != null) {
updateProcessRequestBuilder.readinessHealthCheck(buildReadinessHealthCheck(staging));
}
delegate.processes()
.update(updateProcessRequestBuilder.build())
.block();
}

private ReadinessHealthCheck buildReadinessHealthCheck(Staging staging) {
return ReadinessHealthCheck.builder()
.type(ReadinessHealthCheckType.from(staging.getReadinessHealthCheckType()))
.data(buildReadinessHealthCheckData(staging))
.build();
}

private Data buildReadinessHealthCheckData(Staging staging) {
return Data.builder()
.invocationTimeout(staging.getReadinessHealthCheckInvocationTimeout())
.endpoint(staging.getReadinessHealthCheckHttpEndpoint())
.interval(staging.getReadinessHealthCheckInterval())
.build();
}

private void updateAppFeature(UUID applicationGuid, String featureName, boolean enabled) {
delegate.applicationsV3()
.updateFeature(UpdateApplicationFeatureRequest.builder()
Expand Down
22 changes: 14 additions & 8 deletions ...va/org/cloudfoundry/multiapps/controller/client/facade/adapters/RawInstancesInfoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,36 +3,41 @@
import org.cloudfoundry.client.v3.applications.GetApplicationProcessStatisticsResponse;
import org.cloudfoundry.client.v3.processes.ProcessState;
import org.cloudfoundry.client.v3.processes.ProcessStatisticsResource;
import org.junit.jupiter.api.Test;

import org.cloudfoundry.multiapps.controller.client.facade.domain.ImmutableInstanceInfo;
import org.cloudfoundry.multiapps.controller.client.facade.domain.ImmutableInstancesInfo;
import org.cloudfoundry.multiapps.controller.client.facade.domain.InstanceState;
import org.cloudfoundry.multiapps.controller.client.facade.domain.InstancesInfo;
import org.junit.jupiter.api.Test;

class RawInstancesInfoTest {

@Test
void testDerive() {
RawCloudEntityTest.testDerive(buildExpectedInstancesInfo(), buildActualInstancesInfo());
void testDeriveWithTrueRoutable() {
RawCloudEntityTest.testDerive(buildExpectedInstancesInfo(true), buildActualInstancesInfo("true"));
}

@Test
void testDeriveWithFalseRoutable() {
RawCloudEntityTest.testDerive(buildExpectedInstancesInfo(false), buildActualInstancesInfo("false"));
}

private InstancesInfo buildExpectedInstancesInfo() {
private InstancesInfo buildExpectedInstancesInfo(boolean expectedRoutable) {
return ImmutableInstancesInfo.builder()
.addInstance(ImmutableInstanceInfo.builder()
.index(0)
.state(InstanceState.RUNNING)
.isRoutable(expectedRoutable)
.build())
.build();
}

private RawInstancesInfo buildActualInstancesInfo() {
private RawInstancesInfo buildActualInstancesInfo(String routable) {
return ImmutableRawInstancesInfo.builder()
.processStatisticsResponse(getApplicationProcessStatisticsResponse())
.processStatisticsResponse(getApplicationProcessStatisticsResponse(routable))
.build();
}

private GetApplicationProcessStatisticsResponse getApplicationProcessStatisticsResponse() {
private GetApplicationProcessStatisticsResponse getApplicationProcessStatisticsResponse(String routable) {
return GetApplicationProcessStatisticsResponse.builder()
.resource(ProcessStatisticsResource.builder()
.index(0)
Expand All @@ -43,6 +48,7 @@ private GetApplicationProcessStatisticsResponse getApplicationProcessStatisticsR
.type("web")
.uptime(9042L)
.fileDescriptorQuota(1024L)
.routable(routable)
Copy link
Contributor

@theghost5800 theghost5800 Aug 25, 2025

Choose a reason for hiding this comment

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

why value is not boolean?

Copy link
Contributor Author

@Yavor16 Yavor16 Aug 26, 2025

Choose a reason for hiding this comment

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

Because in the ProcessStatisticsResource class the routable variable is String

.host("10.244.16.10")
.build())
.build();
Expand Down
4 changes: 3 additions & 1 deletion ...e/src/main/java/org/cloudfoundry/multiapps/controller/core/model/SupportedParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,9 @@ public class SupportedParameters {
REGISTER_SERVICE_URL, REGISTER_SERVICE_URL_SERVICE_NAME,
REGISTER_SERVICE_URL_SERVICE_URL, MODULE_CONFIG, MANAGED, PATH,
APPS_UPLOAD_TIMEOUT, APPS_TASK_EXECUTION_TIMEOUT, APPS_START_TIMEOUT,
APPS_STAGE_TIMEOUT, SKIP_DEPLOY, APP_FEATURES);
APPS_STAGE_TIMEOUT, SKIP_DEPLOY, APP_FEATURES, READINESS_HEALTH_CHECK_TYPE,
READINESS_HEALTH_CHECK_HTTP_ENDPOINT, READINESS_HEALTH_CHECK_INTERVAL,
READINESS_HEALTH_CHECK_INVOCATION_TIMEOUT);

public static final Set<String> RESOURCE_PARAMETERS = Set.of(APPLY_NAMESPACE, SERVICE_CONFIG, SYSLOG_DRAIN_URL, DEFAULT_CONTAINER_NAME,
DEFAULT_SERVICE_NAME, DEFAULT_XS_APP_NAME, SERVICE, SERVICE_KEYS,
Expand Down
18 changes: 17 additions & 1 deletion .../main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
import java.util.Map;

import org.cloudfoundry.multiapps.common.ContentException;
import org.cloudfoundry.multiapps.controller.core.Constants;
import org.cloudfoundry.multiapps.controller.client.facade.domain.DockerInfo;
import org.cloudfoundry.multiapps.controller.client.facade.domain.ImmutableStaging;
import org.cloudfoundry.multiapps.controller.client.facade.domain.LifecycleType;
import org.cloudfoundry.multiapps.controller.client.facade.domain.Staging;
import org.cloudfoundry.multiapps.controller.core.Constants;
import org.cloudfoundry.multiapps.controller.core.model.SupportedParameters;
import org.cloudfoundry.multiapps.mta.util.PropertiesUtil;
import org.springframework.util.CollectionUtils;
Expand Down Expand Up @@ -41,6 +41,18 @@ public Staging parse(List<Map<String, Object>> parametersList) {
String healthCheckHttpEndpoint = (String) PropertiesUtil.getPropertyValue(parametersList,
SupportedParameters.HEALTH_CHECK_HTTP_ENDPOINT,
getDefaultHealthCheckHttpEndpoint(healthCheckType));
String readinessHealthCheckType = (String) PropertiesUtil.getPropertyValue(parametersList,
SupportedParameters.READINESS_HEALTH_CHECK_TYPE, null);
String readinessHealthCheckHttpEndpoint = (String) PropertiesUtil.getPropertyValue(parametersList,
SupportedParameters.READINESS_HEALTH_CHECK_HTTP_ENDPOINT,
getDefaultHealthCheckHttpEndpoint(
readinessHealthCheckType));
Integer readinessHealthCheckInvocationTimeout = (Integer) PropertiesUtil.getPropertyValue(parametersList,
SupportedParameters.READINESS_HEALTH_CHECK_INVOCATION_TIMEOUT,
null);
Integer readinessHealthCheckInterval = (Integer) PropertiesUtil.getPropertyValue(parametersList,
SupportedParameters.READINESS_HEALTH_CHECK_INTERVAL,
null);
Boolean isSshEnabled = (Boolean) PropertiesUtil.getPropertyValue(parametersList, SupportedParameters.ENABLE_SSH, null);
Map<String, Boolean> appFeatures = getAppFeatures(parametersList);
overrideSshFeatureIfMissing(appFeatures, isSshEnabled);
Expand All @@ -57,6 +69,10 @@ public Staging parse(List<Map<String, Object>> parametersList) {
.invocationTimeout(healthCheckInvocationTimeout)
.healthCheckType(healthCheckType)
.healthCheckHttpEndpoint(healthCheckHttpEndpoint)
.readinessHealthCheckType(readinessHealthCheckType)
.readinessHealthCheckHttpEndpoint(readinessHealthCheckHttpEndpoint)
.readinessHealthCheckInvocationTimeout(readinessHealthCheckInvocationTimeout)
.readinessHealthCheckInterval(readinessHealthCheckInterval)
.isSshEnabled(isSshEnabled)
.appFeatures(appFeatures)
.dockerInfo(dockerInfo)
Expand Down
26 changes: 24 additions & 2 deletions ...java/org/cloudfoundry/multiapps/controller/process/steps/PollStartAppStatusExecution.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.cloudfoundry.multiapps.controller.client.facade.domain.CloudRoute;
import org.cloudfoundry.multiapps.controller.client.facade.domain.InstanceInfo;
import org.cloudfoundry.multiapps.controller.client.facade.domain.InstanceState;
import org.cloudfoundry.multiapps.controller.client.lib.domain.CloudApplicationExtended;
import org.cloudfoundry.multiapps.controller.core.cf.CloudControllerClientFactory;
import org.cloudfoundry.multiapps.controller.core.security.token.TokenService;
import org.cloudfoundry.multiapps.controller.core.util.UriUtil;
Expand Down Expand Up @@ -79,14 +80,13 @@ private StartupStatus getStartupStatus(ProcessContext context, String appName, L
long downInstances = getInstanceCount(appInstances, InstanceState.DOWN);
long crashedInstances = getInstanceCount(appInstances, InstanceState.CRASHED);
long startingInstances = getInstanceCount(appInstances, InstanceState.STARTING);

String states = composeStatesMessage(appInstances);

context.getStepLogger()
.debug(Messages.APPLICATION_0_X_OF_Y_INSTANCES_RUNNING, appName, runningInstances, expectedInstances, states);

if (runningInstances == expectedInstances) {
return StartupStatus.STARTED;
return checkIfAppHasStarted(context, appInstances);
}
if (startingInstances > 0) {
return StartupStatus.STARTING;
Expand All @@ -100,6 +100,28 @@ private StartupStatus getStartupStatus(ProcessContext context, String appName, L
return StartupStatus.STARTING;
}

private StartupStatus checkIfAppHasStarted(ProcessContext context, List<InstanceInfo> appInstances) {
if (shouldWaitForAppToBecomeRoutable(context)) {
if (isThereAtLeastOneRoutedInstance(appInstances)) {
return StartupStatus.STARTED;
}
return StartupStatus.STARTING;
}
return StartupStatus.STARTED;
}

private boolean shouldWaitForAppToBecomeRoutable(ProcessContext context) {
CloudApplicationExtended appToProcess = context.getVariable(Variables.APP_TO_PROCESS);

return appToProcess.getStaging()
.getReadinessHealthCheckType() != null;
}

private boolean isThereAtLeastOneRoutedInstance(List<InstanceInfo> instanceInfos) {
return instanceInfos.stream()
.anyMatch(InstanceInfo::isRoutable);
}
Comment on lines +120 to +123
Copy link
Contributor

@IvanBorislavovDimitrov IvanBorislavovDimitrov Aug 25, 2025

Choose a reason for hiding this comment

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

We will be checking if there is at least one routeable instance? Compared to current approach with the started state, we wait for all instances. This is inconsistency, is it deliberately this way?

Copy link
Contributor Author

@Yavor16 Yavor16 Aug 28, 2025

Choose a reason for hiding this comment

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

Yes, if there is defined readiness health check type, we will wait for one instance to be routable instead of all of them. If there isn't readiness health check type defined, we will work like we do now


private AsyncExecutionState checkStartupStatus(ProcessContext context, CloudApplication app, StartupStatus status) {
if (status == StartupStatus.CRASHED) {
onError(context, Messages.ERROR_STARTING_APP_0_DESCRIPTION_1, app.getName(), Messages.SOME_INSTANCES_HAVE_CRASHED);
Expand Down
Loading