-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
fix resource/aws_ecs_service: make task_definition truly optional #27390
fix resource/aws_ecs_service: make task_definition truly optional #27390
Conversation
Community NoteVoting for Prioritization
For Submitters
|
6ebec89
to
48170ca
Compare
Thanks sgirones. You could also verify Cloud Trail log and query for "CreateService" events. If requestParameters portion doesn't include "taskDefinition" we should be good I think. |
Sure thing. I actually tested the fix in my environment, where I ran into this bug. Here is the CloudTrail event for the before and after. Before the fix
After the fix
|
Relates #19557. |
This has been failing in CI:
|
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.
LGTM 🚀.
% make testacc TESTARGS='-run=TestAccECSService_' PKG=ecs ACCTEST_PARALLELISM=3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/ecs/... -v -count 1 -parallel 3 -run=TestAccECSService_ -timeout 180m
=== RUN TestAccECSService_basic
=== PAUSE TestAccECSService_basic
=== RUN TestAccECSService_basicImport
=== PAUSE TestAccECSService_basicImport
=== RUN TestAccECSService_disappears
=== PAUSE TestAccECSService_disappears
=== RUN TestAccECSService_PlacementStrategy_unnormalized
=== PAUSE TestAccECSService_PlacementStrategy_unnormalized
=== RUN TestAccECSService_CapacityProviderStrategy_basic
=== PAUSE TestAccECSService_CapacityProviderStrategy_basic
=== RUN TestAccECSService_CapacityProviderStrategy_forceNewDeployment
=== PAUSE TestAccECSService_CapacityProviderStrategy_forceNewDeployment
=== RUN TestAccECSService_CapacityProviderStrategy_update
=== PAUSE TestAccECSService_CapacityProviderStrategy_update
=== RUN TestAccECSService_CapacityProviderStrategy_multiple
=== PAUSE TestAccECSService_CapacityProviderStrategy_multiple
=== RUN TestAccECSService_familyAndRevision
=== PAUSE TestAccECSService_familyAndRevision
=== RUN TestAccECSService_renamedCluster
=== PAUSE TestAccECSService_renamedCluster
=== RUN TestAccECSService_healthCheckGracePeriodSeconds
=== PAUSE TestAccECSService_healthCheckGracePeriodSeconds
=== RUN TestAccECSService_iamRole
=== PAUSE TestAccECSService_iamRole
=== RUN TestAccECSService_DeploymentControllerType_codeDeploy
=== PAUSE TestAccECSService_DeploymentControllerType_codeDeploy
=== RUN TestAccECSService_DeploymentControllerType_codeDeployUpdateDesiredCountAndHealthCheckGracePeriod
=== PAUSE TestAccECSService_DeploymentControllerType_codeDeployUpdateDesiredCountAndHealthCheckGracePeriod
=== RUN TestAccECSService_DeploymentControllerType_external
=== PAUSE TestAccECSService_DeploymentControllerType_external
=== RUN TestAccECSService_DeploymentValues_basic
=== PAUSE TestAccECSService_DeploymentValues_basic
=== RUN TestAccECSService_DeploymentValues_minZeroMaxOneHundred
=== PAUSE TestAccECSService_DeploymentValues_minZeroMaxOneHundred
=== RUN TestAccECSService_deploymentCircuitBreaker
=== PAUSE TestAccECSService_deploymentCircuitBreaker
=== RUN TestAccECSService_loadBalancerChanges
=== PAUSE TestAccECSService_loadBalancerChanges
=== RUN TestAccECSService_clusterName
=== PAUSE TestAccECSService_clusterName
=== RUN TestAccECSService_alb
=== PAUSE TestAccECSService_alb
=== RUN TestAccECSService_multipleTargetGroups
=== PAUSE TestAccECSService_multipleTargetGroups
=== RUN TestAccECSService_forceNewDeployment
=== PAUSE TestAccECSService_forceNewDeployment
=== RUN TestAccECSService_PlacementStrategy_basic
=== PAUSE TestAccECSService_PlacementStrategy_basic
=== RUN TestAccECSService_PlacementStrategy_missing
=== PAUSE TestAccECSService_PlacementStrategy_missing
=== RUN TestAccECSService_PlacementConstraints_basic
=== PAUSE TestAccECSService_PlacementConstraints_basic
=== RUN TestAccECSService_PlacementConstraints_emptyExpression
=== PAUSE TestAccECSService_PlacementConstraints_emptyExpression
=== RUN TestAccECSService_LaunchTypeFargate_basic
=== PAUSE TestAccECSService_LaunchTypeFargate_basic
=== RUN TestAccECSService_LaunchTypeFargate_platformVersion
=== PAUSE TestAccECSService_LaunchTypeFargate_platformVersion
=== RUN TestAccECSService_LaunchTypeFargate_waitForSteadyState
=== PAUSE TestAccECSService_LaunchTypeFargate_waitForSteadyState
=== RUN TestAccECSService_LaunchTypeFargate_updateWaitForSteadyState
=== PAUSE TestAccECSService_LaunchTypeFargate_updateWaitForSteadyState
=== RUN TestAccECSService_LaunchTypeEC2_network
=== PAUSE TestAccECSService_LaunchTypeEC2_network
=== RUN TestAccECSService_DaemonSchedulingStrategy_basic
=== PAUSE TestAccECSService_DaemonSchedulingStrategy_basic
=== RUN TestAccECSService_DaemonSchedulingStrategy_setDeploymentMinimum
=== PAUSE TestAccECSService_DaemonSchedulingStrategy_setDeploymentMinimum
=== RUN TestAccECSService_replicaSchedulingStrategy
=== PAUSE TestAccECSService_replicaSchedulingStrategy
=== RUN TestAccECSService_ServiceRegistries_basic
=== PAUSE TestAccECSService_ServiceRegistries_basic
=== RUN TestAccECSService_ServiceRegistries_container
=== PAUSE TestAccECSService_ServiceRegistries_container
=== RUN TestAccECSService_ServiceRegistries_changes
=== PAUSE TestAccECSService_ServiceRegistries_changes
=== RUN TestAccECSService_Tags_basic
=== PAUSE TestAccECSService_Tags_basic
=== RUN TestAccECSService_Tags_managed
=== PAUSE TestAccECSService_Tags_managed
=== RUN TestAccECSService_Tags_propagate
=== PAUSE TestAccECSService_Tags_propagate
=== RUN TestAccECSService_executeCommand
=== PAUSE TestAccECSService_executeCommand
=== CONT TestAccECSService_basic
=== CONT TestAccECSService_multipleTargetGroups
=== CONT TestAccECSService_iamRole
--- PASS: TestAccECSService_iamRole (60.53s)
=== CONT TestAccECSService_alb
--- PASS: TestAccECSService_basic (98.61s)
=== CONT TestAccECSService_clusterName
--- PASS: TestAccECSService_clusterName (73.84s)
=== CONT TestAccECSService_loadBalancerChanges
--- PASS: TestAccECSService_multipleTargetGroups (228.01s)
=== CONT TestAccECSService_deploymentCircuitBreaker
--- PASS: TestAccECSService_deploymentCircuitBreaker (74.02s)
=== CONT TestAccECSService_DeploymentValues_minZeroMaxOneHundred
--- PASS: TestAccECSService_alb (255.05s)
=== CONT TestAccECSService_DeploymentValues_basic
--- PASS: TestAccECSService_loadBalancerChanges (191.10s)
=== CONT TestAccECSService_DeploymentControllerType_codeDeploy
--- PASS: TestAccECSService_DeploymentValues_minZeroMaxOneHundred (73.94s)
=== CONT TestAccECSService_DaemonSchedulingStrategy_basic
--- PASS: TestAccECSService_DeploymentValues_basic (73.76s)
=== CONT TestAccECSService_executeCommand
--- PASS: TestAccECSService_DaemonSchedulingStrategy_basic (29.43s)
=== CONT TestAccECSService_Tags_propagate
--- PASS: TestAccECSService_executeCommand (120.23s)
=== CONT TestAccECSService_Tags_managed
--- PASS: TestAccECSService_Tags_propagate (117.82s)
=== CONT TestAccECSService_Tags_basic
--- PASS: TestAccECSService_Tags_managed (82.49s)
=== CONT TestAccECSService_ServiceRegistries_changes
--- PASS: TestAccECSService_DeploymentControllerType_codeDeploy (274.95s)
=== CONT TestAccECSService_ServiceRegistries_container
--- PASS: TestAccECSService_Tags_basic (120.01s)
=== CONT TestAccECSService_ServiceRegistries_basic
--- PASS: TestAccECSService_ServiceRegistries_container (169.54s)
=== CONT TestAccECSService_replicaSchedulingStrategy
--- PASS: TestAccECSService_ServiceRegistries_basic (167.64s)
=== CONT TestAccECSService_DeploymentControllerType_codeDeployUpdateDesiredCountAndHealthCheckGracePeriod
--- PASS: TestAccECSService_ServiceRegistries_changes (285.57s)
=== CONT TestAccECSService_DeploymentControllerType_external
--- PASS: TestAccECSService_replicaSchedulingStrategy (88.47s)
=== CONT TestAccECSService_CapacityProviderStrategy_update
--- PASS: TestAccECSService_DeploymentControllerType_external (44.89s)
=== CONT TestAccECSService_healthCheckGracePeriodSeconds
=== CONT TestAccECSService_CapacityProviderStrategy_update
service_test.go:199: Step 1/4 error: Error running apply: exit status 1
Error: creating Auto Scaling Group (tf-acc-test-3225431064940921206): ValidationError: You must use a valid fully-formed launch template. No default subnet for availability zone: 'us-west-2c'.
status code: 400, request id: 945d9889-d659-4200-aa5a-a66d0da852ce
with aws_autoscaling_group.test,
on terraform_plugin_test.tf line 27, in resource "aws_autoscaling_group" "test":
27: resource "aws_autoscaling_group" "test" {
--- FAIL: TestAccECSService_CapacityProviderStrategy_update (69.10s)
=== CONT TestAccECSService_renamedCluster
--- PASS: TestAccECSService_renamedCluster (84.07s)
=== CONT TestAccECSService_familyAndRevision
--- PASS: TestAccECSService_familyAndRevision (83.94s)
=== CONT TestAccECSService_CapacityProviderStrategy_multiple
--- PASS: TestAccECSService_DeploymentControllerType_codeDeployUpdateDesiredCountAndHealthCheckGracePeriod (324.04s)
=== CONT TestAccECSService_PlacementStrategy_unnormalized
--- PASS: TestAccECSService_healthCheckGracePeriodSeconds (266.83s)
=== CONT TestAccECSService_CapacityProviderStrategy_forceNewDeployment
--- PASS: TestAccECSService_PlacementStrategy_unnormalized (62.85s)
=== CONT TestAccECSService_CapacityProviderStrategy_basic
=== CONT TestAccECSService_CapacityProviderStrategy_forceNewDeployment
service_test.go:171: Step 1/2 error: Error running apply: exit status 1
Error: creating Auto Scaling Group (tf-acc-test-8409759270736927209): ValidationError: You must use a valid fully-formed launch template. No default subnet for availability zone: 'us-west-2c'.
status code: 400, request id: 35916f6b-9cff-4d8c-a734-081370400452
with aws_autoscaling_group.test,
on terraform_plugin_test.tf line 27, in resource "aws_autoscaling_group" "test":
27: resource "aws_autoscaling_group" "test" {
--- FAIL: TestAccECSService_CapacityProviderStrategy_forceNewDeployment (26.48s)
=== CONT TestAccECSService_LaunchTypeFargate_basic
=== CONT TestAccECSService_CapacityProviderStrategy_basic
service_test.go:144: Step 1/2 error: Error running apply: exit status 1
Error: creating Auto Scaling Group (tf-acc-test-6661787994854595710): ValidationError: You must use a valid fully-formed launch template. No default subnet for availability zone: 'us-west-2c'.
status code: 400, request id: b0c68474-604d-4b6e-b076-7aff234ae7ce
with aws_autoscaling_group.test,
on terraform_plugin_test.tf line 27, in resource "aws_autoscaling_group" "test":
27: resource "aws_autoscaling_group" "test" {
--- FAIL: TestAccECSService_CapacityProviderStrategy_basic (27.18s)
=== CONT TestAccECSService_LaunchTypeEC2_network
--- PASS: TestAccECSService_CapacityProviderStrategy_multiple (107.90s)
=== CONT TestAccECSService_LaunchTypeFargate_updateWaitForSteadyState
--- PASS: TestAccECSService_LaunchTypeEC2_network (99.70s)
=== CONT TestAccECSService_LaunchTypeFargate_waitForSteadyState
--- PASS: TestAccECSService_LaunchTypeFargate_basic (148.16s)
=== CONT TestAccECSService_LaunchTypeFargate_platformVersion
--- PASS: TestAccECSService_LaunchTypeFargate_waitForSteadyState (156.59s)
=== CONT TestAccECSService_PlacementStrategy_missing
--- PASS: TestAccECSService_PlacementStrategy_missing (1.01s)
=== CONT TestAccECSService_PlacementConstraints_emptyExpression
--- PASS: TestAccECSService_LaunchTypeFargate_platformVersion (149.55s)
=== CONT TestAccECSService_PlacementConstraints_basic
--- PASS: TestAccECSService_LaunchTypeFargate_updateWaitForSteadyState (272.39s)
=== CONT TestAccECSService_disappears
--- PASS: TestAccECSService_PlacementConstraints_emptyExpression (74.05s)
=== CONT TestAccECSService_forceNewDeployment
--- PASS: TestAccECSService_PlacementConstraints_basic (86.42s)
=== CONT TestAccECSService_basicImport
--- PASS: TestAccECSService_forceNewDeployment (75.36s)
=== CONT TestAccECSService_DaemonSchedulingStrategy_setDeploymentMinimum
--- PASS: TestAccECSService_DaemonSchedulingStrategy_setDeploymentMinimum (29.80s)
=== CONT TestAccECSService_PlacementStrategy_basic
--- PASS: TestAccECSService_disappears (182.24s)
--- PASS: TestAccECSService_PlacementStrategy_basic (113.23s)
--- PASS: TestAccECSService_basicImport (208.13s)
FAIL
FAIL github.com/hashicorp/terraform-provider-aws/internal/service/ecs 1812.091s
FAIL
make: *** [testacc] Error 1
Failures are unrelated to this change.
@sgirones Thanks for the contribution 🎉 👏. |
thanks for the quick review/merge 🙌 |
BACKPORT hashicorp#27390 fix resource/aws_ecs_service: make task_definition truly optional
Fix #709, which was backport hashicorp#27390, but missed a line of deletion
This functionality has been released in v4.37.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Description
Make the
task_definition
foraws_ecs_service
truly optional.It is defined as so in the schema, but when preparing the request body to be sent to the AWS API, it was adding the taskDefinition parameter with an empty string.
AWS doesn't accept it when defining a Service with
Deployment=EXTERNAL
:Relations
Closes #27362
References
Output from Acceptance Testing
Not sure what's the best way to test this. The result of applying the ECS service doesn't change, only what is sent to the AWS API