Skip to content

Commit 3134ed3

Browse files
lucymcnattlucymcnatt
committed
more tests + add support for missing location
undo accidental delete fix another fix formatting remove extra log
1 parent 2af8d35 commit 3134ed3

File tree

5 files changed

+192
-13
lines changed

5 files changed

+192
-13
lines changed

‎.vscode/settings.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"files.watcherExclude": {
3+
"**/target": true
4+
}
5+
}

‎runConfigurations/Repo template_ Cromwell server.run.xml‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<component name="ProjectRunConfigurationManager">
2+
<configuration default="false" name="Repo template: Cromwell server" type="Application" factoryName="Application">
3+
<option name="ALTERNATIVE_JRE_PATH" value="$USER_HOME$/.sdkman/candidates/java/current" />
4+
<envs>
5+
<env name="CROMWELL_BUILD_CENTAUR_SLICK_PROFILE" value="slick.jdbc.MySQLProfile$" />
6+
<env name="CROMWELL_BUILD_CENTAUR_JDBC_DRIVER" value="com.mysql.cj.jdbc.Driver" />
7+
<env name="CROMWELL_BUILD_CENTAUR_JDBC_URL" value="jdbc:mysql://localhost:3306/cromwell_test?allowPublicKeyRetrieval=true&amp;useSSL=false&amp;rewriteBatchedStatements=true&amp;serverTimezone=UTC&amp;useInformationSchema=true" />
8+
<env name="CROMWELL_BUILD_RESOURCES_DIRECTORY" value="target/ci/resources" />
9+
<env name="CROMWELL_BUILD_PAPI_JSON_FILE" value="target/ci/resources/cromwell-centaur-service-account.json" />
10+
<env name="CROMWELL_BUILD_CENTAUR_READ_LINES_LIMIT" value="128000" />
11+
<env name="CROMWELL_BUILD_CENTAUR_256_BITS_KEY" value="AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=" />
12+
<env name="GOOGLE_APPLICATION_CREDENTIALS" value="target/ci/resources/cromwell-centaur-service-account.json" />
13+
</envs>
14+
<option name="MAIN_CLASS_NAME" value="cromwell.CromwellApp" />
15+
<module name="root.cromwell" />
16+
<option name="PROGRAM_PARAMETERS" value="server" />
17+
<option name="VM_PARAMETERS" value="-Dconfig.file=target/ci/resources/gcp_batch_application.conf" />
18+
<extension name="net.ashald.envfile">
19+
<option name="IS_ENABLED" value="false" />
20+
<option name="IS_SUBST" value="false" />
21+
<option name="IS_PATH_MACRO_SUPPORTED" value="false" />
22+
<option name="IS_IGNORE_MISSING_FILES" value="false" />
23+
<option name="IS_ENABLE_EXPERIMENTAL_INTEGRATIONS" value="false" />
24+
<ENTRIES>
25+
<ENTRY IS_ENABLED="true" PARSER="runconfig" IS_EXECUTABLE="false" />
26+
</ENTRIES>
27+
</extension>
28+
<method v="2">
29+
<option name="Make" enabled="true" />
30+
</method>
31+
</configuration>
32+
</component>

‎supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/actors/GcpBatchAsyncBackendJobExecutionActor.scala‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,6 +1013,18 @@ class GcpBatchAsyncBackendJobExecutionActor(override val standardParams: Standar
10131013
} yield status
10141014
}
10151015

1016+
override val pollingResultMonitorActor: Option[ActorRef] = Option(
1017+
context.actorOf(
1018+
BatchPollResultMonitorActor.props(serviceRegistryActor,
1019+
workflowDescriptor,
1020+
jobDescriptor,
1021+
validatedRuntimeAttributes,
1022+
platform,
1023+
jobLogger
1024+
)
1025+
)
1026+
)
1027+
10161028
override def isTerminal(runStatus: RunStatus): Boolean =
10171029
runStatus match {
10181030
case _: RunStatus.TerminalRunStatus => true

‎supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/api/request/BatchRequestExecutor.scala‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import cromwell.backend.google.batch.api.{BatchApiRequestManager, BatchApiRespon
1313
import cromwell.backend.google.batch.models.{GcpBatchExitCode, RunStatus}
1414
import cromwell.core.ExecutionEvent
1515
import cromwell.services.cost.InstantiatedVmInfo
16+
import cromwell.services.metadata.CallMetadataKeys
1617

1718
import scala.annotation.unused
1819
import scala.concurrent.{ExecutionContext, Future, Promise}
@@ -146,9 +147,14 @@ object BatchRequestExecutor {
146147
val machineType = instancePolicy.getMachineType
147148
val preemtible = instancePolicy.getProvisioningModelValue == ProvisioningModel.PREEMPTIBLE.getNumber
148149

149-
// Each location can be a region or a zone. Only one region is supported, ex: "regions/us-central1"
150-
val location = allocationPolicy.getLocation.getAllowedLocations(0)
151-
val region = location.split("/").last
150+
// location list = [regions/us-central1, zones/us-central1-b], region is the first element
151+
val location = allocationPolicy.getLocation.getAllowedLocationsList.get(0)
152+
val region =
153+
if (location.isEmpty)
154+
"us-central1"
155+
else
156+
location.split("/").last
157+
152158
val instantiatedVmInfo = Some(InstantiatedVmInfo(region, machineType, preemtible))
153159

154160
if (job.getStatus.getState == JobStatus.State.SUCCEEDED) {
@@ -167,12 +173,20 @@ object BatchRequestExecutor {
167173
GcpBatchExitCode.fromEventMessage(e.name.toLowerCase)
168174
}.headOption
169175

170-
private def getEventList(events: List[StatusEvent]): List[ExecutionEvent] =
176+
private def getEventList(events: List[StatusEvent]): List[ExecutionEvent] = {
177+
val startedRegex = ".*SCHEDULED to RUNNING.*".r
178+
val endedRegex = ".*RUNNING to.*".r // can be SUCCEEDED or FAILED
171179
events.map { e =>
172180
val time = java.time.Instant
173181
.ofEpochSecond(e.getEventTime.getSeconds, e.getEventTime.getNanos.toLong)
174182
.atOffset(java.time.ZoneOffset.UTC)
175-
ExecutionEvent(name = e.getDescription, offsetDateTime = time)
183+
val eventType = e.getDescription match {
184+
case startedRegex() => CallMetadataKeys.VmStartTime
185+
case endedRegex() => CallMetadataKeys.VmEndTime
186+
case _ => e.getType
187+
}
188+
ExecutionEvent(name = eventType, offsetDateTime = time)
176189
}
190+
}
177191
}
178192
}

‎supportedBackends/google/batch/src/test/scala/cromwell/backend/google/batch/api/BatchRequestExecutorSpec.scala‎

Lines changed: 124 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,17 @@ import com.google.cloud.batch.v1.{
88
BatchServiceSettings,
99
GetJobRequest,
1010
Job,
11-
JobStatus
11+
JobStatus,
12+
StatusEvent
1213
}
1314
import com.google.cloud.batch.v1.AllocationPolicy.{
1415
InstancePolicy,
1516
InstancePolicyOrTemplate,
1617
LocationPolicy,
1718
ProvisioningModel
1819
}
20+
import com.google.cloud.batch.v1.JobStatus.State
21+
import com.google.protobuf.Timestamp
1922
import common.mock.MockSugar
2023
import cromwell.backend.google.batch.api.BatchApiResponse
2124
import cromwell.backend.google.batch.models.RunStatus
@@ -32,30 +35,57 @@ class BatchRequestExecutorSpec
3235
with MockSugar
3336
with PrivateMethodTester {
3437

35-
behavior of "BatchRequestExecutor"
36-
37-
it should "create instantiatedVmInfo correctly" in {
38-
38+
def setupBatchClient(machineType: String = "n1-standard-1",
39+
location: String = "regions/us-central1",
40+
jobState: State = JobStatus.State.SUCCEEDED
41+
): BatchServiceClient = {
3942
val instancePolicy = InstancePolicy
4043
.newBuilder()
41-
.setMachineType("n1-standard-1")
44+
.setMachineType(machineType)
4245
.setProvisioningModel(ProvisioningModel.PREEMPTIBLE)
4346
.build()
4447

4548
val allocationPolicy = AllocationPolicy
4649
.newBuilder()
47-
.setLocation(LocationPolicy.newBuilder().addAllowedLocations("regions/us-central1"))
50+
.setLocation(LocationPolicy.newBuilder().addAllowedLocations(location))
4851
.addInstances(InstancePolicyOrTemplate.newBuilder().setPolicy(instancePolicy))
4952
.build()
5053

51-
val jobStatus = JobStatus.newBuilder().setState(JobStatus.State.RUNNING).build()
54+
val startStatusEvent = StatusEvent
55+
.newBuilder()
56+
.setType("STATUS_CHANGED")
57+
.setEventTime(Timestamp.newBuilder().setSeconds(1).build())
58+
.setDescription("Job state is set from SCHEDULED to RUNNING for job...")
59+
.build()
60+
61+
val endStatusEvent = StatusEvent
62+
.newBuilder()
63+
.setType("STATUS_CHANGED")
64+
.setEventTime(Timestamp.newBuilder().setSeconds(2).build())
65+
.setDescription("Job state is set from RUNNING to SOME_OTHER_STATUS for job...")
66+
.build()
67+
68+
val jobStatus = JobStatus
69+
.newBuilder()
70+
.setState(jobState)
71+
.addStatusEvents(startStatusEvent)
72+
.addStatusEvents(endStatusEvent)
73+
.build()
5274

5375
val job = Job.newBuilder().setAllocationPolicy(allocationPolicy).setStatus(jobStatus).build()
5476

5577
val mockClient = mock[BatchServiceClient]
5678
doReturn(job).when(mockClient).getJob(any[GetJobRequest])
5779
doReturn(job).when(mockClient).getJob(any[String])
5880

81+
mockClient
82+
}
83+
84+
behavior of "BatchRequestExecutor"
85+
86+
it should "create instantiatedVmInfo correctly" in {
87+
88+
val mockClient = setupBatchClient(jobState = JobStatus.State.RUNNING)
5989
// Create the BatchRequestExecutor
6090
val batchRequestExecutor = new BatchRequestExecutor.CloudImpl(BatchServiceSettings.newBuilder().build())
6191

@@ -72,4 +102,90 @@ class BatchRequestExecutorSpec
72102
case _ => fail("Expected RunStatus.Running with instantiatedVmInfo")
73103
}
74104
}
105+
106+
it should "create instantiatedVmInfo correctly with different location info" in {
107+
108+
val mockClient = setupBatchClient(location = "zones/us-central1-a", jobState = JobStatus.State.RUNNING)
109+
110+
// Create the BatchRequestExecutor
111+
val batchRequestExecutor = new BatchRequestExecutor.CloudImpl(BatchServiceSettings.newBuilder().build())
112+
113+
// testing a private method see https://www.scalatest.org/user_guide/using_PrivateMethodTester
114+
val internalGetHandler = PrivateMethod[BatchApiResponse.StatusQueried](Symbol("internalGetHandler"))
115+
val result = batchRequestExecutor invokePrivate internalGetHandler(mockClient, GetJobRequest.newBuilder().build())
116+
117+
// Verify the instantiatedVmInfo
118+
result.status match {
119+
case RunStatus.Running(_, Some(instantiatedVmInfo)) =>
120+
instantiatedVmInfo.region shouldBe "us-central1-a"
121+
instantiatedVmInfo.machineType shouldBe "n1-standard-1"
122+
instantiatedVmInfo.preemptible shouldBe true
123+
case _ => fail("Expected RunStatus.Running with instantiatedVmInfo")
124+
}
125+
}
126+
127+
it should "create instantiatedVmInfo correctly with missing location info" in {
128+
129+
val mockClient = setupBatchClient(jobState = JobStatus.State.RUNNING)
130+
131+
// Create the BatchRequestExecutor
132+
val batchRequestExecutor = new BatchRequestExecutor.CloudImpl(BatchServiceSettings.newBuilder().build())
133+
134+
// testing a private method see https://www.scalatest.org/user_guide/using_PrivateMethodTester
135+
val internalGetHandler = PrivateMethod[BatchApiResponse.StatusQueried](Symbol("internalGetHandler"))
136+
val result = batchRequestExecutor invokePrivate internalGetHandler(mockClient, GetJobRequest.newBuilder().build())
137+
138+
// Verify the instantiatedVmInfo
139+
result.status match {
140+
case RunStatus.Running(_, Some(instantiatedVmInfo)) =>
141+
instantiatedVmInfo.region shouldBe "us-central1"
142+
instantiatedVmInfo.machineType shouldBe "n1-standard-1"
143+
instantiatedVmInfo.preemptible shouldBe true
144+
case _ => fail("Expected RunStatus.Running with instantiatedVmInfo")
145+
}
146+
}
147+
148+
it should "send vmStartTime and vmEndTime metadata info when a workflow succeeds" in {
149+
150+
val mockClient = setupBatchClient()
151+
152+
// Create the BatchRequestExecutor
153+
val batchRequestExecutor = new BatchRequestExecutor.CloudImpl(BatchServiceSettings.newBuilder().build())
154+
155+
// testing a private method see https://www.scalatest.org/user_guide/using_PrivateMethodTester
156+
val internalGetHandler = PrivateMethod[BatchApiResponse.StatusQueried](Symbol("internalGetHandler"))
157+
val result = batchRequestExecutor invokePrivate internalGetHandler(mockClient, GetJobRequest.newBuilder().build())
158+
159+
// Verify the events
160+
result.status match {
161+
case RunStatus.Success(events, _) =>
162+
val eventNames = events.map(_.name)
163+
val eventTimes = events.map(_.offsetDateTime.toString)
164+
eventNames should contain theSameElementsAs List("vmStartTime", "vmEndTime")
165+
eventTimes should contain theSameElementsAs List("1970-01-01T00:00:01Z", "1970-01-01T00:00:02Z")
166+
case _ => fail("Expected RunStatus.Success with events")
167+
}
168+
}
169+
170+
it should "send vmStartTime and vmEndTime metadata info when a workflow fails" in {
171+
val mockClient = setupBatchClient(jobState = JobStatus.State.FAILED)
172+
173+
// Create the BatchRequestExecutor
174+
val batchRequestExecutor = new BatchRequestExecutor.CloudImpl(BatchServiceSettings.newBuilder().build())
175+
176+
// testing a private method see https://www.scalatest.org/user_guide/using_PrivateMethodTester
177+
val internalGetHandler = PrivateMethod[BatchApiResponse.StatusQueried](Symbol("internalGetHandler"))
178+
val result = batchRequestExecutor invokePrivate internalGetHandler(mockClient, GetJobRequest.newBuilder().build())
179+
180+
// Verify the events
181+
result.status match {
182+
case RunStatus.Failed(_, events, _) =>
183+
val eventNames = events.map(_.name)
184+
val eventTimes = events.map(_.offsetDateTime.toString)
185+
eventNames should contain theSameElementsAs List("vmStartTime", "vmEndTime")
186+
eventTimes should contain theSameElementsAs List("1970-01-01T00:00:01Z", "1970-01-01T00:00:02Z")
187+
case _ => fail("Expected RunStatus.Success with events")
188+
}
189+
}
190+
75191
}

0 commit comments

Comments
 (0)