Fix container instance tagging #745

PettitWesley · 2019-03-09T18:40:36Z

Issue #, if available:
#744 #670

Description of changes:
Bug fix for tagging, ec2:DescribeTags permissions are needed. Please see the comment in the issue for a full explanation, and workarounds: #744 (comment)

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

PettitWesley · 2019-03-09T18:43:31Z

$ ecs-cli up --capability-iam --cluster tagging-bug --region us-west-1 --tags ugh=ithinkitsabug --keypair SFOPair --force
WARN[0000] Enabling container instance tagging because containerInstanceLongArnFormat is enabled for your identity, arn:aws:iam::REDACTED:user/Wesley3.0. If this is not your account default setting, your instances will fail to join your cluster. You can use the PutAccountSettingDefault API to change your account default.
INFO[0000] Defaulting instance type to t2.micro
INFO[0001] Using recommended Amazon Linux 2 AMI with ECS Agent 1.26.0 and Docker version 18.06.1-ce
INFO[0001] Created cluster                               cluster=tagging-bug region=us-west-1
INFO[0001] Waiting for your CloudFormation stack resources to be deleted...
INFO[0001] Cloudformation stack status                   stackStatus=DELETE_IN_PROGRESS
INFO[0032] Waiting for your cluster resources to be created...
INFO[0032] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
INFO[0093] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
INFO[0154] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
VPC created: vpc-048e16a7e67c52dc7
Security Group created: sg-0183989624bf595be
Subnet created: subnet-0d20dcf2079c259ee
Subnet created: subnet-06b9a5a8fc4988c04
Cluster creation succeeded.

$ aws ecs describe-clusters --clusters tagging-bug --region us-west-1
{
    "clusters": [
        {
            "clusterArn": "arn:aws:ecs:us-west-1:REDACTED:cluster/tagging-bug",
            "clusterName": "tagging-bug",
            "status": "ACTIVE",
            "registeredContainerInstancesCount": 1,
            "runningTasksCount": 0,
            "pendingTasksCount": 0,
            "activeServicesCount": 0,
            "statistics": [],
            "tags": []
        }
    ],
    "failures": []
}

PettitWesley · 2019-03-09T19:26:12Z

Tags are correctly being added:

$ aws resourcegroupstaggingapi  get-resources --tag-filters Key=ugh,Values=ithinkitsabug --region us-west-1 | jq '.ResourceTagMappingList[] | {Thing_with_tags: .ResourceARN }'
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:instance/i-049aafa90a0389fda"
}
{
  "Thing_with_tags": "arn:aws:ecs:us-west-1:REDACTED:container-instance/tagging-bug/c1df58ccb804406e83614d7bb4f894bd"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:instance/i-0a388bb6a6cc728a6"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:security-group/sg-0183989624bf595be"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:vpc/vpc-048e16a7e67c52dc7"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:route-table/rtb-044279977577a73f3"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:instance/i-049da5eee133d7d63"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:internet-gateway/igw-03d15a7be1a8c8493"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:instance/i-0cb8d2aafaacace53"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:instance/i-0d734ed33684f3599"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:subnet/subnet-0d20dcf2079c259ee"
}
{
  "Thing_with_tags": "arn:aws:cloudformation:us-west-1:REDACTED:stack/amazon-ecs-cli-setup-tagging-bug/b0b70f40-4298-11e9-82f0-029c5033407e"
}
{
  "Thing_with_tags": "arn:aws:ecs:us-west-1:REDACTED:cluster/default2"
}
{
  "Thing_with_tags": "arn:aws:ecs:us-west-1:REDACTED:cluster/tagging-bug"
}
{
  "Thing_with_tags": "arn:aws:ecs:us-west-1:REDACTED:cluster/workaround"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:subnet/subnet-06b9a5a8fc4988c04"
}

rroller · 2019-03-09T19:43:06Z

VERSION

@@ -1 +1 @@
-1.12.1
+1.13.0


Should this be 1.13.1 since 1.13.0 was already released?

Yeah, this is happening because dev is one commit behind master, and my code is off of master. So this PR contains the 1.13.0 version bump. I need to put up a separate pull request to catch dev up with master.

Once this fix is approved, we'll release it as 1.13.1.

rroller · 2019-03-09T19:43:26Z

ecs-cli/modules/version/version.go

@@ -22,7 +22,7 @@ package version
 // repository. Only the 'Version' const should change in checked-in source code

 // Version is the version of the ECS CLI
-const Version = "1.12.1"
+const Version = "1.13.0"


Same comment, should this be 1.13.1?

PettitWesley · 2019-03-09T20:09:39Z

Verified that the case with container instance tagging disabled still works:

$ ecs-cli up --capability-iam --cluster longarnsdisabled --region us-west-1 --tags longarns=disabled --keypair SFOPair --force
WARN[0000] Disabling container instance tagging because containerInstanceLongArnFormat is not enabled for your identity, arn:aws:iam::REDACTED:user/Wesley3.0. You can use the PutAccountSettingDefault API to change your account default.
INFO[0000] Defaulting instance type to t2.micro
INFO[0000] Using recommended Amazon Linux 2 AMI with ECS Agent 1.26.0 and Docker version 18.06.1-ce
INFO[0001] Created cluster                               cluster=longarnsdisabled region=us-west-1
INFO[0001] Waiting for your cluster resources to be created...
INFO[0002] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
INFO[0062] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
INFO[0123] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
VPC created: vpc-0117402930a947ab3
Security Group created: sg-0a3f9eb1b393c8525
Subnet created: subnet-089e1e1068d3b022e
Subnet created: subnet-052c2d2028deac81f
Cluster creation succeeded.

$ aws ecs describe-clusters --clusters longarnsdisabled --region us-west-1 | jq ".clusters[] | {Cluster: .clusterName, RegisteredInstances: .registeredContainerInstancesCount}"
{
  "Cluster": "longarnsdisabled",
  "RegisteredInstances": 1
}

$ aws resourcegroupstaggingapi  get-resources --tag-filters Key=longarns,Values=disabled --region us-west-1 | jq '.ResourceTagMappingList[] | {Thing_with_tags: .ResourceARN }'
{
  "Thing_with_tags": "arn:aws:cloudformation:us-west-1:REDACTED:stack/amazon-ecs-cli-setup-longarnsdisabled/ca3db6f0-42a5-11e9-9753-500cf8eeb88d"
}
{
  "Thing_with_tags": "arn:aws:ecs:us-west-1:REDACTED:cluster/longarnsdisabled"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:subnet/subnet-052c2d2028deac81f"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:internet-gateway/igw-0604216247edfec22"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:vpc/vpc-0117402930a947ab3"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:instance/i-056f54885df9eae90"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:subnet/subnet-089e1e1068d3b022e"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:route-table/rtb-02363f99ca636a3fd"
}
{
  "Thing_with_tags": "arn:aws:ec2:us-west-1:REDACTED:security-group/sg-0a3f9eb1b393c8525"
}

PettitWesley · 2019-03-09T20:16:13Z

And finally, verified that cluster up still works without tagging:

$ ecs-cli up --capability-iam --cluster no-tags --region us-west-1 --keypair SFOPair --force
INFO[0000] Defaulting instance type to t2.micro
INFO[0000] Using recommended Amazon Linux 2 AMI with ECS Agent 1.26.0 and Docker version 18.06.1-ce
INFO[0000] Created cluster                               cluster=no-tags region=us-west-1
INFO[0001] Waiting for your cluster resources to be created...
INFO[0001] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
INFO[0062] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
INFO[0123] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
VPC created: vpc-05cec00c412a93011
Security Group created: sg-0b9adbf8b40f66111
Subnet created: subnet-014f23d815d6f16ab
Subnet created: subnet-068b5c3369772322e
Cluster creation succeeded.

 $ aws ecs describe-clusters --clusters no-tags --region us-west-1 | jq ".clusters[] | {Cluster: .clusterName, RegisteredInstances: .registeredContainerInstancesCount}"
{
  "Cluster": "no-tags",
  "RegisteredInstances": 1
}

ecs-cli/modules/clients/aws/cloudformation/cluster_template.go

PettitWesley · 2019-03-11T16:55:55Z

Testing with the new CFN changes:

$ ecs-cli up --capability-iam --cluster tagging-bug --region us-west-1 --tags ugh=ithinkitsabug --keypair SFOPair --force
WARN[0000] Enabling container instance tagging because containerInstanceLongArnFormat is enabled for your identity, arn:aws:iam::REDACTED:user/Wesley3.0. If this is not your account default setting, your instances will fail to join your cluster. You can use the PutAccountSettingDefault API to change your account default.
INFO[0000] Defaulting instance type to t2.micro
INFO[0000] Using recommended Amazon Linux 2 AMI with ECS Agent 1.26.0 and Docker version 18.06.1-ce
INFO[0001] Created cluster                               cluster=tagging-bug region=us-west-1
INFO[0001] Waiting for your cluster resources to be created...
INFO[0001] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
INFO[0062] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
INFO[0123] Cloudformation stack status                   stackStatus=CREATE_IN_PROGRESS
VPC created: vpc-083e703c402e8dac3
Security Group created: sg-01f56ae1c92f108c3
Subnet created: subnet-00f18eeacbcda6a52
Subnet created: subnet-0d4c4ed77ea3af6d0
Cluster creation succeeded.

$ aws ecs describe-clusters --clusters tagging-bug --region us-west-1 | jq ".clusters[] | {Cluster: .clusterName, RegisteredInstances: .registeredContainerInstancesCount}"
{
  "Cluster": "tagging-bug",
  "RegisteredInstances": 1
}

README.md

PettitWesley · 2019-03-11T19:41:38Z

Unfortunately, inheriting tags from EC2 is not robust. There is a race condition; the tags may not have propagated from autoscaling to the EC2 instance when the agent runs:

2019-03-11T17:57:56Z [INFO] Retrieved Tags from EC2 DescribeTags API:
[]

Therefore, we're gonna have to scrap this and go with the other method of adding tags to container instances: specifying the tag values in the user data directly.

PettitWesley · 2019-03-11T23:02:36Z

Closing in favor of #747

PettitWesley requested review from SoManyHs, allisaurus and efekarakus March 9, 2019 18:40

rroller reviewed Mar 9, 2019

View reviewed changes

PettitWesley mentioned this pull request Mar 9, 2019

When using the latest ecs-cli and the new tags option, container instances will not launch. #744

Closed

efekarakus approved these changes Mar 11, 2019

View reviewed changes

Bump version to 1.13.0

ef61921

PettitWesley force-pushed the fix-container-instance-tagging branch from d539ce8 to 5b6b4b7 Compare March 11, 2019 16:11

allisaurus reviewed Mar 11, 2019

View reviewed changes

ecs-cli/modules/clients/aws/cloudformation/cluster_template.go Outdated Show resolved Hide resolved

PettitWesley force-pushed the fix-container-instance-tagging branch from 0e50646 to f04fc41 Compare March 11, 2019 16:54

PettitWesley force-pushed the fix-container-instance-tagging branch from f04fc41 to 36db33f Compare March 11, 2019 17:39

allisaurus reviewed Mar 11, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

Add ec2:DescribeTags to ec2 instance role. Fixes aws#744

c0c8cbc

PettitWesley force-pushed the fix-container-instance-tagging branch from 36db33f to c0c8cbc Compare March 11, 2019 17:45

PettitWesley mentioned this pull request Mar 11, 2019

Agent fails to propagate tags from EC2 Instance aws/amazon-ecs-agent#1935

Closed

PettitWesley force-pushed the dev branch from ef61921 to 2d98b5e Compare March 11, 2019 22:04

PettitWesley closed this Mar 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix container instance tagging #745

Fix container instance tagging #745

PettitWesley commented Mar 9, 2019 •

edited

Loading

PettitWesley commented Mar 9, 2019 •

edited

Loading

PettitWesley commented Mar 9, 2019 •

edited

Loading

rroller Mar 9, 2019

PettitWesley Mar 9, 2019

PettitWesley Mar 9, 2019

rroller Mar 9, 2019

PettitWesley commented Mar 9, 2019

PettitWesley commented Mar 9, 2019

PettitWesley commented Mar 11, 2019

PettitWesley commented Mar 11, 2019

PettitWesley commented Mar 11, 2019

		@@ -1 +1 @@
		1.12.1
		1.13.0

Fix container instance tagging #745

Fix container instance tagging #745

Conversation

PettitWesley commented Mar 9, 2019 • edited Loading

PettitWesley commented Mar 9, 2019 • edited Loading

PettitWesley commented Mar 9, 2019 • edited Loading

rroller Mar 9, 2019

Choose a reason for hiding this comment

PettitWesley Mar 9, 2019

Choose a reason for hiding this comment

PettitWesley Mar 9, 2019

Choose a reason for hiding this comment

rroller Mar 9, 2019

Choose a reason for hiding this comment

PettitWesley commented Mar 9, 2019

PettitWesley commented Mar 9, 2019

PettitWesley commented Mar 11, 2019

PettitWesley commented Mar 11, 2019

PettitWesley commented Mar 11, 2019

PettitWesley commented Mar 9, 2019 •

edited

Loading

PettitWesley commented Mar 9, 2019 •

edited

Loading

PettitWesley commented Mar 9, 2019 •

edited

Loading