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: "compose service up" sets desired count to 1 #988

Merged
merged 1 commit into from
Feb 25, 2020

Conversation

SoManyHs
Copy link
Contributor

@SoManyHs SoManyHs commented Feb 24, 2020

Previously, we would first call CreateService with desired count 0 then
call UpdateService to set the desired count to 1. This eliminates extra
API calls, as well as the need to wait until the service is describable
before "starting" it.

Note: This effectively reverts 264fe13

Related: #79


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
  • Listed manual checks and their outputs in the comments (example)
  • Link to issue or PR for the integration tests:

Documentation

  • [N/A] Contacted our doc writer
  • [N/A] Updated our README

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

Previously, we would first call CreateService with desired count 0 then
call UpdateService to set the desired count to 1. This eliminates extra
API calls, as well as the need to wait until the service is describable
before "starting" it.

Note: This effectively reverts 264fe13

Related: aws#79
@SoManyHs
Copy link
Contributor Author

Compose service up now creates service with desired count = 1:

$ ecs-cli --version
ecs-cli version 1.18.0 (*41b3dc2)

$ ecs-cli compose service up
INFO[0000] Using ECS task definition                     TaskDefinition="test_fields:72"
INFO[0000] Created an ECS service   

$ aws ecs describe-services --region us-west-2 --service test_fields
{
    "services": [
        {
            "status": "ACTIVE", 
            "serviceRegistries": [], 
            "pendingCount": 0, 
            "launchType": "EC2", 
            "enableECSManagedTags": false, 
            "schedulingStrategy": "REPLICA", 
            "loadBalancers": [], 
            "placementConstraints": [], 
            "createdAt": 1582576760.559, 
            "desiredCount": 1, 
            "serviceName": "test_fields", 
            "clusterArn": "arn:aws:ecs:us-west-2:xxxxxxx:cluster/default", 
            "taskDefinition": "arn:aws:ecs:us-west-2:xxxxxxxx:task-definition/test_fields:72", 
            "serviceArn": "arn:aws:ecs:us-west-2:xxxxx:service/test_fields", 
            "propagateTags": "NONE", 
            "deploymentConfiguration": {
                "maximumPercent": 200, 
                "minimumHealthyPercent": 100
            }, 
            "deployments": [
                {
                    "status": "PRIMARY", 
                    "pendingCount": 0, 
                    "launchType": "EC2", 
                    "createdAt": 1582576760.559, 
                    "desiredCount": 1, 
                    "taskDefinition": "arn:aws:ecs:us-west-2:xxxxxxxx:task-definition/test_fields:72", 
                    "updatedAt": 1582576785.549, 
                    "id": "ecs-svc/4901267662372024888", 
                    "runningCount": 1
                }
            ], 
            "events": [
                {
                    "message": "(service test_fields) has reached a steady state.", 
                    "id": "52395df6-cdcc-44a8-a85b-f9d4f7ab91b4", 
                    "createdAt": 1582576785.557
                }, 
                {
                    "message": "(service test_fields) has started 1 tasks: (task 64b69c58-6ed3-4c3f-a4e6-5c8bdc5bd552).", 
                    "id": "de52edbb-f4ca-40ea-98fb-40bd4268f7c8", 
                    "createdAt": 1582576764.429
                }
            ], 
            "runningCount": 1, 
            "placementStrategy": []
        }
    ], 
    "failures": []
}

@SoManyHs
Copy link
Contributor Author

Previous behavior (made two separate API calls):

~/ecs/ecs-cli/testing/projects/test_fields(master)$ ecs-cli --version
ecs-cli version 1.18.0 (*26c7177)

~/ecs/ecs-cli/testing/projects/test_fields(master)$ ecs-cli compose service up
INFO[0000] Using ECS task definition                     TaskDefinition="test_fields:72"
INFO[0000] Created an ECS service                        service=test_fields taskDefinition="test_fields:72"
INFO[0000] Updated ECS service successfully              desiredCount=1 force-deployment=false service=test_fields
INFO[0015] Service status                                desiredCount=1 runningCount=1 serviceName=test_fields
INFO[0015] (service test_fields) has started 1 tasks: (task 4c9d2052-7bd7-4e2c-8b28-7ae655ccb5ac).  timestamp="2020-02-24 20:45:37 +0000 UTC"
INFO[0015] ECS Service has reached a stable state        desiredCount=1 runningCount=1 serviceName=test_fields

Copy link
Contributor

@uttarasridhar uttarasridhar left a comment

Choose a reason for hiding this comment

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

Reading the documentation for the up command:
Creates an Amazon ECS service from your compose file (if it does not already exist) and runs one instance of that task on your cluster (a combination of the create and start commands). This command updates the desired count of the service to 1.
Can we make sure the command is idempotent in the sense that if customers run compose service create and follow it with compose service up it should succeed.

This might require us to check for ServiceAlreadyExists Error in line 249 and call updateService instead

@efekarakus
Copy link
Contributor

Can we make sure the command is idempotent in the sense that if customers run compose service create and follow it with compose service up it should succeed.

I believe we'll go inside if the if-block iff the service does not exist or is inactive.
If the service already exists then l. 253 updateService is invoked.

So the behavior of the command should still be like before :)

@SoManyHs
Copy link
Contributor Author

@uttarasridhar


$ ecs-cli --version
ecs-cli version 1.18.0 (*41b3dc2)

$ ecs-cli compose service create
INFO[0000] Using ECS task definition                     TaskDefinition="test_fields:72"
INFO[0000] Created an ECS service                        service=test_fields taskDefinition="test_fields:72"

$ aws ecs list-services --region us-west-2 
{
    "serviceArns": [
        "arn:aws:ecs:us-west-2:xxxxxx:service/test_fields"
    ]
}

$ ecs-cli compose service up
INFO[0000] Using ECS task definition                     TaskDefinition="test_fields:72"
INFO[0000] Updated ECS service successfully              desiredCount=1 force-deployment=false service=test_fields
INFO[0015] Service status                                desiredCount=1 runningCount=1 serviceName=test_fields
INFO[0015] (service test_fields) has started 1 tasks: (task b24f3e4b-d190-487f-a8ad-5faa44e00ffa).  timestamp="2020-02-25 01:06:19 +0000 UTC"
INFO[0015] ECS Service has reached a stable state        desiredCount=1 runningCount=1 serviceName=test_fields

@SoManyHs SoManyHs merged commit 41b3dc2 into aws:master Feb 25, 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.

3 participants