Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix aws unit tests #39

Merged
merged 3 commits into from
Oct 30, 2023
Merged

Fix aws unit tests #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
418b314
checkpoint
Oct 27, 2023
187e55c
fix ecr and batch tests
Oct 27, 2023
f81a0ab
fix AwsBatchJobSpec.scala
Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions dockerHashing/src/test/scala/cromwell/docker/registryv2/flows/aws/AmazonEcrPublicSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
package cromwell.docker.registryv2.flows.aws

import cats.effect.{IO, Resource}
import cromwell.core.TestKitSuite
import cromwell.docker.registryv2.DockerRegistryV2Abstract
Expand All @@ -12,20 +11,19 @@ import org.mockito.Mockito._
import org.scalatest.{BeforeAndAfter, PrivateMethodTester}
import org.scalatest.flatspec.AnyFlatSpecLike
import org.scalatest.matchers.should.Matchers
import org.scalatestplus.mockito.MockitoSugar
import software.amazon.awssdk.services.ecrpublic.model.{AuthorizationData, GetAuthorizationTokenRequest, GetAuthorizationTokenResponse}
import software.amazon.awssdk.services.ecrpublic.EcrPublicClient

class AmazonEcrPublicSpec extends TestKitSuite with AnyFlatSpecLike with Matchers with MockitoSugar with BeforeAndAfter with PrivateMethodTester {
class AmazonEcrPublicSpec extends TestKitSuite with AnyFlatSpecLike with Matchers with BeforeAndAfter with PrivateMethodTester {
behavior of "AmazonEcrPublic"

val goodUri = "public.ecr.aws/amazonlinux/amazonlinux:latest"
val otherUri = "ubuntu:latest"


val mediaType: MediaType = MediaType.parse(DockerRegistryV2Abstract.ManifestV2MediaType).right.get
val mediaType: MediaType = MediaType.parse(DockerRegistryV2Abstract.DockerManifestV2MediaType).getOrElse(fail("Cant parse mediatype"))
val contentType: Header = `Content-Type`(mediaType)
val mockEcrClient: EcrPublicClient = mock[EcrPublicClient]
val mockEcrClient: EcrPublicClient = mock(classOf[EcrPublicClient])
implicit val mockIOClient: Client[IO] = Client({ _: Request[IO] =>
// This response will have an empty body, so we need to be explicit about the typing:
Resource.pure[IO, Response[IO]](Response(headers = Headers.of(contentType))) : Resource[IO, Response[IO]]
Expand All @@ -44,7 +42,7 @@ class AmazonEcrPublicSpec extends TestKitSuite with AnyFlatSpecLike with Matcher
}

it should "have public.ecr.aws as registryHostName" in {
val registryHostNameMethod = PrivateMethod[String]('registryHostName)
val registryHostNameMethod = PrivateMethod[String](Symbol("registryHostName"))
registry invokePrivate registryHostNameMethod(DockerImageIdentifier.fromString(goodUri).get) shouldEqual "public.ecr.aws"
}

Expand All @@ -63,7 +61,7 @@ class AmazonEcrPublicSpec extends TestKitSuite with AnyFlatSpecLike with Matcher
.build())
.build)

val getTokenMethod = PrivateMethod[IO[Option[String]]]('getToken)
val getTokenMethod = PrivateMethod[IO[Option[String]]](Symbol("getToken"))
registry invokePrivate getTokenMethod(context, mockIOClient) ensuring(io => io.unsafeRunSync().get == token)
}
}
13 changes: 6 additions & 7 deletions dockerHashing/src/test/scala/cromwell/docker/registryv2/flows/aws/AmazonEcrSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,18 @@ import org.mockito.Mockito._
import org.scalatest.{BeforeAndAfter, PrivateMethodTester}
import org.scalatest.flatspec.AnyFlatSpecLike
import org.scalatest.matchers.should.Matchers
import org.scalatestplus.mockito.MockitoSugar
import software.amazon.awssdk.services.ecr.EcrClient
import software.amazon.awssdk.services.ecr.model.{AuthorizationData, GetAuthorizationTokenResponse}

class AmazonEcrSpec extends TestKitSuite with AnyFlatSpecLike with Matchers with MockitoSugar with BeforeAndAfter with PrivateMethodTester{
class AmazonEcrSpec extends TestKitSuite with AnyFlatSpecLike with Matchers with BeforeAndAfter with PrivateMethodTester{
behavior of "AmazonEcr"

val goodUri = "123456789012.dkr.ecr.us-east-1.amazonaws.com/amazonlinux/amazonlinux:latest"
val otherUri = "ubuntu:latest"

val mediaType: MediaType = MediaType.parse(DockerRegistryV2Abstract.ManifestV2MediaType).right.get
val mediaType: MediaType = MediaType.parse(DockerRegistryV2Abstract.DockerManifestV2MediaType).getOrElse(fail("Can't parse media type"))
val contentType: Header = `Content-Type`(mediaType)
val mockEcrClient: EcrClient = mock[EcrClient]
val mockEcrClient: EcrClient = mock(classOf[EcrClient])
implicit val mockIOClient: Client[IO] = Client({ _: Request[IO] =>
// This response will have an empty body, so we need to be explicit about the typing:
Resource.pure[IO, Response[IO]](Response(headers = Headers.of(contentType))) : Resource[IO, Response[IO]]
Expand All @@ -42,12 +41,12 @@ class AmazonEcrSpec extends TestKitSuite with AnyFlatSpecLike with Matchers with
}

it should "use Basic Auth Scheme" in {
val authSchemeMethod = PrivateMethod[AuthScheme]('authorizationScheme)
val authSchemeMethod = PrivateMethod[AuthScheme](Symbol("authorizationScheme"))
registry invokePrivate authSchemeMethod() shouldEqual AuthScheme.Basic
}

it should "return 123456789012.dkr.ecr.us-east-1.amazonaws.com as registryHostName" in {
val registryHostNameMethod = PrivateMethod[String]('registryHostName)
val registryHostNameMethod = PrivateMethod[String](Symbol("registryHostName"))
registry invokePrivate registryHostNameMethod(DockerImageIdentifier.fromString(goodUri).get) shouldEqual "123456789012.dkr.ecr.us-east-1.amazonaws.com"
}

Expand All @@ -66,7 +65,7 @@ class AmazonEcrSpec extends TestKitSuite with AnyFlatSpecLike with Matchers with
.build())
.build)

val getTokenMethod = PrivateMethod[IO[Option[String]]]('getToken)
val getTokenMethod = PrivateMethod[IO[Option[String]]](Symbol("getToken"))
registry invokePrivate getTokenMethod(context, mockIOClient) ensuring(io => io.unsafeRunSync().get == token)
}
}
65 changes: 31 additions & 34 deletions supportedBackends/aws/src/main/scala/cromwell/backend/impl/aws/AwsBatchJob.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,15 @@ final case class AwsBatchJob(jobDescriptor: BackendJobDescriptor, // WDL/CWL
*/
lazy val reconfiguredScript: String = {
//this is the location of the aws cli mounted into the container by the ec2 launch template
val awsCmd = "/usr/local/aws-cli/v2/current/bin/aws "
val awsCmd = "/usr/local/aws-cli/v2/current/bin/aws"
//internal to the container, therefore not mounted
val workDir = "/tmp/scratch"
//working in a mount will cause collisions in long running workers
val replaced = commandScript.replace(AwsBatchWorkingDisk.MountPoint.pathAsString, workDir)
val insertionPoint = replaced.indexOf("\n", replaced.indexOf("#!")) +1 //just after the new line after the shebang!
// load the config
val conf : Config = ConfigFactory.load();

/* generate a series of s3 copy statements to copy any s3 files into the container. */
val inputCopyCommand = inputs.map {
case input: AwsBatchFileInput if input.s3key.startsWith("s3://") && input.s3key.endsWith(".tmp") =>
Expand All @@ -135,14 +135,11 @@ final case class AwsBatchJob(jobDescriptor: BackendJobDescriptor, // WDL/CWL
|sed -i 's#${AwsBatchWorkingDisk.MountPoint.pathAsString}#$workDir#g' $workDir/${input.local}
|""".stripMargin


case input: AwsBatchFileInput if input.s3key.startsWith("s3://") =>
// regular s3 objects : download to working dir.
s"_s3_localize_with_retry ${input.s3key} ${input.mount.mountPoint.pathAsString}/${input.local}"
.replace(AwsBatchWorkingDisk.MountPoint.pathAsString, workDir)



case input: AwsBatchFileInput if efsMntPoint.isDefined && input.s3key.startsWith(efsMntPoint.get) =>
// EFS located file : test for presence on provided path.
Log.debug("EFS input file detected: "+ input.s3key + " / "+ input.local.pathAsString)
Expand Down Expand Up @@ -187,11 +184,11 @@ final case class AwsBatchJob(jobDescriptor: BackendJobDescriptor, // WDL/CWL
| echo "$$s3_path is not an S3 path with a bucket and key. aborting"
| exit 1
| fi
| # copy
| # copy
| $awsCmd s3 cp --no-progress "$$s3_path" "$$destination" ||
| { echo "attempt $$i to copy $$s3_path failed" && sleep $$((7 * "$$i")) && continue; }
| # check data integrity
| _check_data_integrity $$destination $$s3_path ||
| _check_data_integrity $$destination $$s3_path ||
| { echo "data content length difference detected in attempt $$i to copy $$local_path failed" && sleep $$((7 * "$$i")) && continue; }
| # copy succeeded
| break
Expand All @@ -207,7 +204,7 @@ final case class AwsBatchJob(jobDescriptor: BackendJobDescriptor, // WDL/CWL
| # get the multipart chunk size
| chunk_size=$$(_get_multipart_chunk_size $$local_path)
| local MP_THRESHOLD=${mp_threshold}
| # then set them
| # then set them
| $awsCmd configure set default.s3.multipart_threshold $$MP_THRESHOLD
| $awsCmd configure set default.s3.multipart_chunksize $$chunk_size
|
Expand All @@ -220,27 +217,27 @@ final case class AwsBatchJob(jobDescriptor: BackendJobDescriptor, // WDL/CWL
| exit 2
| fi
| # if destination is not a bucket : abort
| if ! [[ $$destination =~ s3://([^/]+)/(.+) ]]; then
| if ! [[ $$destination =~ s3://([^/]+)/(.+) ]]; then
| echo "$$destination is not an S3 path with a bucket and key. aborting"
| exit 1
| fi
| # copy ok or try again.
| if [[ -d "$$local_path" ]]; then
| # make sure to strip the trailing / in destination
| # make sure to strip the trailing / in destination
| destination=$${destination%/}
| # glob directory. do recursive copy
| $awsCmd s3 cp --no-progress $$local_path $$destination --recursive --exclude "cromwell_glob_control_file" ||
| { echo "attempt $$i to copy globDir $$local_path failed" && sleep $$((7 * "$$i")) && continue; }
| $awsCmd s3 cp --no-progress $$local_path $$destination --recursive --exclude "cromwell_glob_control_file" ||
| { echo "attempt $$i to copy globDir $$local_path failed" && sleep $$((7 * "$$i")) && continue; }
| # check integrity for each of the files
| for FILE in $$(cd $$local_path ; ls | grep -v cromwell_glob_control_file); do
| _check_data_integrity $$local_path/$$FILE $$destination/$$FILE ||
| _check_data_integrity $$local_path/$$FILE $$destination/$$FILE ||
| { echo "data content length difference detected in attempt $$i to copy $$local_path/$$FILE failed" && sleep $$((7 * "$$i")) && continue 2; }
| done
| else
| $awsCmd s3 cp --no-progress "$$local_path" "$$destination" ||
| { echo "attempt $$i to copy $$local_path failed" && sleep $$((7 * "$$i")) && continue; }
| else
| $awsCmd s3 cp --no-progress "$$local_path" "$$destination" ||
| { echo "attempt $$i to copy $$local_path failed" && sleep $$((7 * "$$i")) && continue; }
| # check content length for data integrity
| _check_data_integrity $$local_path $$destination ||
| _check_data_integrity $$local_path $$destination ||
| { echo "data content length difference detected in attempt $$i to copy $$local_path failed" && sleep $$((7 * "$$i")) && continue; }
| fi
| # copy succeeded
Expand All @@ -251,9 +248,9 @@ final case class AwsBatchJob(jobDescriptor: BackendJobDescriptor, // WDL/CWL
|function _get_multipart_chunk_size() {
| local file_path=$$1
| # file size
| file_size=$$(stat --printf="%s" $$file_path)
| file_size=$$(stat --printf="%s" $$file_path)
| # chunk_size : you can have at most 10K parts with at least one 5MB part
| # this reflects the formula in s3-copy commands of cromwell (S3FileSystemProvider.java)
| # this reflects the formula in s3-copy commands of cromwell (S3FileSystemProvider.java)
| # => long partSize = Math.max((objectSize / 10000L) + 1, 5 * 1024 * 1024);
| a=$$(( ( file_size / 10000) + 1 ))
| b=$$(( 5 * 1024 * 1024 ))
Expand All @@ -264,26 +261,26 @@ final case class AwsBatchJob(jobDescriptor: BackendJobDescriptor, // WDL/CWL
|function _check_data_integrity() {
| local local_path=$$1
| local s3_path=$$2
|
|
| # remote : use content_length
| if [[ $$s3_path =~ s3://([^/]+)/(.+) ]]; then
| if [[ $$s3_path =~ s3://([^/]+)/(.+) ]]; then
| bucket="$${BASH_REMATCH[1]}"
| key="$${BASH_REMATCH[2]}"
| else
| # this is already checked in the caller function
| echo "$$s3_path is not an S3 path with a bucket and key. aborting"
| exit 1
| fi
| s3_content_length=$$($awsCmd s3api head-object --bucket "$$bucket" --key "$$key" --query 'ContentLength') ||
| { echo "Attempt to get head of object failed for $$s3_path." && return 1 ; }
| s3_content_length=$$($awsCmd s3api head-object --bucket "$$bucket" --key "$$key" --query 'ContentLength') ||
| { echo "Attempt to get head of object failed for $$s3_path." && return 1; }
| # local
| local_content_length=$$(LC_ALL=C ls -dnL -- "$$local_path" | awk '{print $$5; exit}' ) ||
| { echo "Attempt to get local content length failed for $$_local_path." && return 1; }
| local_content_length=$$(LC_ALL=C ls -dnL -- "$$local_path" | awk '{print $$5; exit}' ) ||
| { echo "Attempt to get local content length failed for $$_local_path." && return 1; }
| # compare
| if [[ "$$s3_content_length" -eq "$$local_content_length" ]]; then
| true
| else
| false
| false
| fi
|}
|
Expand Down Expand Up @@ -375,8 +372,8 @@ final case class AwsBatchJob(jobDescriptor: BackendJobDescriptor, // WDL/CWL
s"""_s3_delocalize_with_retry $workDir/${output.local.pathAsString} ${output.s3key}""".stripMargin

// file(name (full path), s3key (delocalized path), local (file basename), mount (disk details))
// files on EFS mounts are optionally delocalized.
case output: AwsBatchFileOutput if efsMntPoint.isDefined && output.mount.mountPoint.pathAsString == efsMntPoint.get =>
// files on EFS mounts are optionally delocalized.
case output: AwsBatchFileOutput if efsMntPoint.isDefined && output.mount.mountPoint.pathAsString == efsMntPoint.get =>
Log.debug("EFS output file detected: "+ output.s3key + s" / ${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}")
// EFS located file : test existence or delocalize.
var test_cmd = ""
Expand All @@ -388,25 +385,25 @@ final case class AwsBatchJob(jobDescriptor: BackendJobDescriptor, // WDL/CWL
// check file for existence
test_cmd = s"test -e ${output.mount.mountPoint.pathAsString}/${output.local.pathAsString} || (echo 'output file: ${output.mount.mountPoint.pathAsString}/${output.local.pathAsString} does not exist' && exit 1)"
}
// need to make md5sum?
// need to make md5sum?
var md5_cmd = ""
if (efsMakeMD5.isDefined && efsMakeMD5.getOrElse(false)) {
Log.debug("Add cmd to create MD5 sibling.")
md5_cmd = s"""
|if [[ ! -f '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}.md5' ]] ; then
| 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' && exit 1 );
|if [[ ! -f '${output.mount.mountPoint.pathAsString}/${output.local.pathAsString}.md5' ]] ; then
| 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' && exit 1 );
|fi
|""".stripMargin
|""".stripMargin
} else {
Log.debug("MD5 not enabled: "+efsMakeMD5.get.toString())
md5_cmd = ""
}
}
// return combined result
s"""
|${test_cmd}
|${md5_cmd}
| """.stripMargin

case output: AwsBatchFileOutput =>
//output on a different mount
Log.debug("output data on other mount")
Expand Down
3 changes: 1 addition & 2 deletions .../src/test/scala/cromwell/backend/impl/aws/AwsBatchAsyncBackendJobExecutionActorSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
package cromwell.backend.impl.aws

import java.util.UUID

import akka.actor.{ActorRef, Props}
import akka.testkit.{ImplicitSender, TestActorRef, TestDuration}
import common.collections.EnhancedCollections._
Expand All @@ -41,7 +40,7 @@ import cromwell.backend._
import cromwell.backend.async.{ExecutionHandle, FailedNonRetryableExecutionHandle, PendingExecutionHandle}
import cromwell.backend.impl.aws.AwsBatchAsyncBackendJobExecutionActor.AwsBatchPendingExecutionHandle
import cromwell.backend.impl.aws.RunStatus.UnsuccessfulRunStatus
import cromwell.backend.impl.aws.io.AwsBatchWorkingDisk
import cromwell.backend.impl.aws.io.{AwsBatchWorkflowPaths, AwsBatchWorkingDisk}
import cromwell.backend.io.JobPathsSpecHelper._
import cromwell.backend.standard.{DefaultStandardAsyncExecutionActorParams, StandardAsyncExecutionActorParams, StandardAsyncJob, StandardExpressionFunctions, StandardExpressionFunctionsParams}
import cromwell.cloudsupport.aws.s3.S3Storage
Expand Down
3 changes: 2 additions & 1 deletion supportedBackends/aws/src/test/scala/cromwell/backend/impl/aws/AwsBatchCallPathsSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ package cromwell.backend.impl.aws

import common.collections.EnhancedCollections._
import cromwell.backend.BackendSpec
import cromwell.backend.io.JobPathsSpecHelper._
import cromwell.backend.impl.aws.io.{AwsBatchJobPaths, AwsBatchWorkflowPaths}
import cromwell.backend.io.JobPathsSpecHelper.{EnhancedJobPaths, EnhancedCallContext}
import org.scalatest.flatspec.AnyFlatSpecLike
import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider
import cromwell.core.Tags.AwsTest
Expand Down
Loading
Loading