Skip to content

Owls 103828 - Fix for the Cluster and Domain 'Available' conditions for admin only domains and intentionally shutdown clusters #3602

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

Merged
merged 9 commits into from
Nov 1, 2022
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -195,15 +195,22 @@ The following is a list of condition types for a Cluster resource.
* There are no pending server shutdown requests.

#### `Available` {id="cluster-available"}
- The `status` attribute is set to `True` when a sufficient number of WebLogic Server pods are
ready in the cluster. This includes either:
* All WebLogic Server pods in the cluster are ready, as configured in the `cluster.spec.replicas` field,
or if it is not configured, then in the `domain.spec.replicas` field.
* Some of the expected server pods for the cluster are temporarily not ready, and the number of
`not ready` server pods is less than or equal to `cluster.spec.maxUnavailable` which defaults to `1`.
- The `status` attribute is also set to `True` when no WebLogic Server pods are running and this
is the expected state, such as when `cluster.spec.replicas` is set to `0`, or when
`cluster.spec.serverStartPolicy` is set to `Never`.
- The `status` attribute is set to `True` when a sufficient number of WebLogic Server pods are
ready in the cluster. Both of the following must be true:
- At least one pod in the cluster is expected to run and is `ready`.
- The number of `not ready` server pods which are expected to run
Copy link
Member

Choose a reason for hiding this comment

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

pods which are expected to run -> pods, which are expected to run, (two commas needed)

is less than or equal to `cluster.spec.maxUnavailable` which defaults to `1`.
Copy link
Member

@rosemarymarano rosemarymarano Nov 2, 2022

Choose a reason for hiding this comment

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

is less than or equal to cluster.spec.maxUnavailable, which defaults to 1. -> (at a minimum, you need a comma before the word "which')
If you want to make it more explicit, then consider this wording -> is less than or equal to the value of cluster.spec.maxUnavailable, which defaults to 1.

- Examples:
- If a cluster has `serverStartPolicy` `Never` or `replicas` `0`,
or a cluster is in a domain with `serverStartPolicy` `AdminOnly` or `Never`,
then the cluster will have `Available` `False`.
Copy link
Member

Choose a reason for hiding this comment

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

Re-write (I can hardly understand the meaning of this sentence, but here's a stab at it): If a cluster has a serverStartPolicy of Never, (comma) or replicas is 0, or a cluster is in a domain with a serverStartPolicy of AdminOnly or Never, (comma) then the cluster will be Available, False.

- If a cluster and domain both have a `serverStartPolicy` of `IfNeeded`,
and `cluster.spec.replicas` is `1`,
then the cluster `Available` will be `True` only when its single pod is `ready`.
- If a cluster and domain both have a `serverStartPolicy` of `IfNeeded`,
`cluster.spec.replicas` is `4`,
and `cluster.spec.maxUnavailable` is `1` (the default),
then the cluster `Available` will be `True` only when three or four of its pods are `ready`.
- **Note**: The `Available` `status` can be `True` even when the `status` for the `Completed`
condition is `False`, a `Failed` condition is reported on the Domain resource, or the cluster
has up to `cluster.spec.maxUnavailable` pods that are not ready.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,11 @@ private ClusterCheck createFrom(ClusterStatus clusterStatus) {
}

private boolean isProcessingCompleted() {
return !haveTooManyReplicas() && allIntendedServersReady();
return isDomainWithNeverStartPolicy() || (!haveTooManyReplicas() && allIntendedServersReady());
}

private boolean isDomainWithNeverStartPolicy() {
return getDomain().getSpec().getServerStartPolicy() == ServerStartPolicy.NEVER;
}

private boolean haveTooManyReplicas() {
Expand Down Expand Up @@ -835,7 +839,7 @@ private boolean isServerNotReady(@Nonnull String serverName) {
}

private boolean isUnavailable(@Nonnull ClusterCheck clusterCheck) {
return !clusterCheck.isAvailable();
return !clusterCheck.isAvailableOrIntentionallyShutdown();
}

private boolean noApplicationServersReady() {
Expand All @@ -857,13 +861,23 @@ private Collection<DomainPresenceInfo.ServerStartupInfo> getServerStartupInfos()

// when the domain start policy is ADMIN_ONLY, the admin server is considered to be an application server.
private boolean isApplicationServer(String serverName) {
return isAdminOnlyDomain() || !isAdminServer(serverName);
return !isAdminServer(serverName);
}

private boolean isAdminOnlyDomain() {
return isAdminOnlyServerStartPolicy()
|| isOnlyAdminServerRunningInDomain();
}

private boolean isAdminOnlyServerStartPolicy() {
return getDomain().getSpec().getServerStartPolicy() == ServerStartPolicy.ADMIN_ONLY;
}

private boolean isOnlyAdminServerRunningInDomain() {
return expectedRunningServers.size() == 1
&& expectedRunningServers.contains(getInfo().getAdminServerName());
}

private boolean isAdminServer(String serverName) {
return status.getServers().stream()
.filter(s -> s.getServerName().equals(serverName))
Expand Down Expand Up @@ -906,6 +920,10 @@ private List<String> getNonStartedClusteredServers(DomainStatus domainStatus, St
}

boolean isAvailable() {
return sufficientServersReady();
}

boolean isAvailableOrIntentionallyShutdown() {
return isClusterIntentionallyShutDown() || sufficientServersReady();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,7 @@ void whenStatusUnchanged_statusStepDoesNotUpdateDomain() {
.withStateGoal(SHUTDOWN_STATE)
.withServerName("server1")
.withHealth(overallHealth("health1"))))
.addCondition(new DomainCondition(AVAILABLE).withStatus(false)
.withMessage(LOGGER.formatMessage(NO_APPLICATION_SERVERS_READY)))
.addCondition(new DomainCondition(AVAILABLE).withStatus(true))
.addCondition(new DomainCondition(COMPLETED).withStatus(true)));

testSupport.clearNumCalls();
Expand Down Expand Up @@ -1041,15 +1040,15 @@ void whenNumServersStartedBelowMinReplicasForDynamicClusterAndAllowed_domainIsAv
}

@Test
void whenReplicaCountIsZero_domainIsNotAvailable() {
void whenReplicaCountIsZeroAndAdminServerRunning_domainIsAvailable() {
defineScenario()
.withDynamicCluster("cluster1", 3, 4)
.notStarting("ms1", "ms2", "ms3", "ms4")
.build();

updateDomainStatus();

assertThat(getRecordedDomain(), hasCondition(AVAILABLE).withStatus(FALSE));
assertThat(getRecordedDomain(), hasCondition(AVAILABLE).withStatus(TRUE));
}

@Test
Expand Down Expand Up @@ -1108,7 +1107,7 @@ void whenReplicaCountNotWithinMaxUnavailableOfReplicas_establishClusterAvailable
}

@Test
void whenClusterIsIntentionallyShutdown_establishClusterAvailableConditionTrue() {
void whenClusterIsIntentionallyShutdown_establishClusterAvailableConditionFalse() {
configureDomain().configureCluster(info, "cluster1").withReplicas(0).withMaxUnavailable(1);
defineScenario().withDynamicCluster("cluster1", 0, 0).build();
info.getReferencedClusters().forEach(testSupport::defineResources);
Expand All @@ -1119,7 +1118,7 @@ void whenClusterIsIntentionallyShutdown_establishClusterAvailableConditionTrue()
assertThat(clusterStatus.getConditions().size(), equalTo(2));
ClusterCondition condition = clusterStatus.getConditions().get(0);
assertThat(condition.getType(), equalTo(ClusterConditionType.AVAILABLE));
assertThat(condition.getStatus(), equalTo(TRUE));
assertThat(condition.getStatus(), equalTo(FALSE));
}

@Test
Expand Down Expand Up @@ -1267,15 +1266,15 @@ void withAllServersShutdown_domainIsCompleted() { // !!!! can the admin server
}

@Test
void withClusterIntentionallyShutdown_domainIsCompleted() {
void withClusterIntentionallyShutdownAndAdminServerRunning_domainIsAvailableAndCompleted() {
defineScenario()
.withCluster("cluster1", "ms1", "ms2")
.notStarting("ms1", "ms2")
.build();

updateDomainStatus();

assertThat(getRecordedDomain(), hasCondition(AVAILABLE).withStatus(FALSE));
assertThat(getRecordedDomain(), hasCondition(AVAILABLE).withStatus(TRUE));
assertThat(getRecordedDomain(), hasCondition(COMPLETED).withStatus(TRUE));
}

Expand Down Expand Up @@ -1621,6 +1620,16 @@ void whenAdminOnlyAndAdminServerIsReady_availableIsTrue() {
assertThat(getRecordedDomain(), hasCondition(AVAILABLE).withStatus(TRUE));
}

@Test
void whenDomainHasNeverStartPolicy_completedIsTrue() {
configureDomain().withDefaultServerStartPolicy(ServerStartPolicy.NEVER);
defineScenario().build();

updateDomainStatus();

assertThat(getRecordedDomain(), hasCondition(COMPLETED).withStatus(TRUE));
}

@Test
void whenAdminOnlyAndAdminServerIsNotReady_availableIsFalse() {
configureDomain().withDefaultServerStartPolicy(ServerStartPolicy.ADMIN_ONLY);
Expand Down Expand Up @@ -1736,11 +1745,56 @@ void whenServerStartupInfoIsNull_availableIsFalse() {
hasItems(new ClusterCondition(ClusterConditionType.AVAILABLE).withStatus(FALSE)));
}

@Test
void whenDomainOnlyHasAdminServer_availableIsTrue() {
configureDomain().configureAdminServer();
defineScenario().build();

updateDomainStatus();

assertThat(getRecordedDomain(), hasCondition(AVAILABLE).withStatus(TRUE));
}

private Collection<ClusterCondition> getClusterConditions() {
return testSupport.<ClusterResource>getResourceWithName(KubernetesTestSupport.CLUSTER, "cluster1")
.getStatus().getConditions();
}

@Test
void whenClusterIntentionallyShutdown_clusterAvailableIsFalseAndDomainAvailableIsTrue() {
configureDomain()
.configureCluster(info, "cluster1").withReplicas(0);
info.getReferencedClusters().forEach(testSupport::defineResources);
defineScenario()
.withCluster("cluster1", "server1", "server2")
.notStarting("server1", "server2")
.build();

updateDomainStatus();

assertThat(getClusterConditions(),
hasItems(new ClusterCondition(ClusterConditionType.AVAILABLE).withStatus(FALSE)));
assertThat(getRecordedDomain(), hasCondition(AVAILABLE).withStatus(TRUE));
}

@Test
void whenClusterIntentionallyShutdownAndSSINotConstructed_clusterAndDomainAvailableIsFalse() {
configureDomain()
.configureCluster(info, "cluster1").withReplicas(0);
info.getReferencedClusters().forEach(testSupport::defineResources);
defineScenario()
.withCluster("cluster1", "server1", "server2")
.notStarting("server1", "server2")
.build();
info.setServerStartupInfo(null);

updateDomainStatus();

assertThat(getClusterConditions(),
hasItems(new ClusterCondition(ClusterConditionType.AVAILABLE).withStatus(FALSE)));
assertThat(getRecordedDomain(), hasCondition(AVAILABLE).withStatus(FALSE));
}

@SuppressWarnings("SameParameterValue")
private ScenarioBuilder defineScenario() {
return new ScenarioBuilder();
Expand Down