-
Notifications
You must be signed in to change notification settings - Fork 42
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.
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
Add health checks again #1685
Conversation
c459828 to
d50f79c
Compare
.../main/java/org/cloudfoundry/multiapps/controller/client/facade/adapters/RawCloudProcess.java
Outdated
Show resolved
Hide resolved
...t/src/main/java/org/cloudfoundry/multiapps/controller/client/facade/domain/CloudProcess.java
Outdated
Show resolved
Hide resolved
.../main/java/org/cloudfoundry/multiapps/controller/client/facade/adapters/RawCloudProcess.java
Show resolved
Hide resolved
.../org/cloudfoundry/multiapps/controller/client/facade/rest/CloudControllerRestClientImpl.java
Outdated
Show resolved
Hide resolved
| .type("web") | ||
| .uptime(9042L) | ||
| .fileDescriptorQuota(1024L) | ||
| .routable(routable) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
...src/main/java/org/cloudfoundry/multiapps/controller/core/parser/StagingParametersParser.java
Outdated
Show resolved
Hide resolved
theghost5800
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Squash before merge
.../main/java/org/cloudfoundry/multiapps/controller/client/facade/adapters/RawCloudProcess.java
Show resolved
Hide resolved
| private boolean isThereAtLeastOneRoutedInstance(List<InstanceInfo> instanceInfos) { | ||
| return instanceInfos.stream() | ||
| .anyMatch(InstanceInfo::isRoutable); | ||
| } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
| Integer readinessHealthCheckInvocationTimeout = null; | ||
| String readinessHealthCheckHttpEndpoint = null; | ||
| Integer readinessHealthCheckInterval = null; | ||
| if (readinessHealthCheckType.getData() != null) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
...a/org/cloudfoundry/multiapps/controller/process/util/StagingApplicationAttributeUpdater.java
Outdated
Show resolved
Hide resolved
a3c5ad2 to
fcf09bc
Compare
LMCROSSITXSADEPLOY-3140
fcf09bc to
4a9c779
Compare
|


No description provided.