Skip to content

Commit

Permalink
containermetadata: expose task definition family and revision as meta…
Browse files Browse the repository at this point in the history
…data fields

The task struct already has this information, so it's just a matter
of plumbing it through.

This commit also fixes some typos.
  • Loading branch information
haikuoliu committed Mar 22, 2018
1 parent 3f2fecc commit 6f16e3c
Show file tree
Hide file tree
Showing 20 changed files with 187 additions and 103 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog

## 1.17.3-dev
* Enhancement - Expose task definition family and task definition revision in container metadata file [#1295](https://github.com/aws/amazon-ecs-agent/pull/1295)
* Bug - Fixed a bug where a stale websocket connection could linger [#1310](https://github.com/aws/amazon-ecs-agent/pull/1310)

## 1.17.2
Expand Down
2 changes: 1 addition & 1 deletion agent/containermetadata/generate_mocks.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// Copyright 2017-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License"). You may
// not use this file except in compliance with the License. A copy of the
Expand Down
27 changes: 14 additions & 13 deletions agent/containermetadata/manager.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// Copyright 2017-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License"). You may
// not use this file except in compliance with the License. A copy of the
Expand All @@ -22,6 +22,7 @@ import (
"github.com/aws/amazon-ecs-agent/agent/config"
"github.com/aws/amazon-ecs-agent/agent/utils/ioutilwrapper"
"github.com/aws/amazon-ecs-agent/agent/utils/oswrapper"
"github.com/aws/amazon-ecs-agent/agent/api"

docker "github.com/fsouza/go-dockerclient"
)
Expand All @@ -39,8 +40,8 @@ const (
// operations
type Manager interface {
SetContainerInstanceARN(string)
Create(*docker.Config, *docker.HostConfig, string, string) error
Update(string, string, string) error
Create(*docker.Config, *docker.HostConfig, *api.Task, string) error
Update(string, *api.Task, string) error
Clean(string) error
}

Expand Down Expand Up @@ -86,22 +87,22 @@ func (manager *metadataManager) SetContainerInstanceARN(containerInstanceARN str
// Create creates the metadata file and adds the metadata directory to
// the container's mounted host volumes
// Pointer hostConfig is modified directly so there is risk of concurrency errors.
func (manager *metadataManager) Create(config *docker.Config, hostConfig *docker.HostConfig, taskARN string, containerName string) error {
func (manager *metadataManager) Create(config *docker.Config, hostConfig *docker.HostConfig, task *api.Task, containerName string) error {
// Create task and container directories if they do not yet exist
metadataDirectoryPath, err := getMetadataFilePath(taskARN, containerName, manager.dataDir)
metadataDirectoryPath, err := getMetadataFilePath(task.Arn, containerName, manager.dataDir)
// Stop metadata creation if path is malformed for any reason
if err != nil {
return fmt.Errorf("container metadata create for task %s container %s: %v", taskARN, containerName, err)
return fmt.Errorf("container metadata create for task %s container %s: %v", task.Arn, containerName, err)
}

err = manager.osWrap.MkdirAll(metadataDirectoryPath, os.ModePerm)
if err != nil {
return fmt.Errorf("creating metadata directory for task %s: %v", taskARN, err)
return fmt.Errorf("creating metadata directory for task %s: %v", task.Arn, err)
}

// Acquire the metadata then write it in JSON format to the file
metadata := manager.parseMetadataAtContainerCreate(taskARN, containerName)
err = manager.marshalAndWrite(metadata, taskARN, containerName)
metadata := manager.parseMetadataAtContainerCreate(task, containerName)
err = manager.marshalAndWrite(metadata, task.Arn, containerName)
if err != nil {
return err
}
Expand All @@ -115,7 +116,7 @@ func (manager *metadataManager) Create(config *docker.Config, hostConfig *docker
}

// Update updates the metadata file after container starts and dynamic metadata is available
func (manager *metadataManager) Update(dockerID string, taskARN string, containerName string) error {
func (manager *metadataManager) Update(dockerID string, task *api.Task, containerName string) error {
// Get docker container information through api call
dockerContainer, err := manager.client.InspectContainer(dockerID, inspectContainerTimeout)
if err != nil {
Expand All @@ -124,12 +125,12 @@ func (manager *metadataManager) Update(dockerID string, taskARN string, containe

// Ensure we do not update a container that is invalid or is not running
if dockerContainer == nil || !dockerContainer.State.Running {
return fmt.Errorf("container metadata update for contiainer %s in task %s: container not running or invalid", containerName, taskARN)
return fmt.Errorf("container metadata update for container %s in task %s: container not running or invalid", containerName, task.Arn)
}

// Acquire the metadata then write it in JSON format to the file
metadata := manager.parseMetadata(dockerContainer, taskARN, containerName)
return manager.marshalAndWrite(metadata, taskARN, containerName)
metadata := manager.parseMetadata(dockerContainer, task, containerName)
return manager.marshalAndWrite(metadata, task.Arn, containerName)
}

// Clean removes the metadata files of all containers associated with a task
Expand Down
29 changes: 18 additions & 11 deletions agent/containermetadata/manager_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// +build !integration
// Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// Copyright 2017-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License"). You may
// not use this file except in compliance with the License. A copy of the
Expand All @@ -21,19 +21,22 @@ import (
"github.com/aws/amazon-ecs-agent/agent/containermetadata/mocks"
"github.com/aws/amazon-ecs-agent/agent/utils/ioutilwrapper/mocks"
"github.com/aws/amazon-ecs-agent/agent/utils/oswrapper/mocks"
"github.com/aws/amazon-ecs-agent/agent/api"

docker "github.com/fsouza/go-dockerclient"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
)

const (
containerInstanceARN = "a6348116-0ba6-43b5-87c9-8a7e10294b75"
dockerID = "888888888887"
invalidTaskARN = "invalidARN"
validTaskARN = "arn:aws:ecs:region:account-id:task/task-id"
containerName = "container"
dataDir = "ecs_mockdata"
containerInstanceARN = "a6348116-0ba6-43b5-87c9-8a7e10294b75"
dockerID = "888888888887"
invalidTaskARN = "invalidARN"
validTaskARN = "arn:aws:ecs:region:account-id:task/task-id"
taskDefinitionFamily = "taskdefinitionfamily"
taskDefinitionRevision = "8"
containerName = "container"
dataDir = "ecs_mockdata"
)

func managerSetup(t *testing.T) (*mock_containermetadata.MockDockerMetadataClient, *mock_ioutilwrapper.MockIOUtil, *mock_oswrapper.MockOS, *mock_oswrapper.MockFile, func()) {
Expand Down Expand Up @@ -63,10 +66,11 @@ func TestCreateMalformedFilepath(t *testing.T) {
defer done()

mockTaskARN := invalidTaskARN
mockTask := &api.Task{Arn: mockTaskARN}
mockContainerName := containerName

newManager := &metadataManager{}
err := newManager.Create(nil, nil, mockTaskARN, mockContainerName)
err := newManager.Create(nil, nil, mockTask, mockContainerName)
assert.Error(t, err)
}

Expand All @@ -76,6 +80,7 @@ func TestCreateMkdirAllFail(t *testing.T) {
defer done()

mockTaskARN := validTaskARN
mockTask := &api.Task{Arn: mockTaskARN}
mockContainerName := containerName

gomock.InOrder(
Expand All @@ -85,7 +90,7 @@ func TestCreateMkdirAllFail(t *testing.T) {
newManager := &metadataManager{
osWrap: mockOS,
}
err := newManager.Create(nil, nil, mockTaskARN, mockContainerName)
err := newManager.Create(nil, nil, mockTask, mockContainerName)
assert.Error(t, err)
}

Expand All @@ -96,14 +101,15 @@ func TestUpdateInspectFail(t *testing.T) {

mockDockerID := dockerID
mockTaskARN := validTaskARN
mockTask := &api.Task{Arn: mockTaskARN}
mockContainerName := containerName

newManager := &metadataManager{
client: mockClient,
}

mockClient.EXPECT().InspectContainer(mockDockerID, inspectContainerTimeout).Return(nil, errors.New("Inspect fail"))
err := newManager.Update(mockDockerID, mockTaskARN, mockContainerName)
err := newManager.Update(mockDockerID, mockTask, mockContainerName)

assert.Error(t, err, "Expected inspect error to result in update fail")
}
Expand All @@ -115,6 +121,7 @@ func TestUpdateNotRunningFail(t *testing.T) {

mockDockerID := dockerID
mockTaskARN := validTaskARN
mockTask := &api.Task{Arn: mockTaskARN}
mockContainerName := containerName
mockState := docker.State{
Running: false,
Expand All @@ -128,7 +135,7 @@ func TestUpdateNotRunningFail(t *testing.T) {
}

mockClient.EXPECT().InspectContainer(mockDockerID, inspectContainerTimeout).Return(mockContainer, nil)
err := newManager.Update(mockDockerID, mockTaskARN, mockContainerName)
err := newManager.Update(mockDockerID, mockTask, mockContainerName)
assert.Error(t, err)
}

Expand Down
10 changes: 7 additions & 3 deletions agent/containermetadata/manager_unix_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// +build !integration,!windows
// Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// Copyright 2017-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License"). You may
// not use this file except in compliance with the License. A copy of the
Expand All @@ -17,6 +17,8 @@ package containermetadata
import (
"testing"

"github.com/aws/amazon-ecs-agent/agent/api"

docker "github.com/fsouza/go-dockerclient"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
Expand All @@ -28,6 +30,7 @@ func TestCreate(t *testing.T) {
defer done()

mockTaskARN := validTaskARN
mockTask := &api.Task{Arn: mockTaskARN}
mockContainerName := containerName
mockConfig := &docker.Config{Env: make([]string, 0)}
mockHostConfig := &docker.HostConfig{Binds: make([]string, 0)}
Expand All @@ -46,7 +49,7 @@ func TestCreate(t *testing.T) {
osWrap: mockOS,
ioutilWrap: mockIOUtil,
}
err := newManager.Create(mockConfig, mockHostConfig, mockTaskARN, mockContainerName)
err := newManager.Create(mockConfig, mockHostConfig, mockTask, mockContainerName)

assert.NoError(t, err)
assert.Equal(t, 1, len(mockConfig.Env), "Unexpected number of environment variables in config")
Expand All @@ -60,6 +63,7 @@ func TestUpdate(t *testing.T) {

mockDockerID := dockerID
mockTaskARN := validTaskARN
mockTask := &api.Task{Arn: mockTaskARN}
mockContainerName := containerName
mockState := docker.State{
Running: true,
Expand Down Expand Up @@ -91,7 +95,7 @@ func TestUpdate(t *testing.T) {
mockOS.EXPECT().Rename(gomock.Any(), gomock.Any()).Return(nil),
mockFile.EXPECT().Close().Return(nil),
)
err := newManager.Update(mockDockerID, mockTaskARN, mockContainerName)
err := newManager.Update(mockDockerID, mockTask, mockContainerName)

assert.NoError(t, err)
}
10 changes: 7 additions & 3 deletions agent/containermetadata/manager_windows_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// +build !integration, windows
// Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// Copyright 2017-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License"). You may
// not use this file except in compliance with the License. A copy of the
Expand All @@ -17,6 +17,8 @@ package containermetadata
import (
"testing"

"github.com/aws/amazon-ecs-agent/agent/api"

docker "github.com/fsouza/go-dockerclient"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
Expand All @@ -28,6 +30,7 @@ func TestCreate(t *testing.T) {
defer done()

mockTaskARN := validTaskARN
mockTask := &api.Task{Arn: mockTaskARN}
mockContainerName := containerName
mockConfig := &docker.Config{Env: make([]string, 0)}
mockHostConfig := &docker.HostConfig{Binds: make([]string, 0)}
Expand All @@ -43,7 +46,7 @@ func TestCreate(t *testing.T) {
newManager := &metadataManager{
osWrap: mockOS,
}
err := newManager.Create(mockConfig, mockHostConfig, mockTaskARN, mockContainerName)
err := newManager.Create(mockConfig, mockHostConfig, mockTask, mockContainerName)

assert.NoError(t, err)
assert.Equal(t, 1, len(mockConfig.Env), "Unexpected number of environment variables in config")
Expand All @@ -57,6 +60,7 @@ func TestUpdate(t *testing.T) {

mockDockerID := dockerID
mockTaskARN := validTaskARN
mockTask := &api.Task{Arn: mockTaskARN}
mockContainerName := containerName
mockState := docker.State{
Running: true,
Expand Down Expand Up @@ -85,7 +89,7 @@ func TestUpdate(t *testing.T) {
mockFile.EXPECT().Sync().Return(nil),
mockFile.EXPECT().Close().Return(nil),
)
err := newManager.Update(mockDockerID, mockTaskARN, mockContainerName)
err := newManager.Update(mockDockerID, mockTask, mockContainerName)

assert.NoError(t, err)
}
6 changes: 4 additions & 2 deletions agent/containermetadata/mocks/containermetadata_mocks.go

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

22 changes: 16 additions & 6 deletions agent/containermetadata/parse_metadata.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// Copyright 2017-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License"). You may
// not use this file except in compliance with the License. A copy of the
Expand Down Expand Up @@ -27,10 +27,15 @@ import (
// available prior to container creation
// Since we accept incomplete metadata fields, we should not return
// errors here and handle them at this or the above stage.
func (manager *metadataManager) parseMetadataAtContainerCreate(taskARN string, containerName string) Metadata {
func (manager *metadataManager) parseMetadataAtContainerCreate(task *api.Task, containerName string) Metadata {
return Metadata{
cluster: manager.cluster,
taskMetadata: TaskMetadata{containerName: containerName, taskARN: taskARN},
taskMetadata: TaskMetadata{
containerName: containerName,
taskARN: task.Arn,
taskDefinitionFamily: task.Family,
taskDefinitionRevision: task.Version,
},
containerInstanceARN: manager.containerInstanceARN,
metadataStatus: MetadataInitial,
}
Expand All @@ -40,11 +45,16 @@ func (manager *metadataManager) parseMetadataAtContainerCreate(taskARN string, c
// configuration and data then packages it for JSON Marshaling
// Since we accept incomplete metadata fields, we should not return
// errors here and handle them at this or the above stage.
func (manager *metadataManager) parseMetadata(dockerContainer *docker.Container, taskARN string, containerName string) Metadata {
dockerMD := parseDockerContainerMetadata(taskARN, containerName, dockerContainer)
func (manager *metadataManager) parseMetadata(dockerContainer *docker.Container, task *api.Task, containerName string) Metadata {
dockerMD := parseDockerContainerMetadata(task.Arn, containerName, dockerContainer)
return Metadata{
cluster: manager.cluster,
taskMetadata: TaskMetadata{containerName: containerName, taskARN: taskARN},
taskMetadata: TaskMetadata{
containerName: containerName,
taskARN: task.Arn,
taskDefinitionFamily: task.Family,
taskDefinitionRevision: task.Version,
},
dockerContainerMetadata: dockerMD,
containerInstanceARN: manager.containerInstanceARN,
metadataStatus: MetadataReady,
Expand Down
Loading

0 comments on commit 6f16e3c

Please sign in to comment.