-
Notifications
You must be signed in to change notification settings - Fork 123
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
Computeenvironment and jobqueue #1542
Computeenvironment and jobqueue #1542
Conversation
/test-examples="examples/batch/v1beta1/computeenvironment.yaml" |
3 similar comments
/test-examples="examples/batch/v1beta1/computeenvironment.yaml" |
/test-examples="examples/batch/v1beta1/computeenvironment.yaml" |
/test-examples="examples/batch/v1beta1/computeenvironment.yaml" |
The Uptest fails because of this error. Any idea on how to fix this? I can see that |
config/batch/config.go
Outdated
func Configure(p *config.Provider) { | ||
p.AddResourceConfigurator("aws_batch_compute_environment", func(r *config.Resource) { | ||
r.References = config.References{ | ||
"compute_resources.instance_role": config.Reference{ | ||
TerraformName: "aws_iam_instance_profile", | ||
}, | ||
"compute_resources.placement_group": config.Reference{ | ||
TerraformName: "aws_placement_group", | ||
}, | ||
"service_role": config.Reference{ | ||
TerraformName: "aws_iam_role", | ||
}, | ||
"compute_resources.subnets": config.Reference{ | ||
TerraformName: "aws_subnet", | ||
}, | ||
"compute_resources.security_group_ids": config.Reference{ | ||
TerraformName: "aws_security_group", | ||
}, | ||
} | ||
r.UseAsync = true | ||
}) | ||
|
||
p.AddResourceConfigurator("aws_batch_job_queue", func(r *config.Resource) { | ||
r.References = config.References{ | ||
"compute_environment_order.compute_environment": config.Reference{ | ||
TerraformName: "aws_batch_compute_environment", | ||
Extractor: common.PathARNExtractor, | ||
}, | ||
} | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested these two resources in my local, and except for the automatically defined references, defining the above seems sufficient. Could you please remove the above and add only the following configurations?
func Configure(p *config.Provider) {
p.AddResourceConfigurator("aws_batch_compute_environment", func(r *config.Resource) {
r.References["compute_resources.subnets"] = config.Reference{
TerraformName: "aws_subnet",
}
r.References["compute_resources.security_group_ids"] = config.Reference{
TerraformName: "aws_security_group",
}
})
}
examples/batch/v1beta1/jobqueue.yaml
Outdated
providerConfigRef: | ||
name: aws-provider-config-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use the default provider config in uptest; could you please remove the above?
examples/batch/v1beta1/jobqueue.yaml
Outdated
providerConfigRef: | ||
name: aws-provider-config-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use the default provider config in uptest; could you please remove the above?
Could you please also remove the batch group from externalnamenottested.go Thank you 🙏 |
Signed-off-by: sachinmalanki <[email protected]>
a42afae
to
e5a4733
Compare
/test-examples="examples/batch/v1beta1/computeenvironment.yaml" |
/test-examples="examples/iot/v1beta1/topicruledestination.yaml" |
Signed-off-by: sachinmalanki <[email protected]>
ce668b3
to
33d589a
Compare
/test-examples="examples/batch/v1beta1/computeenvironment.yaml" |
33d589a
to
e5a4733
Compare
/test-examples="examples/batch/v1beta1/computeenvironment.yaml" |
Signed-off-by: sachinmalanki <[email protected]>
/test-examples="examples/batch/v1beta1/computeenvironment.yaml" |
/test-examples="examples/batch/v1beta1/jobqueue.yaml" |
Signed-off-by: sachinmalanki <[email protected]>
/test-examples="examples/batch/v1beta1/computeenvironment.yaml" Uptest run: https://github.com/crossplane-contrib/provider-upjet-aws/actions/runs/11562496508 |
/test-examples="examples/batch/v1beta1/jobqueue.yaml" Uptest run: https://github.com/crossplane-contrib/provider-upjet-aws/actions/runs/11562654341 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for your effort and quick actions @sachinmalanki. The tests were finally passed successfully 🎉
Finally, a few cosmetic corrections would be nice before moving forward. I left a few comments for your consideration.
Signed-off-by: sachinmalanki <[email protected]>
Thanks @turkenf for your help. I have made the small changes suggested |
Description of your changes
Creating new AWS Batch Resources -
ComputeEnvironment
andJobQueue
Continuing from - #1489
I have:
make reviewable
to ensure this PR is ready for review.Addedbackport release-x.y
labels to auto-backport this PR if necessary.How has this code been tested
This code has been tested locally and with uptest in the below comments.