From 3914e22a2fe0f4d010434150483491aa109a86b7 Mon Sep 17 00:00:00 2001 From: Russell Gold Date: Thu, 8 Jul 2021 15:25:24 -0400 Subject: [PATCH] Don't remove other conditions when adding ConfigChangesPendingRestart --- .../domain/model/DomainConditionType.java | 17 ++---- .../weblogic/domain/model/DomainStatus.java | 19 +++---- .../domain/model/DomainStatusTest.java | 57 +++++++++---------- 3 files changed, 38 insertions(+), 55 deletions(-) diff --git a/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/DomainConditionType.java b/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/DomainConditionType.java index d3e54c6918f..c41fcd55f14 100644 --- a/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/DomainConditionType.java +++ b/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/DomainConditionType.java @@ -13,13 +13,8 @@ DomainConditionType[] typesToRemove() { Available, ConfigChangesPendingRestart { @Override - String getStatusMessage(DomainCondition condition) { - return condition.getMessage(); - } - - @Override - String getStatusReason(DomainCondition condition) { - return condition.getReason(); + DomainConditionType[] typesToRemove() { + return new DomainConditionType[] {}; } }, Failed { @@ -34,8 +29,8 @@ String getStatusReason(DomainCondition condition) { } @Override - DomainConditionType[] typesToRemoveAlways() { - return new DomainConditionType[] {Progressing}; + DomainConditionType[] typesToRemove() { + return new DomainConditionType[] {Progressing, Available}; } }; @@ -43,10 +38,6 @@ DomainConditionType[] typesToRemove() { return new DomainConditionType[] {Progressing, Available, Failed}; } - DomainConditionType[] typesToRemoveAlways() { - return new DomainConditionType[] {}; - } - String getStatusMessage(DomainCondition condition) { return null; } diff --git a/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/DomainStatus.java b/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/DomainStatus.java index a25dbd2f034..0753e7d78ee 100644 --- a/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/DomainStatus.java +++ b/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/DomainStatus.java @@ -5,6 +5,7 @@ import java.time.OffsetDateTime; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.HashSet; @@ -116,13 +117,10 @@ public DomainStatus(DomainStatus that) { */ public DomainStatus addCondition(DomainCondition newCondition) { if (conditions.contains(newCondition)) { - conditions = conditions.stream() - .filter(c -> preserve(c, newCondition.getType().typesToRemoveAlways())).collect(Collectors.toList()); return this; } - conditions = conditions.stream() - .filter(c -> preserve(c, newCondition.getType().typesToRemove())).collect(Collectors.toList()); + conditions = conditions.stream().filter(c -> preserve(c, newCondition.getType())).collect(Collectors.toList()); conditions.add(newCondition); reason = newCondition.getStatusReason(); @@ -130,14 +128,13 @@ public DomainStatus addCondition(DomainCondition newCondition) { return this; } - private boolean preserve(DomainCondition condition, DomainConditionType[] types) { - for (DomainConditionType type : types) { - if (condition.getType() == type) { - return false; - } + // Returns true if adding a condition of the new type should not remove the specified condition. + private boolean preserve(DomainCondition condition, DomainConditionType newType) { + if (newType == condition.getType() && !"True".equalsIgnoreCase(condition.getStatus())) { + return false; + } else { + return !Arrays.asList(newType.typesToRemove()).contains(condition.getType()); } - - return true; } /** diff --git a/operator/src/test/java/oracle/kubernetes/weblogic/domain/model/DomainStatusTest.java b/operator/src/test/java/oracle/kubernetes/weblogic/domain/model/DomainStatusTest.java index 449d7a3b712..8a57267194d 100644 --- a/operator/src/test/java/oracle/kubernetes/weblogic/domain/model/DomainStatusTest.java +++ b/operator/src/test/java/oracle/kubernetes/weblogic/domain/model/DomainStatusTest.java @@ -18,6 +18,7 @@ import static oracle.kubernetes.operator.WebLogicConstants.SHUTDOWN_STATE; import static oracle.kubernetes.weblogic.domain.model.DomainConditionMatcher.hasCondition; import static oracle.kubernetes.weblogic.domain.model.DomainConditionType.Available; +import static oracle.kubernetes.weblogic.domain.model.DomainConditionType.ConfigChangesPendingRestart; import static oracle.kubernetes.weblogic.domain.model.DomainConditionType.Failed; import static oracle.kubernetes.weblogic.domain.model.DomainConditionType.Progressing; import static oracle.kubernetes.weblogic.domain.model.DomainStatusTest.ClusterStatusMatcher.clusterStatus; @@ -64,38 +65,12 @@ public void whenAddedConditionEqualsPresentCondition_ignoreIt() { } @Test - public void whenAddedConditionProgressing_addedItWithoutRemovingFailed() { - DomainCondition originalCondition1 = new DomainCondition(Failed).withStatus("True"); - domainStatus.addCondition(originalCondition1); - DomainCondition originalCondition2 = new DomainCondition(Progressing).withStatus("True"); - domainStatus.addCondition(originalCondition2); + public void whenAddedConditionIsFailed_retainOldFailedCondition() { + domainStatus.addCondition(new DomainCondition(Failed).withStatus("True").withMessage("problem 1")); + domainStatus.addCondition(new DomainCondition(Failed).withStatus("True").withMessage("problem 2")); - assertThat(domainStatus.getConditionWithType(Failed), sameInstance(originalCondition1)); - assertThat(domainStatus.getConditionWithType(Progressing), sameInstance(originalCondition2)); - } - - @Test - public void whenAddedConditionFailed_removeProgressingCondition() { - DomainCondition originalCondition1 = new DomainCondition(Failed).withStatus("True"); - domainStatus.addCondition(originalCondition1); - DomainCondition originalCondition2 = new DomainCondition(Progressing).withStatus("True"); - domainStatus.addCondition(originalCondition2); - - SystemClockTestSupport.increment(); - domainStatus.addCondition(new DomainCondition(Failed).withStatus("True")); - - assertThat(domainStatus.getConditionWithType(Failed), sameInstance(originalCondition1)); - assertThat(domainStatus.getConditionWithType(Progressing), is(nullValue())); - } - - @Test - public void whenAddedConditionIsFailed_replaceOldFailedCondition() { - domainStatus.addCondition(new DomainCondition(Failed).withStatus("False")); - - domainStatus.addCondition(new DomainCondition(Failed).withStatus("True")); - - assertThat(domainStatus, hasCondition(Failed).withStatus("True")); - assertThat(domainStatus, not(hasCondition(Failed).withStatus("False"))); + assertThat(domainStatus, hasCondition(Failed).withMessage("problem 1")); + assertThat(domainStatus, hasCondition(Failed).withMessage("problem 2")); } @Test @@ -178,6 +153,26 @@ public void whenAddedConditionIsProgress_doNotRmoveExistedFailedCondition() { assertThat(domainStatus, hasCondition(Progressing).withStatus("True")); } + @Test + public void whenAddedConditionIsConfigChangesPending_doNotRemoveExistingFailedCondition() { + domainStatus.addCondition(new DomainCondition(Failed).withStatus("True")); + + domainStatus.addCondition(new DomainCondition(ConfigChangesPendingRestart).withStatus("True")); + + assertThat(domainStatus, hasCondition(Failed)); + assertThat(domainStatus, hasCondition(ConfigChangesPendingRestart)); + } + + @Test + public void whenAddedConditionIsConfigChangesPending_doNotRemoveExistingAvailableCondition() { + domainStatus.addCondition(new DomainCondition(Available)); + + domainStatus.addCondition(new DomainCondition(ConfigChangesPendingRestart).withStatus("True")); + + assertThat(domainStatus, hasCondition(Available)); + assertThat(domainStatus, hasCondition(ConfigChangesPendingRestart)); + } + @Test public void beforeConditionAdded_statusFailsPredicate() { assertThat(domainStatus.hasConditionWith(c -> c.hasType(Available)), is(false));