-
Notifications
You must be signed in to change notification settings - Fork 470
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
Supported AWS Fargate #129
Conversation
Worked perfectly for me |
@tacchino |
@kakakakakku it is not working for me yet, sadly
Here's a verbose dump, with env. vars truncated and account id redacted |
@kakakakakku thank you for your work on this. I am getting the same error as @zeroasterisk. I upgraded my local awscli to 1.14.10 in case it was a change to the API but that didn't help. For reference I am testing updating a task/service that is NOT using Fargate. Also unit tests fail. If you have Docker and Docker Compose installed you can run
|
@zeroasterisk @fillup |
3e3b592
to
675e0e0
Compare
Your task definition (JSON) seem doesn't have So, We must support these patterns.
Then I fixed with using Before# [1]
$ echo '{ "status": "ACTIVE" }' | jq -r '.requiresCompatibilities[]'
jq: error (at <stdin>:1): Cannot iterate over null (null)
# [2]
$ echo '{ "status": "ACTIVE", "requiresCompatibilities": null }' | jq -r '.requiresCompatibilities[]'
jq: error (at <stdin>:1): Cannot iterate over null (null)
# [3]
$ echo '{ "status": "ACTIVE", "requiresCompatibilities": [ "EC2" ] }' | jq -r '.requiresCompatibilities[]'
EC2
# [4]
$ echo '{ "status": "ACTIVE", "requiresCompatibilities": [ "FARGATE" ] }' | jq -r '.requiresCompatibilities[]'
FARGATE After# [1]
$ echo '{ "status": "ACTIVE" }' | jq -r '. | select(.requiresCompatibilities != null) | .requiresCompatibilities[]'
# [2]
$ echo '{ "status": "ACTIVE", "requiresCompatibilities": null }' | jq -r '. | select(.requiresCompatibilities != null) | .requiresCompatibilities[]'
# [3]
$ echo '{ "status": "ACTIVE", "requiresCompatibilities": [ "EC2" ] }' | jq -r '. | select(.requiresCompatibilities != null) | .requiresCompatibilities[]'
EC2
# [4]
$ echo '{ "status": "ACTIVE", "requiresCompatibilities": [ "FARGATE" ] }' | jq -r '. | select(.requiresCompatibilities != null) | .requiresCompatibilities[]'
FARGATE testsI added tests, and pass succsessflly.
Please review again 👍 |
@kakakakakku I am looking at https://github.com/kakakakakku/ecs-deploy/tree/fargate-support and your last commit is I updated it and ran it and got:
|
@zeroasterisk
Thank you for your reports.
If you try EC2 mode,
Thanks 👍 |
These are I do have a memory allocation at the |
I found success or failure to be AWS CLI version dependent. Older aws cli versions didn't work, newer ones did. For example we had aws v0.11.39 on a build machine which failed to update a Fargate service, but updating aws to 0.14.9 worked just fine for the same service. |
@richardbolt |
@zeroasterisk Please give me your task definition (JSON), or http://sebsauvage.net/paste/?3f76a75b911230c8#FDlFZAxjQJwA1oAPflGIrsogE6Ll/A10wAFodUpNPcs= is it ? |
@kakakakakku |
@zeroasterisk |
Is this ready to go now? |
Yes, I'm OK ! |
I tried the code on our system and the deployment worked but did not perform a graceful blue-green deployment. The code killed the running revision immediately and ran the new revision successfully on ECS, but produced a script error:
|
@shimont Please give me |
@kakakakakku The task diffs looks good - the problem seems to be with the blue-green deployment logic. Task 1 was killed immediately before having task 2 ready. Diff 1
Diff 2
Task Definision 1
Task Definition 2
|
@kakakakakku after performing further tests I think that the problem was on my end. The task didnt register correctly via the ALB due to an error in the configuration of the health check endpoint. I've performed the test again and I can confirm that a full cycle of deployment to FARGATE is working great! The only thing I suggest people do is change the --timeout argument from 90sec to 360sec as FARGATE can take some time to deploy the new container |
@shimont Yes, I think so too ! (My opinion) Because "Fargate" require
Owner, please review continuously 👍 |
any plans for merging this ? |
Could you please merge this? |
Tested and approved. +1 for merging this. |
Working! Please merge |
Bumping this, looking forward to a merge! |
I can also confirm that this fixes Fargate deploys. Please merge! |
Bumping, need this sooner than later =) |
Hi all, I'm really sorry for taking forever to catch up with this. I just pulled this down but one of the new tests is failing, @kakakakakku can you check this?
Also as an FYI, this does not work with tasks that were created a while ago (dont, maybe pre-fargate?). When running this update against a test service I have it throws a different error that seems to be from the aws-cli itself as it has python type errors:
I went into the AWS console and created a new revision without changing anything and re-ran ecs-deploy and it worked just fine. When we get the test issue resolved I'll be happy to get this merged in. |
@fillup But all tests are passed on December, see my comment / #129 (comment) . |
6d40c1a
to
9624c4e
Compare
9624c4e
to
423b003
Compare
@fillup on January $ diff -u output.json expected.json
--- output.json 2018-03-31 13:00:13.000000000 +0900
+++ expected.json 2018-03-31 13:00:23.000000000 +0900
@@ -22,7 +22,6 @@
"cpu": 200,
"volumesFrom": []
}],
- "placementConstraints": null,
"networkMode": "awsvpc",
"executionRoleArn": "arn:aws:iam::121212345678:role/ecsTaskExecutionRole",
"requiresCompatibilities": ["FARGATE"], So I rebased from develop tests$ docker-compose run --rm test
fetch http://dl-3.alpinelinux.org/alpine/edge/community/x86_64/APKINDEX.tar.gz
fetch http://dl-cdn.alpinelinux.org/alpine/v3.5/main/x86_64/APKINDEX.tar.gz
fetch http://dl-cdn.alpinelinux.org/alpine/v3.5/community/x86_64/APKINDEX.tar.gz
(1/1) Installing bats (0.4.0-r2)
Executing busybox-1.25.1-r1.trigger
OK: 65 MiB in 31 packages
1..33
ok 1 # skip (0) check that usage() returns string and exits with status code 20
ok 2 # skip (0) test assertRequiredArgumentsSet success
ok 3 # skip (0) test assertRequiredArgumentsSet status=5
ok 4 # skip (0) test assertRequiredArgumentsSet status=6
ok 5 # skip (0) test assertRequiredArgumentsSet status=7
ok 6 # skip (0) test assertRequiredArgumentsSet status=8
ok 7 # skip (0) test assertRequiredArgumentsSet status=9
ok 8 # skip (0) test parseImageName missing image name
ok 9 # skip (0) test parseImageName invalid image name 1
ok 10 # skip (0) test parseImageName invalid port
ok 11 # skip (0) test parseImageName root image no tag
ok 12 # skip (0) test parseImageName root image with tag
ok 13 # skip (0) test parseImageName repo image no tag
ok 14 # skip (0) test parseImageName repo image with tag
ok 15 # skip (0) test parseImageName repo multilevel image no tag
ok 16 # skip (0) test parseImageName repo multilevel image with tag
ok 17 # skip (0) test parseImageName domain plus repo image no tag
ok 18 # skip (0) test parseImageName domain plus repo image with tag
ok 19 # skip (0) test parseImageName domain plus repo multilevel image no tag
ok 20 # skip (0) test parseImageName domain plus repo multilevel image with tag
ok 21 # skip (0) test parseImageName domain plus port plus repo image no tag
ok 22 # skip (0) test parseImageName domain plus port plus repo image with tag
ok 23 # skip (0) test parseImageName domain plus port plus repo multilevel image no tag
ok 24 # skip (0) test parseImageName domain plus port plus repo multilevel image with tag
ok 25 # skip (0) test parseImageName domain plus port plus repo image with tag from var
ok 26 # skip (0) test parseImageName domain plus port plus repo multilevel image with tag from var
ok 27 # skip (0) test parseImageName using ecr style domain
ok 28 # skip (0) test parseImageName using ecr style image name and tag from var
ok 29 # skip (0) test createNewTaskDefJson with single container in definition
ok 30 # skip (0) test createNewTaskDefJson with single container in definition for AWS Fargate
ok 31 # skip (0) test createNewTaskDefJson with multiple containers in definition
ok 32 # skip (0) test parseImageName with tagonly option
ok 33 # skip (0) test createNewTaskDefJson with multiple containers in definition and replace only tags Please review, Thanks ! |
Thanks @kakakakakku for getting that fixed, it all looks good to me so I've merged it in and will roll out a release shortly |
WHAT IS THIS
AWS released new service "AWS Fargate" on November 30, 2017.
This Pull Request goal is supporting AWS Fargate deployment, and solve this Issue ( #127 ).
ERROR
This error occurred by latest ecs-deploy.
Because
Launch Types
( parameter name isrequiresCompatibilities
) added on Task Definition.Then
requiresCompatibilities
set valueFARGATE
orEC2
, butEC2
is default ECS.REQUIRE
If you deploy AWS Fargate using
aws-cli 1.14.0 or later
please.DETAILS
requiresCompatibilities
via Task Definition (JSON)FARGATE
jq filters updatedDeployment is successful in my Fargate Cluster.
ANY HELP
I am little worried about deployment for other Fargate Cluster.
If someone has own Fargate Cluster, please check deployment with my version ??? 🙏
( This is reason why this Pull Request is [WIP] . )Thanks 👍 👍 👍