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.

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

WX-927 GCP Batch labels tests #7523

Merged
merged 5 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: gcpbatch_google_labels_bad
testFormat: workflowfailure
backends: [GCPBATCH]

files {
workflow: google_labels/google_labels.wdl
options: google_labels/gcpbatch_bad_options.json
}

metadata {
workflowName: google_labels
status: Failed

"failures.0.message": "Invalid 'google_labels' in workflow options"
"failures.0.causedBy.0.message": "Invalid label key field: `custom-label-label-label-label-label-label-label-label-label-label` is 66 characters. The maximum is 63."
"failures.0.causedBy.1.message": "Invalid label value field: `Custom-Value!!!` did not match the regex '[a-z0-9]([-a-z_0-9]*[a-z0-9])?'"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: gcpbatch_google_labels_good
testFormat: workflowsuccess
backends: [GCPBATCH]

files {
workflow: google_labels/gcpbatch_google_labels.wdl
options: google_labels/good_options.json
}

metadata {
workflowName: google_labels
status: Succeeded

"calls.google_labels.check_labels.labels.wdl-task-name": "check_labels"

"calls.google_labels.check_labels.backendLabels.custom-label": "custom-value"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: gcpbatch_google_labels_sub
testFormat: workflowsuccess
backends: [GCPBATCH]

files {
workflow: google_labels/gcpbatch_wrapper.wdl
options: google_labels/good_options.json
imports: [
google_labels/google_labels.wdl
]
}

metadata {
workflowName: google_labels_wrapper
status: Succeeded
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: gcpbatch_jes_labels
testFormat: workflowsuccess
tags: [ labels ]
backends: [GCPBATCH]

files {
workflow: labels/jes_labels.wdl
labels: labels/valid.labels
}

metadata {
workflowName: I_hope_nobody_names_workflows_like_this_as_it_seems_very_unnecessary_
status: Succeeded
"calls.I_hope_nobody_names_workflows_like_this_as_it_seems_very_unnecessary_.my_task_aliased.backendLabels.cromwell-workflow-id": "cromwell-<<UUID>>"
"calls.I_hope_nobody_names_workflows_like_this_as_it_seems_very_unnecessary_.my_task_aliased.backendLabels.wdl-task-name": "my_task"
"calls.I_hope_nobody_names_workflows_like_this_as_it_seems_very_unnecessary_.my_task_aliased.backendLabels.wdl-call-alias": "my_task_aliased"
"calls.I_hope_nobody_names_workflows_like_this_as_it_seems_very_unnecessary_.my_task_aliased.labels.wdl-task-name": "MY_TASK"
"calls.I_hope_nobody_names_workflows_like_this_as_it_seems_very_unnecessary_.my_task_aliased.labels.wdl-call-alias": "my_task_aliased"
"labels.cromwell-workflow-id": "cromwell-<<UUID>>"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"google_labels": {
"custom-label-label-label-label-label-label-label-label-label-label": "Custom-Value!!!"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
version 1.0

workflow google_labels {

meta {
description: "Confirms that the google_labels workflow option is propagated correctly to tasks"
}

input {
Array[Pair[String, String]] expected_kvps = [ ("wdl-task-name", "check_labels") ]
Boolean check_aliases = true
}

# Build an array of checker lines to confirm the existence of each KVP:
scatter (p in expected_kvps) {
String checker_lines = "echo $INSTANCE_INFO | grep -q '\"~{p.left}\": \"~{p.right}\"' || echo \"Couldn't find correct '~{p.left}' label in instance info: $INSTANCE_INFO\" 1>&2"
}

call check_labels { input: label_checker_lines = checker_lines }

if (check_aliases) {
call check_labels as check_label_alias { input: label_checker_lines = [
"echo $INSTANCE_INFO | grep -q '\"wdl-task-name\": \"check_labels\"' || echo \"Couldn't find correct 'wdl-task-name' label in instance info: $INSTANCE_INFO\" 1>&2",
"echo $INSTANCE_INFO | grep -q '\"wdl-call-alias\": \"check_label_alias\"' || echo \"Couldn't find correct 'wdl-call-alias' label in instance info: $INSTANCE_INFO\" 1>&2"
] }
}

output {
Boolean done = true
}

}

task check_labels {

input {
Array[String] label_checker_lines
}

command {
# Check that the task metadata is correct:
INSTANCE=$(curl -s "http://metadata.google.internal/computeMetadata/v1/instance/name" -H "Metadata-Flavor: Google")
TOKEN=$(gcloud auth application-default print-access-token)
INSTANCE_INFO=$(curl -s "https://www.googleapis.com/compute/v1/projects/broad-dsde-cromwell-dev/zones/us-central1-c/instances/$INSTANCE" \
-H "Authorization: Bearer $TOKEN" \
-H 'Accept: application/json')
Comment on lines +44 to +46
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this from within WDL is super cool 💯


# Check for the custom label in the instance info:
~{sep="\n" label_checker_lines}
}
runtime {
docker: "gcr.io/google.com/cloudsdktool/cloud-sdk:slim"
# Specify this zone because it's used in the curl commands above. We probably *could* work this out ad-hoc but it's easier to hard-code it here:
zones: ["us-central1-c"]
failOnStderr: true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
version 1.0

import "google_labels.wdl"

workflow google_labels_wrapper {
call google_labels.google_labels { input:
expected_kvps = [ ("cromwell-sub-workflow-name", "google_labels"), ("wdl-task-name", "check_labels") ],
check_aliases = false
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
name: google_labels_bad
testFormat: workflowfailure
# possible error message discrepancy
backends: [Papiv2, GCPBATCH_FAIL]
backends: [Papiv2, GCPBATCH_ALT]

files {
workflow: google_labels/google_labels.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
name: google_labels_good
testFormat: workflowsuccess
# GCPBATCH is unexpectedly converting dashes to underscores, see doc 2.13
backends: [Papiv2, GCPBATCH_FAIL]
backends: [Papiv2, GCPBATCH_ALT]

files {
workflow: google_labels/google_labels.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
name: google_labels_sub
testFormat: workflowsuccess
# see doc 2.13
backends: [Papiv2, GCPBATCH_FAIL]
backends: [Papiv2, GCPBATCH_ALT]

files {
workflow: google_labels/wrapper.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
name: jes_labels
testFormat: workflowsuccess
tags: [ labels ]
# slashes being replaced with underscores?
backends: [Papi, GCPBATCH_FAIL]
backends: [Papi, GCPBATCH_ALT]

files {
workflow: labels/jes_labels.wdl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@

// This function is used to coerce a string into one that meets the requirements for a label submission to Google Batch API.
// https://cloud.google.com/compute/docs/labeling-resources#requirements
def safeGoogleName(mainText: String, emptyAllowed: Boolean = false): String =
validateLabelRegex(mainText, true) match {
def safeGoogleName(mainText: String, isKey: Boolean, emptyAllowed: Boolean = false): String =
validateLabelRegex(mainText, isKey) match {

Check warning on line 29 in supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/models/GcpLabel.scala

View check run for this annotation

Codecov / codecov/patch

supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/models/GcpLabel.scala#L29

Added line #L29 was not covered by tests
case Valid(labelText) => labelText
case invalid @ _ if mainText.equals("") && emptyAllowed => mainText
case invalid @ _ =>
def appendSafe(current: String, nextChar: Char): String =
nextChar match {
case c if c.isLetterOrDigit || c == '-' => current + c.toLower
case c if c.isLetterOrDigit || c == '-' || c == '_' => current + c.toLower

Check warning on line 35 in supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/models/GcpLabel.scala

View check run for this annotation

Codecov / codecov/patch

supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/models/GcpLabel.scala#L35

Added line #L35 was not covered by tests
case _ => current + '-'
}

Expand Down Expand Up @@ -75,7 +75,7 @@

def safeLabels(values: (String, String)*): Seq[GcpLabel] = {
def safeGoogleLabel(kvp: (String, String)): GcpLabel =
GcpLabel(safeGoogleName(kvp._1), safeGoogleName(kvp._2, emptyAllowed = true))
GcpLabel(safeGoogleName(kvp._1, isKey = true), safeGoogleName(kvp._2, isKey = false, emptyAllowed = true))

Check warning on line 78 in supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/models/GcpLabel.scala

View check run for this annotation

Codecov / codecov/patch

supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/models/GcpLabel.scala#L78

Added line #L78 was not covered by tests
values.map(safeGoogleLabel)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class GcpBatchLabelSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matche
}

it should s"convert the bad label string '$label' into the safe label string '$conversion'" in {
GcpLabel.safeGoogleName(label) should be(conversion)
GcpLabel.safeGoogleName(label, isKey = true) should be(conversion)
}
}
}
Loading