Skip to content

Commit d0037e4

Browse files
LizBaldoLiz Baldo
andauthored
CTM-279: copy fix over from PR #7826 (#7833)
Co-authored-by: Liz Baldo <lizbaldo@Liz-Baldo-H5267Y2XDX.local>
1 parent 4791bd6 commit d0037e4

File tree

3 files changed

+115
-103
lines changed

3 files changed

+115
-103
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## 92 Release Notes
44

55
### AWS Batch
6+
* 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.
67
* Split the option to tag resources between AWS Batch jobs vs. EC2 and EBS volumes hardware
78
* Moved the option to tag job resources from runtime attributes to backend config.
89
* Appended the custom labels to the list of resource tags to propagate

supportedBackends/aws/src/main/scala/cromwell/backend/impl/aws/AwsBatchJob.scala

Lines changed: 108 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -465,114 +465,121 @@ final case class AwsBatchJob(
465465
val stdErr = dockerStderr.replace("/cromwell_root", workDir)
466466

467467
// generate a series of s3 commands to delocalize artifacts from the container to storage at the end of the task
468-
val outputCopyCommand = outputs
469-
.map {
470-
// local is relative path, no mountpoint disk in front.
471-
case output: AwsBatchFileOutput if output.local.pathAsString.contains("*") => "" // filter out globs
472-
case output: AwsBatchFileOutput if output.s3key.endsWith(".list") && output.s3key.contains("glob-") =>
473-
Log.debug("Globbing : check for EFS settings.")
474-
val s3GlobOutDirectory = output.s3key.replace(".list", "")
475-
// glob paths are not generated with 127 char limit, using generateGlobPaths(). name can be used safely
476-
val globDirectory = output.name.replace(".list", "")
477-
/*
478-
* Need to process this list and de-localize each file if the list file actually exists
479-
* if it doesn't exist then 'touch' it so that it can be copied otherwise later steps will get upset
480-
* about the missing file
481-
*/
482-
if (efsMntPoint.isDefined && output.mount.mountPoint.pathAsString == efsMntPoint.get) {
483-
Log.debug(
484-
"EFS glob output file detected: " + output.s3key + s" / ${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}"
485-
)
486-
val test_cmd = if (efsDelocalize.isDefined && efsDelocalize.getOrElse(false)) {
487-
Log.debug("delocalization on EFS is enabled")
488-
Log.debug(s"Delocalizing $globDirectory to $s3GlobOutDirectory\n")
468+
val outputCopyCommand = {
469+
val coreDelocalize =
470+
s"""
471+
|if [ -f "$workDir/${jobPaths.returnCodeFilename}" ]; then _s3_delocalize_with_retry "$workDir/${jobPaths.returnCodeFilename}" "${jobPaths.callRoot.pathAsString}/${jobPaths.returnCodeFilename}" ; fi
472+
|if [ -f "$stdErr" ]; then _s3_delocalize_with_retry "$stdErr" "${jobPaths.standardPaths.error.pathAsString}"; fi
473+
|if [ -f "$stdOut" ]; then _s3_delocalize_with_retry "$stdOut" "${jobPaths.standardPaths.output.pathAsString}"; fi
474+
|""".stripMargin
475+
476+
val otherOutputs = outputs
477+
.map {
478+
// local is relative path, no mountpoint disk in front.
479+
case output: AwsBatchFileOutput if output.local.pathAsString.contains("*") => "" // filter out globs
480+
case output: AwsBatchFileOutput if output.s3key.endsWith(".list") && output.s3key.contains("glob-") =>
481+
Log.debug("Globbing : check for EFS settings.")
482+
val s3GlobOutDirectory = output.s3key.replace(".list", "")
483+
// glob paths are not generated with 127 char limit, using generateGlobPaths(). name can be used safely
484+
val globDirectory = output.name.replace(".list", "")
485+
/*
486+
* Need to process this list and de-localize each file if the list file actually exists
487+
* if it doesn't exist then 'touch' it so that it can be copied otherwise later steps will get upset
488+
* about the missing file
489+
*/
490+
if (efsMntPoint.isDefined && output.mount.mountPoint.pathAsString == efsMntPoint.get) {
491+
Log.debug(
492+
"EFS glob output file detected: " + output.s3key + s" / ${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}"
493+
)
494+
val test_cmd = if (efsDelocalize.isDefined && efsDelocalize.getOrElse(false)) {
495+
Log.debug("delocalization on EFS is enabled")
496+
Log.debug(s"Delocalizing $globDirectory to $s3GlobOutDirectory\n")
497+
s"""
498+
|touch "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}"
499+
|_s3_delocalize_with_retry "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" "${output.s3key}"
500+
|if [ -e $globDirectory ]; then _s3_delocalize_with_retry "$globDirectory" "$s3GlobOutDirectory" ; fi
501+
|""".stripMargin
502+
} else {
503+
504+
// check file for existence
505+
s"""
506+
|# test the glob list
507+
|test -e "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" || (echo 'output file: ${output.mount.mountPoint.pathAsString}/${output.local.pathAsString} does not exist' && DELOCALIZATION_FAILED=1)
508+
|# test individual files.
509+
|SAVEIFS="$$IFS"
510+
|IFS=$$'\n'
511+
|for F in $$(cat "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}"); do
512+
| test -e "${globDirectory}/$$F" || (echo 'globbed file: "${globDirectory}/$$F" does not exist' && DELOCALIZATION_FAILED=1 && break)
513+
|done
514+
|IFS="$$SAVEIFS"
515+
|"""
516+
}
517+
// need to make md5sum?
518+
val md5_cmd = if (efsMakeMD5.isDefined && efsMakeMD5.getOrElse(false)) {
519+
Log.debug("Add cmd to create MD5 sibling.")
520+
// generate MD5 if missing or if local file is newer than sibling md5
521+
s"""
522+
|if [[ ! -f '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}.md5' || '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}' -nt '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}.md5' ]]; then
523+
| # the glob list
524+
| md5sum '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}' > '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}.md5' || (echo 'Could not generate ${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}.md5' && DELOCALIZATION_FAILED=1 );
525+
| # globbed files, using specified number of cpus for parallel processing.
526+
| SAVEIFS="$$IFS"
527+
| IFS=$$'\n'
528+
| cat "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" | xargs -I% -P${runtimeAttributes.cpu.##.toString} bash -c "md5sum ${globDirectory}/% > ${globDirectory}/%.md5"
529+
| IFS="$$SAVEIFS"
530+
|fi
531+
|""".stripMargin
532+
}
533+
// return combined result
489534
s"""
490-
|touch "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}"
491-
|_s3_delocalize_with_retry "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" "${output.s3key}"
492-
|if [ -e $globDirectory ]; then _s3_delocalize_with_retry "$globDirectory" "$s3GlobOutDirectory" ; fi
493-
|""".stripMargin
535+
|${test_cmd}
536+
|${md5_cmd}
537+
| """.stripMargin
494538
} else {
495-
496-
// check file for existence
539+
// default delocalization command.
540+
Log.debug(s"Delocalize from ${output.name} to ${output.s3key}\n")
497541
s"""
498-
|# test the glob list
499-
|test -e "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" || (echo 'output file: ${output.mount.mountPoint.pathAsString}/${output.local.pathAsString} does not exist' && DELOCALIZATION_FAILED=1)
500-
|# test individual files.
501-
|SAVEIFS="$$IFS"
502-
|IFS=$$'\n'
503-
|for F in $$(cat "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}"); do
504-
| test -e "${globDirectory}/$$F" || (echo 'globbed file: "${globDirectory}/$$F" does not exist' && DELOCALIZATION_FAILED=1 && break)
505-
|done
506-
|IFS="$$SAVEIFS"
507-
|"""
542+
|touch "${output.name}"
543+
|_s3_delocalize_with_retry "${output.name}" "${output.s3key}"
544+
|if [ -e "$globDirectory" ]; then _s3_delocalize_with_retry "$globDirectory" "$s3GlobOutDirectory" ; fi""".stripMargin
508545
}
509-
// need to make md5sum?
510-
val md5_cmd = if (efsMakeMD5.isDefined && efsMakeMD5.getOrElse(false)) {
511-
Log.debug("Add cmd to create MD5 sibling.")
512-
// generate MD5 if missing or if local file is newer than sibling md5
513-
s"""
514-
|if [[ ! -f '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}.md5' || '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}' -nt '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}.md5' ]]; then
515-
| # the glob list
516-
| md5sum '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}' > '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}.md5' || (echo 'Could not generate ${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}.md5' && DELOCALIZATION_FAILED=1 );
517-
| # globbed files, using specified number of cpus for parallel processing.
518-
| SAVEIFS="$$IFS"
519-
| IFS=$$'\n'
520-
| cat "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" | xargs -I% -P${runtimeAttributes.cpu.##.toString} bash -c "md5sum ${globDirectory}/% > ${globDirectory}/%.md5"
521-
| IFS="$$SAVEIFS"
522-
|fi
523-
|""".stripMargin
546+
547+
// files on /cromwell/ working dir must be delocalized
548+
case output: AwsBatchFileOutput
549+
if output.s3key.startsWith(
550+
"s3://"
551+
) && output.mount.mountPoint.pathAsString == AwsBatchWorkingDisk.MountPoint.pathAsString =>
552+
// output is on working disk mount
553+
s"""_s3_delocalize_with_retry "$workDir/${output.local.pathAsString}" "${output.s3key}" "${output.optional}" """.stripMargin
554+
555+
// files on EFS mounts are optionally delocalized.
556+
case output: AwsBatchFileOutput
557+
if efsMntPoint.isDefined && output.mount.mountPoint.pathAsString == efsMntPoint.get =>
558+
Log.debug(
559+
"EFS output file detected: " + output.s3key + s" / ${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}"
560+
)
561+
// EFS located file : test existence or delocalize.
562+
if (efsDelocalize.isDefined && efsDelocalize.getOrElse(false)) {
563+
Log.debug("efs-delocalization enabled")
564+
s"""_s3_delocalize_with_retry "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" "${output.s3key}" "${output.optional}" """.stripMargin
565+
} else {
566+
Log.debug("efs-delocalization disabled")
567+
s"""_check_efs_outfile "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" "${efsMakeMD5
568+
.getOrElse(false)}" "${output.optional}" """.stripMargin
524569
}
525-
// return combined result
526-
s"""
527-
|${test_cmd}
528-
|${md5_cmd}
529-
| """.stripMargin
530-
} else {
531-
// default delocalization command.
532-
Log.debug(s"Delocalize from ${output.name} to ${output.s3key}\n")
533-
s"""
534-
|touch "${output.name}"
535-
|_s3_delocalize_with_retry "${output.name}" "${output.s3key}"
536-
|if [ -e "$globDirectory" ]; then _s3_delocalize_with_retry "$globDirectory" "$s3GlobOutDirectory" ; fi""".stripMargin
537-
}
538570

539-
// files on /cromwell/ working dir must be delocalized
540-
case output: AwsBatchFileOutput
541-
if output.s3key.startsWith(
542-
"s3://"
543-
) && output.mount.mountPoint.pathAsString == AwsBatchWorkingDisk.MountPoint.pathAsString =>
544-
// output is on working disk mount
545-
s"""_s3_delocalize_with_retry "$workDir/${output.local.pathAsString}" "${output.s3key}" "${output.optional}" """.stripMargin
546-
547-
// files on EFS mounts are optionally delocalized.
548-
case output: AwsBatchFileOutput
549-
if efsMntPoint.isDefined && output.mount.mountPoint.pathAsString == efsMntPoint.get =>
550-
Log.debug(
551-
"EFS output file detected: " + output.s3key + s" / ${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}"
552-
)
553-
// EFS located file : test existence or delocalize.
554-
if (efsDelocalize.isDefined && efsDelocalize.getOrElse(false)) {
555-
Log.debug("efs-delocalization enabled")
556-
s"""_s3_delocalize_with_retry "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" "${output.s3key}" "${output.optional}" """.stripMargin
557-
} else {
558-
Log.debug("efs-delocalization disabled")
559-
s"""_check_efs_outfile "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" "${efsMakeMD5
560-
.getOrElse(false)}" "${output.optional}" """.stripMargin
561-
}
571+
case output: AwsBatchFileOutput =>
572+
// output on a different mount
573+
Log.debug("output data on other mount")
574+
Log.debug(output.toString)
575+
s"""_s3_delocalize_with_retry "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" "${output.s3key}" """.stripMargin
576+
case _ => ""
577+
}
578+
.mkString("\n")
562579

563-
case output: AwsBatchFileOutput =>
564-
// output on a different mount
565-
Log.debug("output data on other mount")
566-
Log.debug(output.toString)
567-
s"""_s3_delocalize_with_retry "${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}" "${output.s3key}" """.stripMargin
568-
case _ => ""
569-
}
570-
.mkString("\n") + "\n" +
571-
s"""
572-
|if [ -f "$workDir/${jobPaths.returnCodeFilename}" ]; then _s3_delocalize_with_retry "$workDir/${jobPaths.returnCodeFilename}" "${jobPaths.callRoot.pathAsString}/${jobPaths.returnCodeFilename}" ; fi
573-
|if [ -f "$stdErr" ]; then _s3_delocalize_with_retry "$stdErr" "${jobPaths.standardPaths.error.pathAsString}"; fi
574-
|if [ -f "$stdOut" ]; then _s3_delocalize_with_retry "$stdOut" "${jobPaths.standardPaths.output.pathAsString}"; fi
575-
|""".stripMargin
580+
// Prepend core delocalize lines so rc, stderr, stdout are always delocalized first.
581+
coreDelocalize + "\n" + otherOutputs + "\n"
582+
}
576583

577584
// insert the preamble at the insertion point and the postscript copy command at the end
578585
replaced.patch(insertionPoint, preamble, 0) +

supportedBackends/aws/src/test/scala/cromwell/backend/impl/aws/AwsBatchJobSpec.scala

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,7 @@ class AwsBatchJobSpec extends TestKitSuite with AnyFlatSpecLike with Matchers wi
475475
val job = generateJobWithS3InOut
476476
val postscript =
477477
s"""
478+
|{
478479
|set -e
479480
|# (re-)add tags to include added volumes:
480481
|if [[ "false" == "true" ]]; then
@@ -484,12 +485,13 @@ class AwsBatchJobSpec extends TestKitSuite with AnyFlatSpecLike with Matchers wi
484485
|
485486
|echo '*** DELOCALIZING OUTPUTS ***'
486487
|DELOCALIZATION_FAILED=0
487-
|_s3_delocalize_with_retry "/tmp/scratch/baa" "s3://bucket/somewhere/baa" "false"
488488
|
489489
|if [ -f "/tmp/scratch/hello-rc.txt" ]; then _s3_delocalize_with_retry "/tmp/scratch/hello-rc.txt" "${job.jobPaths.returnCode}" ; fi
490490
|if [ -f "/tmp/scratch/hello-stderr.log" ]; then _s3_delocalize_with_retry "/tmp/scratch/hello-stderr.log" "${job.jobPaths.standardPaths.error}"; fi
491491
|if [ -f "/tmp/scratch/hello-stdout.log" ]; then _s3_delocalize_with_retry "/tmp/scratch/hello-stdout.log" "${job.jobPaths.standardPaths.output}"; fi
492492
|
493+
|_s3_delocalize_with_retry "/tmp/scratch/baa" "s3://bucket/somewhere/baa" "false"
494+
|
493495
|echo "DELOCALIZATION RESULT: $$DELOCALIZATION_FAILED"
494496
|if [[ $$DELOCALIZATION_FAILED -eq 1 ]]; then
495497
| echo '*** DELOCALIZATION FAILED ***'
@@ -504,7 +506,9 @@ class AwsBatchJobSpec extends TestKitSuite with AnyFlatSpecLike with Matchers wi
504506
|exit $$rc
505507
|}
506508
|""".stripMargin
507-
job.reconfiguredScript should include(postscript)
509+
510+
val normalize: String => String = s => s.replaceAll("\\s+", " ").trim
511+
normalize(job.reconfiguredScript) should include(normalize(postscript))
508512
}
509513

510514
it should "generate preamble with input copy command in reconfigured script" in {

0 commit comments

Comments
 (0)