Merge branch 'develop' into CTM-133-cache

Author: lucymcnatt

.github/workflows/chart_update_on_merge.yml

@@ -58,11 +58,16 @@ jobs:
         password: ${{ secrets.DSDEJENKINS_PASSWORD }}
     # Build & push `cromwell`, `womtool`, `cromiam`, and `cromwell-drs-localizer`
     # This step is validated in the GHA 'docker_build_test.yml' without the accompanying docker push
-    - name: Build Cromwell Docker
+    - name: Build & push all images
       run: |
         set -e
         cd cromwell
         sbt -Dproject.isSnapshot=false dockerBuildAndPush
+    - name: Build & push server debug image
+      run: |
+        set -e
+        cd cromwell
+        sbt -Dproject.isDebug=true server/dockerBuildAndPush
     - name: Deploy to dev and board release train (Cromwell)
       uses: broadinstitute/repository-dispatch@master
       with:

CHANGELOG.md

@@ -2,7 +2,16 @@
 
 ## 92 Release Notes
 
+### Migration of PK to BIGINT
+The PK of the JOB_KEY_VALUE_ENTRY table will be migrated from INT to BIGINT.
+
+### AWS ECR Docker Remote Hashing
+* Fixed an issue where ECR images without an explicit repository prefix
+(e.g., `123456789012.dkr.ecr.us-east-1.amazonaws.com/example-tool`) would fail during remote hash computation due to incorrect manifest URI construction. 
+The Docker registry implementation now correctly handles ECR's support for repository-less image paths.
+
 ### AWS Batch
+* Fixed an issue where job failures before all outputs were written would cause delocalization to fail, preventing the upload of return code, stdout, and stderr files needed for debugging.
 * Split the option to tag resources between AWS Batch jobs vs. EC2 and EBS volumes hardware
 * Moved the option to tag job resources from runtime attributes to backend config.
 * Appended the custom labels to the list of resource tags to propagate

database/migration/src/main/resources/changelog.xml

@@ -93,6 +93,7 @@
     <include file="changesets/workflow_store_drop_state_index.xml" relativeToChangelogFile="true" />
     <include file="changesets/add_group_metrics_table.xml" relativeToChangelogFile="true" />
     <include file="changesets/add_group_metrics_unique_constraint.xml" relativeToChangelogFile="true" />
+    <include file="changesets/enlarge_job_key_value_entry_id.xml" relativeToChangelogFile="true" />
     <include file="changesets/add_timestamp_in_call_caching.xml" relativeToChangelogFile="true" />
     <!-- WARNING!
       This changeset should always be last.

database/migration/src/main/resources/changesets/enlarge_job_key_value_entry_id.xml

@@ -0,0 +1,41 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<databaseChangeLog objectQuotingStrategy="QUOTE_ALL_OBJECTS"
+                   xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+                   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.3.xsd">
+
+    <!-- BEGIN JOB_KEY_VALUE_ENTRY PK widening -->
+    <!-- For Postgresql there are 2 changesets: one for modifying the table column type, and another one for altering the autoincrementing sequence.
+              The other DBs can be refactored similarly with a single addAutoIncrement changeset. -->
+    <changeSet author="lbaldo" id="enlarge_job_key_value_entry_id" dbms="mysql,hsqldb,mariadb">
+        <addAutoIncrement
+                columnName="JOB_KEY_VALUE_ENTRY_ID"
+                columnDataType="BIGINT"
+                incrementBy="1"
+                tableName="JOB_KEY_VALUE_ENTRY"
+        />
+    </changeSet>
+
+    <changeSet author="lbaldo" id="postgresql_enlarge_job_key_value_entry_id" dbms="postgresql">
+        <modifyDataType
+                columnName="JOB_KEY_VALUE_ENTRY_ID"
+                tableName="JOB_KEY_VALUE_ENTRY"
+                newDataType="BIGINT"
+        />
+    </changeSet>
+
+    <changeSet author="lbaldoh" id="postgresql_enlarge_job_key_value_entry_id_seq" dbms="postgresql">
+        <preConditions onFail="MARK_RAN">
+            <!-- idempotency check (noop if the sequence is present and already consistent what the alter would do) -->
+            <sqlCheck expectedResult="0">
+                SELECT count(*)
+                FROM information_schema.sequences
+                WHERE sequence_name = 'JOB_KEY_VALUE_ENTRY_JOB_KEY_VALUE_ENTRY_ID_seq'
+                AND data_type = 'bigint';
+            </sqlCheck>
+        </preConditions>
+        <sql>alter sequence "JOB_KEY_VALUE_ENTRY_JOB_KEY_VALUE_ENTRY_ID_seq" as bigint;</sql>
+    </changeSet>
+    <!-- END JOB_KEY_VALUE_ENTRY PK widening -->
+
+</databaseChangeLog>

database/sql/src/main/scala/cromwell/database/slick/tables/JobKeyValueEntryComponent.scala

@@ -9,7 +9,7 @@ trait JobKeyValueEntryComponent {
   import driver.api._
 
   class JobKeyValueEntries(tag: Tag) extends Table[JobKeyValueEntry](tag, "JOB_KEY_VALUE_ENTRY") {
-    def jobKeyValueEntryId = column[Int]("JOB_KEY_VALUE_ENTRY_ID", O.PrimaryKey, O.AutoInc)
+    def jobKeyValueEntryId = column[Long]("JOB_KEY_VALUE_ENTRY_ID", O.PrimaryKey, O.AutoInc)
 
     def workflowExecutionUuid = column[String]("WORKFLOW_EXECUTION_UUID", O.Length(255))
 

database/sql/src/main/scala/cromwell/database/sql/tables/JobKeyValueEntry.scala

@@ -7,5 +7,5 @@ case class JobKeyValueEntry(
   jobAttempt: Int,
   storeKey: String,
   storeValue: String,
-  jobKeyValueEntryId: Option[Int] = None
+  jobKeyValueEntryId: Option[Long] = None
 )

dockerHashing/src/main/scala/cromwell/docker/registryv2/DockerRegistryV2Abstract.scala

@@ -192,6 +192,14 @@ abstract class DockerRegistryV2Abstract(override val config: DockerRegistryConfi
     */
   protected def serviceName: Option[String] = None
 
+  /**
+   * Returns the repository path to use in manifest and token URIs.
+   * Default behavior adds "library" repository for images without an explicit repository.
+   * Registries can override this for custom behavior (e.g., ECR doesn't need "library").
+   */
+  protected def getRepositoryPath(dockerImageID: DockerImageIdentifier): String =
+    dockerImageID.nameWithDefaultRepository
+
   /**
     * Builds the token URI to be queried based on a DockerImageID
     */
@@ -201,7 +209,7 @@ abstract class DockerRegistryV2Abstract(override val config: DockerRegistryConfi
       scheme = Option(Scheme.https),
       authority = Option(Authority(host = Uri.RegName(authorizationServerHostName(dockerImageID)))),
       path = "/token",
-      query = Query.fromString(s"${service}scope=repository:${dockerImageID.nameWithDefaultRepository}:pull")
+      query = Query.fromString(s"${service}scope=repository:${getRepositoryPath(dockerImageID)}:pull")
     )
   }
 
@@ -233,7 +241,7 @@ abstract class DockerRegistryV2Abstract(override val config: DockerRegistryConfi
     Uri.apply(
       scheme = Option(Scheme.https),
       authority = Option(Authority(host = Uri.RegName(registryHostName(dockerImageID)))),
-      path = s"/v2/${dockerImageID.nameWithDefaultRepository}/manifests/${dockerImageID.reference}"
+      path = s"/v2/${getRepositoryPath(dockerImageID)}/manifests/${dockerImageID.reference}"
     )
 
   /**

dockerHashing/src/main/scala/cromwell/docker/registryv2/flows/aws/AmazonEcrAbstract.scala

@@ -11,6 +11,13 @@ import org.slf4j.{Logger, LoggerFactory}
 abstract class AmazonEcrAbstract(override val config: DockerRegistryConfig) extends DockerRegistryV2Abstract(config) {
   private val logger: Logger = LoggerFactory.getLogger(this.getClass)
 
+  /**
+   * ECR supports images with no repository (e.g., 123456789012.dkr.ecr.us-east-1.amazonaws.com/example-tool)
+   * so we don't add the default "library" repository prefix
+   */
+  override protected def getRepositoryPath(dockerImageID: DockerImageIdentifier): String =
+    dockerImageID.name
+
   /**
     * Not used as getToken is overridden
     */