Adding aws_batch_job_definition to v1beta1 version #857
Conversation
huffmanjohnf
requested review from
ulucinar,
sergenyalcin and
turkenf
as code owners
|
|
|
/test-examples="examples/batch/jobdefinition.yaml" |
2 similar comments
|
/test-examples="examples/batch/jobdefinition.yaml" |
|
/test-examples="examples/batch/jobdefinition.yaml" |
There was a problem hiding this comment.
Thank you for this PR @huffmanjohnf, I left a comment to consider.
| @@ -1546,6 +1546,9 @@ var ExternalNameConfigs = map[string]config.ExternalName{ | |||
| // | |||
| // Batch Scheduling Policy can be imported using the arn: arn:aws:batch:us-east-1:123456789012:scheduling-policy/sample | |||
| "aws_batch_scheduling_policy": config.TemplatedStringAsIdentifier("name", "arn:aws:batch:{{ .setup.configuration.region }}:{{ .setup.client_metadata.account_id }}:scheduling-policy/{{ .external_name }}"), | |||
| // Batch Job Definition can be imported using ARN that has a random substring, revision at the end: | |||
| // arn:aws:batch:us-east-1:123456789012:job-definition/sample:1 | |||
| "aws_batch_job_definition": config.IdentifierFromProvider, | |||
There was a problem hiding this comment.
| "aws_batch_job_definition": config.IdentifierFromProvider, | |
| "aws_batch_job_definition": config.TemplatedStringAsIdentifier("name", "arn:aws:batch:{{ .setup.configuration.region }}:{{ .setup.client_metadata.account_id }}:job-definition/{{ .external_name }}"), |
If you haven't tried it, can you please try the configuration above?
There was a problem hiding this comment.
Thank you for the suggestion @turkenf. That was the first configuration I attempted, which ended up reproducing the behavior described in #162.
In summary, the configuration you suggested would successfully create the external resource but then the controller could not observe it afterwards due to:
Message: observe failed: cannot run refresh: refresh failed: "name" ("test:1") must be up to 128 letters (uppercase and lowercase), numbers, underscores and dashes, and must start with an alphanumeric.:
In other words, the external name part of the ARN for a job-definition has a name (defined in config, i.e. test) and a version (provided by AWS, i.e. :1).
If I understood the Contributing docs correctly, the version suffix provided by AWS breaks the pattern for Case 5: Non-random Substrings as Identifier used for most ARNs and instead falls into the random substring case:
However, there are cases where the ARN includes random substring and that would fall under Case 4.
I ended up choosing config.IdentifierFromProvider based on other resources with ARNs following the name:version pattern, such as an aws_ecs_task_definition external name. This worked locally, as I could create and observe the external resource from a local Crossplane cluster (created by make local-deploy) against my own AWS account.
I've never worked with golang before and mostly relied on the docs and Makefile to create this PR, so I'm certainly open for guidance on the best way to configure this. I am not sure how to troubleshoot the automated test logs or the error (uptest-v0.5.0: error: cannot run e2e tests successfully: cannot execute tests: kuttl failed: exit status 1) and can't seem to get make e2e run on my machine. Any help is much appreciated!
|
/test-examples="examples/batch/jobdefinition.yaml" |
|
@huffmanjohnf, uptest failed because of: |
|
Awesome, thank you for sharing the exception! That points me in the right direction. I'll update the Where did you find that error trace? I didn't see it in the action logs. |
|
Ahh, I see. Thank you! |
|
/test-examples="examples/batch/jobdefinition.yaml" |
1 similar comment
|
/test-examples="examples/batch/jobdefinition.yaml" |
Description of your changes
Adding (1) resources in the batch group:
Fixes #162
Related to #200
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
Applied in personal AWS account using local deployment created by
make local-deploy.