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

feat(compose): Add EFS support for task workflows #1076

Merged
merged 9 commits into from
Jun 30, 2020

Conversation

bvtujo
Copy link
Contributor

@bvtujo bvtujo commented Jun 29, 2020

Picks up from #1056 and adds tests, additional safety check for launch type, and adds test helper function to task_test.go.

Addresses #1043


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)
  • Link to issue or PR for the integration tests:

Documentation

  • Contacted our doc writer
  • Updated our README

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

tatsuya-ogawa and others added 5 commits June 6, 2020 17:36
* Adds additional check in service to ensure that there can be no panic
  if ecsParams is not specified
* Adds launchType check in task to ensure that PV is never unnecessarily
  set for EC2 launch types.
@bvtujo
Copy link
Contributor Author

bvtujo commented Jun 29, 2020

Integ tests launched 12:20pm.

ed 1:31: Integ tests passed.

[Container] 2020/06/29 19:23:23 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 (473.17s)
31 | configure.go:53: Created config ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-ec2-tutorial-config
32 | up.go:68: Created cluster ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-ec2-tutorial-cluster in stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-ec2-tutorial-cluster
33 | ecs.go:43: Number of container instances mismatch, wanted = 2, got = 0
34 | runner.go:98: Current timestamp=2020-06-29 19:27:15.270993924 +0000 UTC m=+181.799953023, sleeping for 15s
35 | ecs.go:43: Number of container instances mismatch, wanted = 2, got = 0
36 | runner.go:98: Current timestamp=2020-06-29 19:27:30.3069127 +0000 UTC m=+196.835871794, sleeping for 15s
37 | ecs.go:43: Number of container instances mismatch, wanted = 2, got = 0
38 | runner.go:98: Current timestamp=2020-06-29 19:27:45.343627081 +0000 UTC m=+211.872586180, sleeping for 15s
39 | ecs.go:43: Number of container instances mismatch, wanted = 2, got = 1
40 | runner.go:98: Current timestamp=2020-06-29 19:28:00.42362086 +0000 UTC m=+226.952579960, sleeping for 15s
41 | ecs.go:51: Cluster ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-ec2-tutorial-cluster has 2 instances
42 | ec2_task_test.go:98: Created /tmp/docker-compose-080049368.yml successfully
43 | compose.go:70: Created containers for e2e-ec2-tutorial-taskdef
44 | ecs.go:75: Cluster ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-ec2-tutorial-cluster has 1 tasks
45 | ps.go:36: Project e2e-ec2-tutorial-taskdef has 2 running containers
46 | compose.go:141: Scaled the task e2e-ec2-tutorial-taskdef to 2
47 | ecs.go:75: Cluster ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-ec2-tutorial-cluster has 2 tasks
48 | ps.go:36: Project e2e-ec2-tutorial-taskdef has 4 running containers
49 | compose.go:200: Deleted task e2e-ec2-tutorial-taskdef
50 | ecs.go:75: Cluster ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-ec2-tutorial-cluster has 0 tasks
51 | down.go:49: Deleted cluster ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-ec2-tutorial-cluster
52 | cfn.go:50: Success: stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-ec2-tutorial-cluster does not exist
53 | down.go:52: Deleted stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-ec2-tutorial-cluster
54 | === RUN   TestCreateClusterWithFargateService
55 | --- PASS: TestCreateClusterWithFargateService (274.76s)
56 | configure.go:42: Created config ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-fargate-tutorial-config
57 | up.go:68: Created cluster ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-fargate-tutorial-cluster in stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-fargate-tutorial-cluster
58 | fargate_service_test.go:86: Created /tmp/docker-compose-904826591.yml successfully
59 | fargate_service_test.go:117: Created /tmp/ecs-params-572535986.yml successfully
60 | compose.go:102: Created service with name ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-e2e-fargate-test-service
61 | compose.go:280: Container is RUNNING: e1db862d-a785-49f8-a9aa-a9ac31a4b4fe/wordpress  RUNNING  54.202.91.104:80->80/tcp  ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-e2e-fargate-test-service:1  UNKNOWN
62 | compose.go:113: Project ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-e2e-fargate-test-service has 1 running containers
63 | compose.go:173: Scaled the service ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-e2e-fargate-test-service to 2 tasks
64 | compose.go:280: Container is RUNNING: d44077ed-b80b-45c5-9ab7-7d48dc902cfe/wordpress  RUNNING  54.70.227.31:80->80/tcp   ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-e2e-fargate-test-service:1  UNKNOWN
65 | compose.go:280: Container is RUNNING: e1db862d-a785-49f8-a9aa-a9ac31a4b4fe/wordpress  RUNNING  54.202.91.104:80->80/tcp  ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-e2e-fargate-test-service:1  UNKNOWN
66 | compose.go:113: Project ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-e2e-fargate-test-service has 2 running containers
67 | compose.go:232: Deleted service ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-e2e-fargate-test-service
68 | down.go:49: Deleted cluster ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-fargate-tutorial-cluster
69 | cfn.go:50: Success: stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-fargate-tutorial-cluster does not exist
70 | down.go:52: Deleted stack amazon-ecs-cli-setup-ecs-cli-on-demand-integ-test-779b5966-49de-4d9e-afd3-79dd9417423e-fargate-tutorial-cluster
71 | === RUN   TestECSLocal
72 | === RUN   TestECSLocal/clean_state
73 | === RUN   TestECSLocal/from_task_def
74 | --- PASS: TestECSLocal (70.01s)
75 | --- PASS: TestECSLocal/clean_state (0.11s)
76 | --- PASS: TestECSLocal/from_task_def (69.90s)
77 | PASS
78 | ok      github.com/aws/amazon-ecs-cli/ecs-cli/integ/e2e 817.946s

* Refactor ecsParamsWithEFSVolume() helper function to take no arguments
* Change EFSVolumes check in task.go to be > 0, not != 0.
Copy link
Contributor

@allisaurus allisaurus left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Contributor

@SoManyHs SoManyHs left a comment

Choose a reason for hiding this comment

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

LGTM! Just small (non-blocking) questions.

log.Warnf("Detected an EFS Volume in task definition %s", taskDefinition)
log.Warn("Using Fargate platform version 1.4.0, which includes changes to the networking flows for VPC endpoint customers.")
log.Warn("Learn more: https://aws.amazon.com/blogs/containers/aws-fargate-launches-platform-version-1-4/")
runTaskInput.PlatformVersion = aws.String("1.4.0")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: wondering if it's worth constantizing this?

@bvtujo bvtujo requested a review from SoManyHs June 30, 2020 20:14
@mergify mergify bot merged commit c91e48e 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

4 participants