Improve test around wait for manifest stable task

.github/workflows/release.yml

@@ -20,7 +20,7 @@ jobs:
       # Given a tag, determine what branch we are on, so we can bump dependencies in the correct branch
       - name: Get Branch
         run: |
-          BRANCHES=$(git branch -r --contains ${{ github.ref }})
+          BRANCHES=$(git branch -r --contains ${{ github.ref }} | grep -v 'HEAD')
           echo "BRANCHES is '${BRANCHES}'"
           # Check for no branches explicitly...Otherwise echo adds a newline so wc thinks there's
           # one branch.  And echo -n makes it appears that there's one less branch than there

gradle.properties

@@ -1,4 +1,4 @@
-fiatVersion=1.53.0
+fiatVersion=1.54.0
 korkVersion=7.251.0
 kotlinVersion=1.6.21
 org.gradle.parallel=true

orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStage.java

@@ -92,20 +92,22 @@ public boolean processExpressions(
 
   @Override
   public void afterStages(@Nonnull StageExecution stage, @Nonnull StageGraphBuilder graph) {
-    TrafficManagement trafficManagement =
-        stage.mapTo("/trafficManagement", TrafficManagement.class);
-    if (trafficManagement.isEnabled()) {
-      switch (trafficManagement.getOptions().getStrategy()) {
-        case RED_BLACK:
-        case BLUE_GREEN:
-          oldManifestActionAppender.deleteOrDisableOldManifest(stage.getContext(), graph);
-          break;
-        case HIGHLANDER:
-          oldManifestActionAppender.disableOldManifest(stage.getContext(), graph);
-          oldManifestActionAppender.deleteOldManifest(stage.getContext(), graph);
-          break;
-        case NONE:
-          // do nothing
+    if (stage.getContext().get("trafficManagement") != null) {
+      TrafficManagement trafficManagement =
+          stage.mapTo("/trafficManagement", TrafficManagement.class);
+      if (trafficManagement.isEnabled()) {
+        switch (trafficManagement.getOptions().getStrategy()) {
+          case RED_BLACK:
+          case BLUE_GREEN:
+            oldManifestActionAppender.deleteOrDisableOldManifest(stage.getContext(), graph);
+            break;
+          case HIGHLANDER:
+            oldManifestActionAppender.disableOldManifest(stage.getContext(), graph);
+            oldManifestActionAppender.deleteOldManifest(stage.getContext(), graph);
+            break;
+          case NONE:
+            // do nothing
+        }
       }
     }
     if (shouldRemoveStageOutputs(stage)) {

orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStageTest.java

@@ -115,6 +115,14 @@ void rolloutStrategyDisabled() {
     assertThat(getAfterStages(stage)).isEmpty();
   }
 
+  @Test
+  void rolloutStrategyMissing() {
+    StageExecutionImpl stage = new StageExecutionImpl();
+    stage.setContext(getContext(DeployManifestContext.builder().build()));
+    stage.getContext().remove("trafficManagement");
+    assertThat(getAfterStages(stage)).isEmpty();
+  }
+
   @Test
   void rolloutStrategyRedBlack() {
     givenManifestIsStable();

orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/tasks/manifest/WaitForManifestStableTaskTest.java

@@ -17,10 +17,15 @@
 package com.netflix.spinnaker.orca.clouddriver.tasks.manifest;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
 import com.netflix.spinnaker.orca.api.pipeline.TaskResult;
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus;
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionType;
@@ -32,6 +37,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import okhttp3.Request;
 import org.assertj.core.api.AssertionsForClassTypes;
 import org.junit.jupiter.api.Test;
 
@@ -148,7 +154,6 @@ void doesNotRecheckManifests() {
 
     reset(oortService);
 
-    verify(oortService, times(0)).getManifest(ACCOUNT, NAMESPACE, MANIFEST_1, false);
     when(oortService.getManifest(ACCOUNT, NAMESPACE, MANIFEST_2, false))
         .thenReturn(manifestBuilder().stable(true).failed(false).build());
 
@@ -160,6 +165,7 @@ void doesNotRecheckManifests() {
                     .putAll(result.getContext())
                     .build()));
     AssertionsForClassTypes.assertThat(result.getStatus()).isEqualTo(ExecutionStatus.SUCCEEDED);
+    verify(oortService, times(0)).getManifest(ACCOUNT, NAMESPACE, MANIFEST_1, false);
   }
 
   @Test
@@ -273,6 +279,44 @@ void waitsForAllManifestsWhenOneFailedAndOneUnknown() {
         .containsExactly(failedMessage(MANIFEST_1), waitingToStabilizeMessage(MANIFEST_2));
   }
 
+  @Test
+  void waitTaskContextIsRestartedWhenClouddriverReturnsException() {
+    OortService oortService = mock(OortService.class);
+    WaitForManifestStableTask task = new WaitForManifestStableTask(oortService);
+
+    StageExecutionImpl myStage =
+        createStageWithManifests(
+            ImmutableMap.of(NAMESPACE, ImmutableList.of(MANIFEST_1, MANIFEST_2)));
+
+    when(oortService.getManifest(ACCOUNT, NAMESPACE, MANIFEST_1, false))
+        .thenReturn(manifestBuilder().stable(true).failed(false).build());
+    when(oortService.getManifest(ACCOUNT, NAMESPACE, MANIFEST_2, false))
+        .thenReturn(manifestBuilder().stable(false).failed(false).build());
+
+    TaskResult result = task.execute(myStage);
+    AssertionsForClassTypes.assertThat(result.getStatus()).isEqualTo(ExecutionStatus.RUNNING);
+    assertThat(getMessages(result)).containsExactly(waitingToStabilizeMessage(MANIFEST_2));
+    assertThat(getErrors(result)).isEmpty();
+
+    reset(oortService);
+
+    when(oortService.getManifest(ACCOUNT, NAMESPACE, MANIFEST_2, false))
+        .thenThrow(
+            new SpinnakerServerException(new Request.Builder().url("http://localhost").build()));
+
+    result =
+        task.execute(
+            createStageWithContext(
+                ImmutableMap.<String, Object>builder()
+                    .putAll(myStage.getContext())
+                    .putAll(result.getContext())
+                    .build()));
+
+    AssertionsForClassTypes.assertThat(result.getStatus()).isEqualTo(ExecutionStatus.RUNNING);
+    assertThat(getMessages(result)).isEmpty();
+    assertThat(getErrors(result)).isEmpty();
+  }
+
   private static String waitingToStabilizeMessage(String manifest) {
     return String.format(
         "'%s' in '%s' for account %s: waiting for manifest to stabilize",