Skip to content

Branch remove token #1711

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 4, 2018
Merged

Branch remove token #1711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
200de25
RCI idempotency changes
tommyhahn Dec 3, 2018
0fe43ba
app/agent.go: remove token from state file
tommyhahn Dec 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## 1.23.0-dev
* Bug - Fixed a bug where agent can register container instance back to back and gets
assigned two container instance ARNs [#1711](https://github.com/aws/amazon-ecs-agent/pull/1711)

## 1.22.0
* Feature - Add support for ECS Secrets integrating with AWS Systems Manager Parameter Store
* Feature - Support for `--pid`, `--ipc` Docker run flags. [#1584](https://github.com/aws/amazon-ecs-agent/pull/1584)
Expand Down
10 changes: 6 additions & 4 deletions agent/api/ecsclient/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (client *APIECSClient) CreateCluster(clusterName string) (string, error) {
// instance ARN allows a container instance to update its registered
// resources.
func (client *APIECSClient) RegisterContainerInstance(containerInstanceArn string,
attributes []*ecs.Attribute, tags []*ecs.Tag) (string, string, error) {
attributes []*ecs.Attribute, tags []*ecs.Tag, registrationToken string) (string, string, error) {
clusterRef := client.config.Cluster
// If our clusterRef is empty, we should try to create the default
if clusterRef == "" {
Expand All @@ -120,7 +120,7 @@ func (client *APIECSClient) RegisterContainerInstance(containerInstanceArn strin
}()
// Attempt to register without checking existence of the cluster so we don't require
// excess permissions in the case where the cluster already exists and is active
containerInstanceArn, availabilityzone, err := client.registerContainerInstance(clusterRef, containerInstanceArn, attributes, tags)
containerInstanceArn, availabilityzone, err := client.registerContainerInstance(clusterRef, containerInstanceArn, attributes, tags, registrationToken)
if err == nil {
return containerInstanceArn, availabilityzone, nil
}
Expand All @@ -131,11 +131,11 @@ func (client *APIECSClient) RegisterContainerInstance(containerInstanceArn strin
return "", "", err
}
}
return client.registerContainerInstance(clusterRef, containerInstanceArn, attributes, tags)
return client.registerContainerInstance(clusterRef, containerInstanceArn, attributes, tags, registrationToken)
}

func (client *APIECSClient) registerContainerInstance(clusterRef string, containerInstanceArn string,
attributes []*ecs.Attribute, tags []*ecs.Tag) (string, string, error) {
attributes []*ecs.Attribute, tags []*ecs.Tag, registrationToken string) (string, string, error) {
registerRequest := ecs.RegisterContainerInstanceInput{Cluster: &clusterRef}
var registrationAttributes []*ecs.Attribute
if containerInstanceArn != "" {
Expand Down Expand Up @@ -169,6 +169,8 @@ func (client *APIECSClient) registerContainerInstance(clusterRef string, contain
}

registerRequest.TotalResources = resources

registerRequest.ClientToken = &registrationToken
resp, err := client.standardClient.RegisterContainerInstance(&registerRequest)
if err != nil {
seelog.Errorf("Unable to register as a container instance with ECS: %v", err)
Expand Down
25 changes: 13 additions & 12 deletions agent/api/ecsclient/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const (
configuredCluster = "mycluster"
iid = "instanceIdentityDocument"
iidSignature = "signature"
registrationToken = "clientToken"
)

var (
Expand Down Expand Up @@ -343,6 +344,7 @@ func TestReRegisterContainerInstance(t *testing.T) {
mc.EXPECT().RegisterContainerInstance(gomock.Any()).Do(func(req *ecs.RegisterContainerInstanceInput) {
assert.Equal(t, "arn:test", *req.ContainerInstanceArn, "Wrong container instance ARN")
assert.Equal(t, configuredCluster, *req.Cluster, "Wrong cluster")
assert.Equal(t, registrationToken, *req.ClientToken, "Wrong client token")
assert.Equal(t, iid, *req.InstanceIdentityDocument, "Wrong IID")
assert.Equal(t, iidSignature, *req.InstanceIdentityDocumentSignature, "Wrong IID sig")
assert.Equal(t, 4, len(req.TotalResources), "Wrong length of TotalResources")
Expand Down Expand Up @@ -376,13 +378,10 @@ func TestReRegisterContainerInstance(t *testing.T) {
nil),
)

arn, availabilityzone, err := client.RegisterContainerInstance("arn:test", capabilities, containerInstanceTags)
if err != nil {
t.Errorf("Should not be an error: %v", err)
}
if arn != "registerArn" {
t.Errorf("Wrong arn: %v", arn)
}
arn, availabilityzone, err := client.RegisterContainerInstance("arn:test", capabilities, containerInstanceTags, registrationToken)

assert.NoError(t, err)
assert.Equal(t, "registerArn", arn)
assert.Equal(t, "us-west-2b", availabilityzone, "availabilityZone is incorrect")
}

Expand Down Expand Up @@ -410,6 +409,7 @@ func TestRegisterContainerInstance(t *testing.T) {
mc.EXPECT().RegisterContainerInstance(gomock.Any()).Do(func(req *ecs.RegisterContainerInstanceInput) {
assert.Nil(t, req.ContainerInstanceArn)
assert.Equal(t, configuredCluster, *req.Cluster, "Wrong cluster")
assert.Equal(t, registrationToken, *req.ClientToken, "Wrong client token")
assert.Equal(t, iid, *req.InstanceIdentityDocument, "Wrong IID")
assert.Equal(t, iidSignature, *req.InstanceIdentityDocumentSignature, "Wrong IID sig")
assert.Equal(t, 4, len(req.TotalResources), "Wrong length of TotalResources")
Expand Down Expand Up @@ -438,7 +438,7 @@ func TestRegisterContainerInstance(t *testing.T) {
nil),
)

arn, availabilityzone, err := client.RegisterContainerInstance("", capabilities, containerInstanceTags)
arn, availabilityzone, err := client.RegisterContainerInstance("", capabilities, containerInstanceTags, registrationToken)
assert.NoError(t, err)
assert.Equal(t, "registerArn", arn)
assert.Equal(t, "us-west-2b", availabilityzone)
Expand Down Expand Up @@ -472,6 +472,7 @@ func TestRegisterContainerInstanceNoIID(t *testing.T) {
mc.EXPECT().RegisterContainerInstance(gomock.Any()).Do(func(req *ecs.RegisterContainerInstanceInput) {
assert.Nil(t, req.ContainerInstanceArn)
assert.Equal(t, configuredCluster, *req.Cluster, "Wrong cluster")
assert.Equal(t, registrationToken, *req.ClientToken, "Wrong client token")
assert.Equal(t, "", *req.InstanceIdentityDocument, "Wrong IID")
assert.Equal(t, "", *req.InstanceIdentityDocumentSignature, "Wrong IID sig")
assert.Equal(t, 4, len(req.TotalResources), "Wrong length of TotalResources")
Expand Down Expand Up @@ -500,7 +501,7 @@ func TestRegisterContainerInstanceNoIID(t *testing.T) {
nil),
)

arn, availabilityzone, err := client.RegisterContainerInstance("", capabilities, containerInstanceTags)
arn, availabilityzone, err := client.RegisterContainerInstance("", capabilities, containerInstanceTags, registrationToken)
assert.NoError(t, err)
assert.Equal(t, "registerArn", arn)
assert.Equal(t, "us-west-2b", availabilityzone)
Expand All @@ -527,7 +528,7 @@ func TestRegisterContainerInstanceWithNegativeResource(t *testing.T) {
mockEC2Metadata.EXPECT().GetDynamicData(ec2.InstanceIdentityDocumentResource).Return("instanceIdentityDocument", nil),
mockEC2Metadata.EXPECT().GetDynamicData(ec2.InstanceIdentityDocumentSignatureResource).Return("signature", nil),
)
_, _, err := client.RegisterContainerInstance("", nil, nil)
_, _, err := client.RegisterContainerInstance("", nil, nil, "")
assert.Error(t, err, "Register resource with negative value should cause registration fail")
}

Expand Down Expand Up @@ -557,7 +558,7 @@ func TestRegisterContainerInstanceWithEmptyTags(t *testing.T) {
nil),
)

_, _, err := client.RegisterContainerInstance("", nil, make([]*ecs.Tag, 0))
_, _, err := client.RegisterContainerInstance("", nil, make([]*ecs.Tag, 0), "")
assert.NoError(t, err)
}

Expand Down Expand Up @@ -631,7 +632,7 @@ func TestRegisterBlankCluster(t *testing.T) {
nil),
)

arn, availabilityzone, err := client.RegisterContainerInstance("", nil, nil)
arn, availabilityzone, err := client.RegisterContainerInstance("", nil, nil, "")
if err != nil {
t.Errorf("Should not be an error: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion agent/api/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type ECSClient interface {
// instance ARN allows a container instance to update its registered
// resources.
RegisterContainerInstance(existingContainerInstanceArn string,
attributes []*ecs.Attribute, tags []*ecs.Tag) (string, string, error)
attributes []*ecs.Attribute, tags []*ecs.Tag, registrationToken string) (string, string, error)
// SubmitTaskStateChange sends a state change and returns an error
// indicating if it was submitted
SubmitTaskStateChange(change TaskStateChange) error
Expand Down
8 changes: 4 additions & 4 deletions agent/api/mocks/api_mocks.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions agent/app/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import (
aws_credentials "github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/aws/defaults"
"github.com/cihub/seelog"
"github.com/pborman/uuid"
)

const (
Expand Down Expand Up @@ -481,11 +482,11 @@ func (agent *ecsAgent) registerContainerInstance(

if agent.containerInstanceARN != "" {
seelog.Infof("Restored from checkpoint file. I am running as '%s' in cluster '%s'", agent.containerInstanceARN, agent.cfg.Cluster)
return agent.reregisterContainerInstance(client, capabilities, tags)
return agent.reregisterContainerInstance(client, capabilities, tags, uuid.New())
}

seelog.Info("Registering Instance with ECS")
containerInstanceArn, availabilityZone, err := client.RegisterContainerInstance("", capabilities, tags)
containerInstanceArn, availabilityZone, err := client.RegisterContainerInstance("", capabilities, tags, uuid.New())
if err != nil {
seelog.Errorf("Error registering: %v", err)
if retriable, ok := err.(apierrors.Retriable); ok && !retriable.Retry() {
Expand Down Expand Up @@ -513,8 +514,8 @@ func (agent *ecsAgent) registerContainerInstance(
// registered with ECS. This is for cases where the ECS Agent is being restored
// from a check point.
func (agent *ecsAgent) reregisterContainerInstance(client api.ECSClient,
capabilities []*ecs.Attribute, tags []*ecs.Tag) error {
_, _, err := client.RegisterContainerInstance(agent.containerInstanceARN, capabilities, tags)
capabilities []*ecs.Attribute, tags []*ecs.Tag, registrationToken string) error {
_, _, err := client.RegisterContainerInstance(agent.containerInstanceARN, capabilities, tags, registrationToken)

if err == nil {
return nil
Expand Down
Loading