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

fix(service): Replace errors from invalid compose service up flags with warnings #1071

Merged
merged 2 commits into from
Jun 30, 2020

Conversation

bvtujo
Copy link
Contributor

@bvtujo bvtujo commented Jun 25, 2020

Previously, running ecs-cli compose service up on an already existing service and specifying the flags --enable-service-discovery would result in errors.

This was bad behavior, as commands are sometimes scripted without respect to the status of the service and we should offer idempotency in this case when possible.

This PR adjusts the behavior when the service already exists to display warning messages and continue, ignoring those flags, instead of failing.

Addresses #1069


Enter [N/A] in the box, if an item is not applicable to your change.

Testing

  • Unit tests passed
  • Integration tests passed
  • Unit tests added for new functionality
  • [n/a] Listed manual checks and their outputs in the comments (example)
  • [n/a ] Link to issue or PR for the integration tests:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@bvtujo
Copy link
Contributor Author

bvtujo commented Jun 25, 2020

Integ Test Output

Installing dependencies...
--
23 | go get github.com/wadey/gocovmerge
24 | Building ecs-cli.test...
25 | env -i PATH=$PATH GOPATH=$(go env GOPATH) GOROOT=$(go env GOROOT) GOCACHE=$(go env GOCACHE) \
26 | go test -coverpkg ./ecs-cli/modules/... -c -tags testrunmain -o ./bin/local/ecs-cli.test ./ecs-cli
27 | Running integration tests...
28 | go test -timeout 60m -tags integ -v ./ecs-cli/integ/e2e/...
29 | === RUN   TestCreateClusterWithEC2Task
30 | --- PASS: TestCreateClusterWithEC2Task (427.67s)
31 | configure.go:53: Created config ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-ec2-tutorial-config
32 | up.go:68: Created cluster ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-ec2-tutorial-cluster in stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-ec2-tutorial-cluster
33 | ecs.go:43: Number of container instances mismatch, wanted = 2, got = 0
34 | runner.go:98: Current timestamp=2020-06-25 22:03:30.059456209 +0000 UTC m=+181.718651173, sleeping for 15s
35 | ecs.go:43: Number of container instances mismatch, wanted = 2, got = 0
36 | runner.go:98: Current timestamp=2020-06-25 22:03:45.098298388 +0000 UTC m=+196.757493388, sleeping for 15s
37 | ecs.go:43: Number of container instances mismatch, wanted = 2, got = 1
38 | runner.go:98: Current timestamp=2020-06-25 22:04:00.136760399 +0000 UTC m=+211.795955359, sleeping for 15s
39 | ecs.go:51: Cluster ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-ec2-tutorial-cluster has 2 instances
40 | ec2_task_test.go:98: Created /tmp/docker-compose-009089854.yml successfully
41 | compose.go:70: Created containers for e2e-ec2-tutorial-taskdef
42 | ecs.go:75: Cluster ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-ec2-tutorial-cluster has 1 tasks
43 | ps.go:36: Project e2e-ec2-tutorial-taskdef has 2 running containers
44 | compose.go:141: Scaled the task e2e-ec2-tutorial-taskdef to 2
45 | ecs.go:75: Cluster ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-ec2-tutorial-cluster has 2 tasks
46 | ps.go:36: Project e2e-ec2-tutorial-taskdef has 4 running containers
47 | compose.go:200: Deleted task e2e-ec2-tutorial-taskdef
48 | ecs.go:75: Cluster ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-ec2-tutorial-cluster has 0 tasks
49 | down.go:49: Deleted cluster ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-ec2-tutorial-cluster
50 | cfn.go:50: Success: stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-ec2-tutorial-cluster does not exist
51 | down.go:52: Deleted stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-ec2-tutorial-cluster
52 | === RUN   TestCreateClusterWithFargateService
53 | --- PASS: TestCreateClusterWithFargateService (304.77s)
54 | configure.go:42: Created config ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-fargate-tutorial-config
55 | up.go:68: Created cluster ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-fargate-tutorial-cluster in stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-fargate-tutorial-cluster
56 | fargate_service_test.go:86: Created /tmp/docker-compose-192075725.yml successfully
57 | fargate_service_test.go:117: Created /tmp/ecs-params-789512904.yml successfully
58 | compose.go:102: Created service with name ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-e2e-fargate-test-service
59 | compose.go:280: Container is RUNNING: dd43c7f2-fa09-49ff-a694-a1f2044451f2/wordpress  RUNNING  54.218.215.47:80->80/tcp  ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-e2e-fargate-test-service:1  UNKNOWN
60 | compose.go:113: Project ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-e2e-fargate-test-service has 1 running containers
61 | compose.go:173: Scaled the service ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-e2e-fargate-test-service to 2 tasks
62 | compose.go:280: Container is RUNNING: 89edc68e-d0ec-48a6-a8d4-8211e02ffea8/wordpress  RUNNING  35.162.147.124:80->80/tcp  ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-e2e-fargate-test-service:1  UNKNOWN
63 | compose.go:280: Container is RUNNING: dd43c7f2-fa09-49ff-a694-a1f2044451f2/wordpress  RUNNING  54.218.215.47:80->80/tcp   ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-e2e-fargate-test-service:1  UNKNOWN
64 | compose.go:113: Project ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-e2e-fargate-test-service has 2 running containers
65 | compose.go:232: Deleted service ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-e2e-fargate-test-service
66 | down.go:49: Deleted cluster ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-fargate-tutorial-cluster
67 | cfn.go:50: Success: stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-fargate-tutorial-cluster does not exist
68 | down.go:52: Deleted stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-5b0e0fb7-0a31-4354-991a-971dd96c103a-fargate-tutorial-cluster
69 | === RUN   TestECSLocal
70 | === RUN   TestECSLocal/clean_state
71 | === RUN   TestECSLocal/from_task_def
72 | --- PASS: TestECSLocal (65.04s)
73 | --- PASS: TestECSLocal/clean_state (0.11s)
74 | --- PASS: TestECSLocal/from_task_def (64.93s)
75 | PASS
76 | ok      github.com/aws/amazon-ecs-cli/ecs-cli/integ/e2e 797.491s
77 | Code coverage

@bvtujo bvtujo added the do-not-merge blocks auto-merging by mergify label Jun 30, 2020
@bvtujo
Copy link
Contributor Author

bvtujo commented Jun 30, 2020

Pending re-running integ tests

@bvtujo
Copy link
Contributor Author

bvtujo commented Jun 30, 2020

[Container] 2020/06/30 20:37:02 Running command make integ-test
--
22 | Installing dependencies...
23 | go get github.com/wadey/gocovmerge
24 | Building ecs-cli.test...
25 | env -i PATH=$PATH GOPATH=$(go env GOPATH) GOROOT=$(go env GOROOT) GOCACHE=$(go env GOCACHE) \
26 | go test -coverpkg ./ecs-cli/modules/... -c -tags testrunmain -o ./bin/local/ecs-cli.test ./ecs-cli
27 | Running integration tests...
28 | go test -timeout 60m -tags integ -v ./ecs-cli/integ/e2e/...
29 | === RUN   TestCreateClusterWithEC2Task
30 | --- PASS: TestCreateClusterWithEC2Task (442.81s)
31 | configure.go:53: Created config ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-ec2-tutorial-config
32 | up.go:68: Created cluster ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-ec2-tutorial-cluster in stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-ec2-tutorial-cluster
33 | ecs.go:43: Number of container instances mismatch, wanted = 2, got = 0
34 | runner.go:98: Current timestamp=2020-06-30 20:40:56.205246186 +0000 UTC m=+181.742441374, sleeping for 15s
35 | ecs.go:43: Number of container instances mismatch, wanted = 2, got = 1
36 | runner.go:98: Current timestamp=2020-06-30 20:41:11.238951934 +0000 UTC m=+196.776147091, sleeping for 15s
37 | ecs.go:51: Cluster ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-ec2-tutorial-cluster has 2 instances
38 | ec2_task_test.go:98: Created /tmp/docker-compose-909739111.yml successfully
39 | compose.go:70: Created containers for e2e-ec2-tutorial-taskdef
40 | ecs.go:75: Cluster ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-ec2-tutorial-cluster has 1 tasks
41 | ps.go:36: Project e2e-ec2-tutorial-taskdef has 2 running containers
42 | compose.go:141: Scaled the task e2e-ec2-tutorial-taskdef to 2
43 | ecs.go:75: Cluster ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-ec2-tutorial-cluster has 2 tasks
44 | ps.go:36: Project e2e-ec2-tutorial-taskdef has 4 running containers
45 | compose.go:200: Deleted task e2e-ec2-tutorial-taskdef
46 | ecs.go:75: Cluster ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-ec2-tutorial-cluster has 0 tasks
47 | down.go:49: Deleted cluster ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-ec2-tutorial-cluster
48 | cfn.go:50: Success: stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-ec2-tutorial-cluster does not exist
49 | down.go:52: Deleted stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-ec2-tutorial-cluster
50 | === RUN   TestCreateClusterWithFargateService
51 | --- PASS: TestCreateClusterWithFargateService (234.43s)
52 | configure.go:42: Created config ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-fargate-tutorial-config
53 | up.go:68: Created cluster ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-fargate-tutorial-cluster in stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-fargate-tutorial-cluster
54 | fargate_service_test.go:86: Created /tmp/docker-compose-209405826.yml successfully
55 | fargate_service_test.go:117: Created /tmp/ecs-params-923587833.yml successfully
56 | compose.go:102: Created service with name ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-e2e-fargate-test-service
57 | compose.go:280: Container is RUNNING: 8d96af14-646d-46a6-8116-a50fe4bbca5c/wordpress  RUNNING  54.184.100.255:80->80/tcp  ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-e2e-fargate-test-service:1  UNKNOWN
58 | compose.go:113: Project ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-e2e-fargate-test-service has 1 running containers
59 | compose.go:173: Scaled the service ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-e2e-fargate-test-service to 2 tasks
60 | compose.go:280: Container is RUNNING: 8d96af14-646d-46a6-8116-a50fe4bbca5c/wordpress  RUNNING  54.184.100.255:80->80/tcp  ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-e2e-fargate-test-service:1  UNKNOWN
61 | compose.go:280: Container is RUNNING: 92a14c8d-2adc-4284-8727-f0da79242cca/wordpress  RUNNING  18.236.194.70:80->80/tcp   ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-e2e-fargate-test-service:1  UNKNOWN
62 | compose.go:113: Project ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-e2e-fargate-test-service has 2 running containers
63 | compose.go:232: Deleted service ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-e2e-fargate-test-service
64 | down.go:49: Deleted cluster ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-fargate-tutorial-cluster
65 | cfn.go:50: Success: stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-fargate-tutorial-cluster does not exist
66 | down.go:52: Deleted stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-a1c9b033-6531-4b34-8a33-4e5f11e62e4b-fargate-tutorial-cluster
67 | === RUN   TestECSLocal
68 | === RUN   TestECSLocal/clean_state
69 | === RUN   TestECSLocal/from_task_def
70 | --- PASS: TestECSLocal (63.80s)
71 | --- PASS: TestECSLocal/clean_state (0.11s)
72 | --- PASS: TestECSLocal/from_task_def (63.69s)
73 | PASS
74 | ok      github.com/aws/amazon-ecs-cli/ecs-cli/integ/e2e 741.052s

@bvtujo bvtujo removed the do-not-merge blocks auto-merging by mergify label Jun 30, 2020
@mergify mergify bot merged commit e39b08b into aws:master Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants