OWLS-107005 correct Failed condition handling at the async request processing layer (#4030)

* Correct Failed condition handling at the async request processing layer

Author: doxiao

operator/src/main/java/oracle/kubernetes/operator/ClusterResourceStatusUpdater.java

@@ -28,7 +28,6 @@
 import oracle.kubernetes.operator.work.NextAction;
 import oracle.kubernetes.operator.work.Packet;
 import oracle.kubernetes.operator.work.Step;
-import oracle.kubernetes.operator.work.Step.StepAndPacket;
 import oracle.kubernetes.weblogic.domain.model.ClusterCondition;
 import oracle.kubernetes.weblogic.domain.model.ClusterResource;
 import oracle.kubernetes.weblogic.domain.model.ClusterStatus;
@@ -59,15 +58,6 @@ public static Step createClusterResourceStatusUpdaterStep(Step next) {
     return new ClusterResourceStatusUpdaterStep(next);
   }
 
-  private static Step createUpdateClusterResourceStatusSteps(Packet packet,
-      Collection<ClusterResource> clusterResources) {
-    List<StepAndPacket> result = clusterResources.stream()
-        .filter(res -> createContext(packet, res).isClusterResourceStatusChanged())
-        .map(res -> createContext(packet, res).createReplaceClusterResourceStatusStep()).collect(
-        Collectors.toList());
-    return result.isEmpty() ? null : new RunInParallelStep(result);
-  }
-
   private static ReplaceClusterStatusContext createContext(Packet packet, ClusterResource resource) {
     return new ReplaceClusterStatusContext(packet, resource);
   }
@@ -93,6 +83,15 @@ public NextAction apply(Packet packet) {
       return doNext(chainStep(step, getNext()), packet);
     }
 
+    private static Step createUpdateClusterResourceStatusSteps(Packet packet,
+                                                               Collection<ClusterResource> clusterResources) {
+      List<StepAndPacket> result = clusterResources.stream()
+          .filter(res -> createContext(packet, res).isClusterResourceStatusChanged())
+          .map(res -> new StepAndPacket(createContext(packet, res).createReplaceClusterResourceStatusStep(), packet))
+          .collect(Collectors.toList());
+      return result.isEmpty() ? null : new RunInParallelStep(result);
+    }
+
     private static Step chainStep(Step one, Step two) {
       if (one == null) {
         return two;
@@ -210,7 +209,7 @@ boolean isClusterResourceStatusChanged() {
       return !Objects.equals(getNewStatus(), resource.getStatus());
     }
 
-    private StepAndPacket createReplaceClusterResourceStatusStep() {
+    private Step createReplaceClusterResourceStatusStep() {
       LOGGER.fine(MessageKeys.CLUSTER_STATUS, getClusterResourceName(),
           getNewStatus());
 
@@ -229,7 +228,7 @@ private StepAndPacket createReplaceClusterResourceStatusStep() {
           .map(ncs -> getClusterStatusConditionEvents(ncs.getConditions())).orElse(Collections.emptyList())
           .stream().map(EventHelper::createClusterResourceEventStep).forEach(result::add);
 
-      return new StepAndPacket(Step.chain(result), packet);
+      return Step.chain(result);
     }
 
     private List<EventData> getClusterStatusConditionEvents(List<ClusterCondition> conditions) {
@@ -317,7 +316,7 @@ public NextAction apply(Packet packet) {
       // Get the ClusterResource, that was refreshed, from DomainPresenceInfo.
       DomainPresenceInfo info = DomainPresenceInfo.fromPacket(packet).orElseThrow();
       ClusterResource res = info.getClusterResource(clusterName);
-      return doNext(createUpdateClusterResourceStatusSteps(packet, Collections.singletonList(res)), packet);
+      return doNext(createContext(packet, res).createReplaceClusterResourceStatusStep(), packet);
     }
   }
 }

operator/src/main/java/oracle/kubernetes/operator/DomainStatusUpdater.java

@@ -765,7 +765,11 @@ class Conditions {
         public Conditions(DomainStatus status) {
           this.status = status != null ? status : new DomainStatus();
           this.clusterChecks = createClusterChecks();
-          conditionList.add(new DomainCondition(COMPLETED).withStatus(isProcessingCompleted()));
+          boolean isCompleted = isProcessingCompleted();
+          conditionList.add(new DomainCondition(COMPLETED).withStatus(isCompleted));
+          if (isCompleted && this.status.hasConditionWithType(FAILED)) {
+            this.status.removeConditionsWithType(FAILED);
+          }
           conditionList.add(createAvailableCondition());
           if (allIntendedServersReady()) {
             this.status.removeConditionsWithType(ROLLING);

operator/src/main/java/oracle/kubernetes/operator/helpers/ResponseStep.java

@@ -1,4 +1,4 @@
-// Copyright (c) 2018, 2022, Oracle and/or its affiliates.
+// Copyright (c) 2018, 2023, Oracle and/or its affiliates.
 // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
 
 package oracle.kubernetes.operator.helpers;
@@ -26,6 +26,7 @@
 import oracle.kubernetes.weblogic.domain.model.DomainCondition;
 import oracle.kubernetes.weblogic.domain.model.DomainResource;
 
+import static oracle.kubernetes.operator.KubernetesConstants.HTTP_CONFLICT;
 import static oracle.kubernetes.operator.KubernetesConstants.HTTP_FORBIDDEN;
 import static oracle.kubernetes.operator.KubernetesConstants.HTTP_NOT_FOUND;
 import static oracle.kubernetes.operator.KubernetesConstants.HTTP_UNAUTHORIZED;
@@ -177,7 +178,9 @@ private NextAction doPotentialRetry(Step conflictStep, Packet packet, CallRespon
   }
 
   private NextAction logNoRetry(Packet packet, CallResponse<T> callResponse) {
-    if ((callResponse != null) && (callResponse.getStatusCode() != HTTP_NOT_FOUND)) {
+    if ((callResponse != null)
+        && (callResponse.getStatusCode() != HTTP_NOT_FOUND)
+        && (callResponse.getStatusCode() != HTTP_CONFLICT)) {
       addDomainFailureStatus(packet, callResponse.getRequestParams(), callResponse.getE());
       if (LOGGER.isWarningEnabled()) {
         LOGGER.warning(

operator/src/test/java/oracle/kubernetes/operator/calls/AsyncRequestStepTest.java

@@ -1,4 +1,4 @@
-// Copyright (c) 2018, 2022, Oracle and/or its affiliates.
+// Copyright (c) 2018, 2023, Oracle and/or its affiliates.
 // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
 
 package oracle.kubernetes.operator.calls;
@@ -196,6 +196,14 @@ void afterFailedCallback_failedStatusConditionSet() {
     ));
   }
 
+  @Test
+  void afterFailedCallback409_failedStatusConditionNotSet() {
+    testSupport.addDomainPresenceInfo(info);
+    sendFailedCallback(HttpURLConnection.HTTP_CONFLICT);
+
+    assertThat(domain.getStatus().hasConditionWithType(FAILED), is(false));
+  }
+
   @Test
   void whenDomainStatusIsNull_ignoreSuccess() {
     assertDoesNotThrow(() -> {