Merge branch 'release-1.32.x' into fixRunJob-1.32.x

.github/workflows/build.yml

@@ -37,6 +37,19 @@ jobs:
         env:
           ORG_GRADLE_PROJECT_version: ${{ steps.build_variables.outputs.VERSION }}
         run: ./gradlew build --stacktrace ${{ steps.build_variables.outputs.REPO }}-web:installDist
+      - name: Build local slim container image for testing
+        uses: docker/build-push-action@v5
+        with:
+          context: .
+          file: Dockerfile.slim
+          load: true
+          platforms: local
+          tags: |
+            "${{ steps.build_variables.outputs.REPO }}:${{ steps.build_variables.outputs.VERSION }}-unvalidated"
+      - name: Test local slim container image
+        env:
+          FULL_DOCKER_IMAGE_NAME: "${{ steps.build_variables.outputs.REPO }}:${{ steps.build_variables.outputs.VERSION }}-unvalidated"
+        run: ./gradlew ${{ steps.build_variables.outputs.REPO }}-integration:test
       - name: Login to GAR
         # Only run this on repositories in the 'spinnaker' org, not on forks.
         if: startsWith(github.repository, 'spinnaker/')

.github/workflows/pr.yml

@@ -51,3 +51,16 @@ jobs:
           tags: |
             "${{ env.CONTAINER_REGISTRY }}/${{ steps.build_variables.outputs.REPO }}:latest-ubuntu"
             "${{ env.CONTAINER_REGISTRY }}/${{ steps.build_variables.outputs.REPO }}:${{ steps.build_variables.outputs.VERSION }}-ubuntu"
+      - name: Build local slim container image for testing
+        uses: docker/build-push-action@v5
+        with:
+          context: .
+          file: Dockerfile.slim
+          load: true
+          platforms: local
+          tags: |
+            "${{ steps.build_variables.outputs.REPO }}:${{ steps.build_variables.outputs.VERSION }}"
+      - name: Test local slim container image
+        env:
+          FULL_DOCKER_IMAGE_NAME: "${{ steps.build_variables.outputs.REPO }}:${{ steps.build_variables.outputs.VERSION }}"
+        run: ./gradlew ${{ steps.build_variables.outputs.REPO }}-integration:test

Dockerfile.slim

@@ -1,9 +1,10 @@
 FROM alpine:3.16
 LABEL maintainer="[email protected]"
-RUN apk --no-cache add --update bash openjdk11-jre
+RUN apk --no-cache add --update bash curl openjdk11-jre
 RUN addgroup -S -g 10111 spinnaker
 RUN adduser -S -G spinnaker -u 10111 spinnaker
 COPY orca-web/build/install/orca /opt/orca
 RUN mkdir -p /opt/orca/plugins && chown -R spinnaker:nogroup /opt/orca/plugins
 USER spinnaker
+HEALTHCHECK CMD curl --fail http://localhost:8083/health
 CMD ["/opt/orca/bin/orca"]

Dockerfile.ubuntu

@@ -1,8 +1,9 @@
 FROM ubuntu:bionic
 LABEL maintainer="[email protected]"
-RUN apt-get update && apt-get -y install openjdk-11-jre-headless wget
+RUN apt-get update && apt-get -y install curl openjdk-11-jre-headless wget
 RUN adduser --system --uid 10111 --group spinnaker
 COPY orca-web/build/install/orca /opt/orca
 RUN mkdir -p /opt/orca/plugins && chown -R spinnaker:nogroup /opt/orca/plugins
 USER spinnaker
+HEALTHCHECK CMD curl --fail http://localhost:8083/health
 CMD ["/opt/orca/bin/orca"]

gradle.properties

@@ -1,5 +1,5 @@
-fiatVersion=2.32.1
-korkVersion=2.32.1
+fiatVersion=1.42.1
+korkVersion=7.188.1
 kotlinVersion=1.4.32
 org.gradle.parallel=true
 spinnakerGradleVersion=8.31.0

keiko-sql/src/main/kotlin/com/netflix/spinnaker/q/sql/SqlQueue.kt

@@ -77,7 +77,8 @@ class SqlQueue(
   override val publisher: EventPublisher,
   private val sqlRetryProperties: SqlRetryProperties,
   private val ULID: ULID = ULID(),
-  private val poolName: String = "default"
+  private val poolName: String = "default",
+  private val containsMessageBatchSize: Int = 100,
 ) : MonitorableQueue {
 
   companion object {
@@ -187,33 +188,32 @@ class SqlQueue(
   }
 
   private fun doContainsMessage(predicate: (Message) -> Boolean): Boolean {
-    val batchSize = 100
+    val batchSize = containsMessageBatchSize
     var found = false
     var lastId = "0"
 
     do {
-      val rs: ResultSet = withRetry(READ) {
+      val rs = withRetry(READ) {
         jooq.select(idField, fingerprintField, bodyField)
           .from(messagesTable)
           .where(idField.gt(lastId))
+          .orderBy(idField.asc())
           .limit(batchSize)
           .fetch()
-          .intoResultSet()
       }
 
-      while (!found && rs.next()) {
+      val rsIterator = rs.iterator()
+      while (!found && rsIterator.hasNext()) {
+        val record = rsIterator.next()
+        val body = record[bodyField, String::class.java]
         try {
-          found = predicate.invoke(mapper.readValue(rs.getString("body")))
+          found = predicate.invoke(mapper.readValue(body))
         } catch (e: Exception) {
-          log.error(
-            "Failed reading message with fingerprint: ${rs.getString("fingerprint")} " +
-              "message: ${rs.getString("body")}",
-            e
-          )
+          log.error("Failed reading message with fingerprint: ${record[fingerprintField, String::class.java]} message: $body", e)
         }
-        lastId = rs.getString("id")
+        lastId = record[idField, String::class.java]
       }
-    } while (!found && rs.row == batchSize)
+    } while (!found && rs.isNotEmpty)
 
     return found
   }

keiko-sql/src/test/kotlin/com/netflix/spinnaker/q/sql/SqlQueueTest.kt

@@ -15,6 +15,12 @@ import com.netflix.spinnaker.q.TestMessage
 import com.netflix.spinnaker.q.metrics.EventPublisher
 import com.netflix.spinnaker.q.metrics.MonitorableQueueTest
 import com.netflix.spinnaker.q.metrics.QueueEvent
+import com.netflix.spinnaker.time.MutableClock
+import com.nhaarman.mockito_kotlin.mock
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.BeforeEach
 import java.time.Clock
 import java.time.Duration
 import java.util.Optional
@@ -37,7 +43,8 @@ private val createQueueNoPublisher = { clock: Clock,
 
 private fun createQueue(clock: Clock,
                         deadLetterCallback: DeadMessageCallback,
-                        publisher: EventPublisher?): SqlQueue {
+                        publisher: EventPublisher?,
+                        containsMessageBatchSize: Int = 5): SqlQueue {
   return SqlQueue(
     queueName = "test",
     schemaVersion = 1,
@@ -66,7 +73,8 @@ private fun createQueue(clock: Clock,
     sqlRetryProperties = SqlRetryProperties(
       transactions = retryPolicy,
       reads = retryPolicy
-    )
+    ),
+    containsMessageBatchSize = containsMessageBatchSize,
   )
 }
 
@@ -78,3 +86,49 @@ private val retryPolicy: RetryProperties = RetryProperties(
   maxRetries = 1,
   backoffMs = 10 // minimum allowed
 )
+
+class SqlQueueSpecificTests {
+  private val batchSize = 5
+  private val clock = MutableClock()
+  private val deadMessageHandler: DeadMessageCallback = mock()
+  private val publisher: EventPublisher = mock()
+  private var queue: SqlQueue? = null
+
+  @BeforeEach
+  fun setup() {
+    queue = createQueue(clock, deadMessageHandler, publisher, batchSize)
+  }
+
+  @AfterEach
+  fun cleanup() {
+    cleanupCallback()
+  }
+
+  @Test
+  fun `doContainsMessage works with no messages present`() {
+    assertThat(doContainsMessagePayload("test")).isFalse
+  }
+
+  @Test
+  fun `doContainsMessage works with a single batch`() {
+    pushTestMessages(batchSize)
+    assertThat(doContainsMessagePayload("${batchSize-1}")).isTrue
+    assertThat(doContainsMessagePayload("")).isFalse
+  }
+
+  @Test
+  fun `doContainsMessage handles multiple batches during search`() {
+    pushTestMessages(batchSize * 2)
+    assertThat(doContainsMessagePayload("${batchSize+1}")).isTrue
+    assertThat(doContainsMessagePayload("")).isFalse
+  }
+
+  private fun pushTestMessages(numberOfMessages: Int) {
+    for (i in 1 .. numberOfMessages) {
+      queue?.push(TestMessage(i.toString()))
+    }
+  }
+
+  private fun doContainsMessagePayload(payload: String): Boolean? =
+    queue?.containsMessage { message -> message is TestMessage && message.payload == payload }
+}

orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/rollback/ExplicitRollback.groovy

@@ -21,6 +21,7 @@ import com.netflix.spinnaker.kork.core.RetrySupport
 import com.netflix.spinnaker.kork.exceptions.SpinnakerException
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
+import com.netflix.spinnaker.orca.clouddriver.config.RollbackConfigurationProperties
 import com.netflix.spinnaker.orca.clouddriver.pipeline.providers.aws.ApplySourceServerGroupCapacityStage
 import com.netflix.spinnaker.orca.clouddriver.pipeline.providers.aws.CaptureSourceServerGroupCapacityStage
 import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.DisableServerGroupStage
@@ -34,6 +35,7 @@ import com.netflix.spinnaker.orca.api.pipeline.SyntheticStageOwner
 import com.netflix.spinnaker.security.AuthenticatedRequest
 import groovy.util.logging.Slf4j
 import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.beans.factory.annotation.Value
 
 import javax.annotation.Nullable
 import java.util.concurrent.Callable
@@ -49,6 +51,10 @@ class ExplicitRollback implements Rollback {
   Boolean disableOnly
   Boolean enableAndDisableOnly
 
+  @Autowired
+  @JsonIgnore
+  RollbackConfigurationProperties rollbackConfigurationProperties;
+
   @JsonIgnore
   private final ExecutorService executor = java.util.concurrent.Executors.newSingleThreadExecutor()
 
@@ -149,6 +155,7 @@ class ExplicitRollback implements Rollback {
 
   @Nullable TargetServerGroup lookupServerGroup(StageExecution parentStage, String serverGroupName) {
     def fromContext = parentStage.mapTo(ResizeStrategy.Source)
+    def rollbackTimeout = rollbackConfigurationProperties.explicitRollback.getTimeout()
 
     try {
       // we use an executor+future to timebox how long we can spend in this remote call
@@ -163,11 +170,11 @@ class ExplicitRollback implements Rollback {
       })
 
       executor.submit(authenticatedRequest)
-        .get(5, TimeUnit.SECONDS)
+        .get(rollbackTimeout, TimeUnit.SECONDS)
         .get()  // not sure what would cause the Optional to not be present but we would catch and log it
     } catch(Exception e) {
       log.error('Could not generate resize stage because there was an error looking up {}', serverGroupName, e)
-      throw new SpinnakerException("failed to look up ${serverGroupName}", e)
+      throw new SpinnakerException("Failed Clouddriver look up for ${serverGroupName} in ${rollbackTimeout} sec. Consider increasing rollback.explicitRollback.timeout in orca profile.", e)
     }
   }
 

orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/config/RollbackConfigurationProperties.java

@@ -0,0 +1,73 @@
+/*
+ * Copyright 2024 Harness, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.spinnaker.orca.clouddriver.config;
+
+import lombok.Data;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+import lombok.Setter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.boot.context.properties.DeprecatedConfigurationProperty;
+import org.springframework.boot.context.properties.NestedConfigurationProperty;
+import org.springframework.context.annotation.Configuration;
+
+@Data
+@Configuration
+@ConfigurationProperties(prefix = "rollback")
+public class RollbackConfigurationProperties {
+
+  private static final Logger logger =
+      LoggerFactory.getLogger(RollbackConfigurationProperties.class);
+
+  @Value("${rollback.timeout.enabled:false}")
+  private boolean dynamicRollbackEnabled;
+
+  @Deprecated
+  @DeprecatedConfigurationProperty(replacement = "rollback.dynamicRollback.enabled")
+  public boolean getDynamicRollbackEnabled() {
+    return dynamicRollbackEnabled;
+  }
+
+  @NestedConfigurationProperty private ExplicitRollback explicitRollback = new ExplicitRollback();
+
+  @NestedConfigurationProperty private DynamicRollback dynamicRollback = new DynamicRollback();
+
+  @Getter
+  @Setter
+  @NoArgsConstructor
+  public static class ExplicitRollback {
+    private int timeout = 5;
+  }
+
+  @Getter
+  @Setter
+  @NoArgsConstructor
+  public static class DynamicRollback {
+    private boolean enabled = false;
+  }
+
+  public boolean isDynamicRollbackEnabled() {
+    if (getDynamicRollbackEnabled()) {
+      logger.warn(
+          "The rollback.timeout.enabled property is deprecated and will be removed in a future release. Please use rollback.dynamicRollback.enabled instead.");
+    }
+    return dynamicRollback.isEnabled() || getDynamicRollbackEnabled();
+  }
+}

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

@@ -257,6 +257,16 @@ ImmutableList<String> getOldManifestNames(DeployedManifest dm) {
     boolean previousDeploymentNeitherStableNorFailed(String account, String location, String name) {
       var oldManifest = this.oortService.getManifest(account, location, name, false);
 
+      Map<String, Double> statusSpec =
+          (Map<String, Double>) oldManifest.getManifest().getOrDefault("status", emptyMap());
+      if (statusSpec.containsKey("readyReplicas") && statusSpec.containsKey("availableReplicas")) {
+        var readyReplicas = statusSpec.get("readyReplicas");
+        var availableReplicas = statusSpec.get("availableReplicas");
+        if (readyReplicas > 0 && availableReplicas > 0) {
+          return false;
+        }
+      }
+
       var status = oldManifest.getStatus();
       var notStable = !status.getStable().isState();
       var notFailed = !status.getFailed().isState();

orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/cluster/DetermineRollbackCandidatesTask.java

@@ -31,6 +31,7 @@
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution;
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService;
 import com.netflix.spinnaker.orca.clouddriver.FeaturesService;
+import com.netflix.spinnaker.orca.clouddriver.config.RollbackConfigurationProperties;
 import com.netflix.spinnaker.orca.clouddriver.model.Cluster;
 import com.netflix.spinnaker.orca.clouddriver.model.ServerGroup;
 import com.netflix.spinnaker.orca.clouddriver.model.ServerGroup.Capacity;
@@ -52,7 +53,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
 
 /**
@@ -92,13 +92,13 @@ public DetermineRollbackCandidatesTask(
       RetrySupport retrySupport,
       CloudDriverService cloudDriverService,
       FeaturesService featuresService,
-      @Value("${rollback.timeout.enabled:false}") boolean dynamicRollbackTimeoutEnabled) {
+      RollbackConfigurationProperties rollbackConfigurationProperties) {
     this.retrySupport = retrySupport;
     this.cloudDriverService = cloudDriverService;
     this.previousImageRollbackSupport =
         new PreviousImageRollbackSupport(
             objectMapper, cloudDriverService, featuresService, retrySupport);
-    this.dynamicRollbackTimeoutEnabled = dynamicRollbackTimeoutEnabled;
+    this.dynamicRollbackTimeoutEnabled = rollbackConfigurationProperties.isDynamicRollbackEnabled();
   }
 
   @Override