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

handlers: Add Volumes to v1 and v2 metadata #1531

Merged
merged 5 commits into from
Aug 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog

## 1.20.2-dev
* Enhancement - Added Volumes metadata as part of v1 and v2 metadata endpoints [#1531](https://github.com/aws/amazon-ecs-agent/pull/1531)
* Bug - Fixed a bug where unrecognized task cannot be stopped [#1467](https://github.com/aws/amazon-ecs-agent/pull/1467)
* Enhancement - Added ECS config field to provide optionally comparing shared volumes' full details (driver options and labels), or names only [#1519](https://github.com/aws/amazon-ecs-agent/pull/1519)

Expand Down
19 changes: 19 additions & 0 deletions agent/api/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,9 @@ type Container struct {
// KnownPortBindingsUnsafe is an array of port bindings for the container.
KnownPortBindingsUnsafe []PortBinding `json:"KnownPortBindings"`

// VolumesUnsafe is an array of volume mounts in the container.
VolumesUnsafe []docker.Mount `json:"-"`

Choose a reason for hiding this comment

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

Please update this unit test to cover this:

func TestSynchronizeContainerStatus(t *testing.T) {


// SteadyStateStatusUnsafe specifies the steady state status for the container
// If uninitialized, it's assumed to be set to 'ContainerRunning'. Even though
// it's not only supposed to be set when the container is being created, it's
Expand Down Expand Up @@ -535,6 +538,22 @@ func (c *Container) GetKnownPortBindings() []PortBinding {
return c.KnownPortBindingsUnsafe
}

// SetVolumes sets the volumes mounted in a container
func (c *Container) SetVolumes(volumes []docker.Mount) {
c.lock.Lock()
defer c.lock.Unlock()

c.VolumesUnsafe = volumes
}

// GetVolumes returns the volumes mounted in a container
func (c *Container) GetVolumes() []docker.Mount {
c.lock.RLock()
defer c.lock.RUnlock()

return c.VolumesUnsafe
}

// HealthStatusShouldBeReported returns true if the health check is defined in
// the task definition
func (c *Container) HealthStatusShouldBeReported() bool {
Expand Down
8 changes: 6 additions & 2 deletions agent/engine/docker_task_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,8 +337,12 @@ func updateContainerMetadata(metadata *dockerapi.DockerContainerMetadata, contai
}

// Update volume for empty volume container
if metadata.Volumes != nil && container.IsInternal() {
task.UpdateMountPoints(container, metadata.Volumes)
if metadata.Volumes != nil {
if container.IsInternal() {
task.UpdateMountPoints(container, metadata.Volumes)
} else {
container.SetVolumes(metadata.Volumes)
}
}

// Set Exitcode if it's not set
Expand Down
9 changes: 9 additions & 0 deletions agent/engine/docker_task_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1935,19 +1935,28 @@ func TestSynchronizeContainerStatus(t *testing.T) {
labels := map[string]string{
"name": "metadata",
}
volumes := []docker.Mount{
{
Name: "volume",
Source: "/src/vol",
Destination: "/vol",
},
}
created := time.Now()
gomock.InOrder(
client.EXPECT().DescribeContainer(gomock.Any(), dockerID).Return(apicontainerstatus.ContainerRunning,
dockerapi.DockerContainerMetadata{
Labels: labels,
DockerID: dockerID,
CreatedAt: created,
Volumes: volumes,
}),
imageManager.EXPECT().RecordContainerReference(dockerContainer.Container),
)
taskEngine.(*DockerTaskEngine).synchronizeContainerStatus(dockerContainer, nil)
assert.Equal(t, created, dockerContainer.Container.GetCreatedAt())
assert.Equal(t, labels, dockerContainer.Container.GetLabels())
assert.Equal(t, volumes, dockerContainer.Container.GetVolumes())
}

// TestHandleDockerHealthEvent tests the docker health event will only cause the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@
"awslogs-region":"$$$TEST_REGION$$$",
"awslogs-stream-prefix":"ecs-functional-tests-agent-introspection-validator"
}
}
}]
},
"mountPoints": [
{
"sourceVolume": "task-local",
"containerPath": "/ecs/"
}
]
}],
"volumes":[
{
"name": "task-local",
"dockerVolumeConfiguration" :{
"scope": "task",
"driver": "local"
}
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@
"awslogs-region":"$$$TEST_REGION$$$",
"awslogs-stream-prefix":"ecs-functional-tests-taskmetadata-validator"
}
}
}]
},
"mountPoints": [
{
"sourceVolume": "shared-local",
"containerPath": "/ecs/"
}
]
}],
"volumes":[
{
"name": "shared-local",
"dockerVolumeConfiguration" :{
"scope": "shared",
"autoprovision":true,
"driver": "local"
}
}
]
}
6 changes: 2 additions & 4 deletions agent/functional_tests/tests/functionaltests_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -625,9 +625,7 @@ func TestAgentIntrospectionValidator(t *testing.T) {
},
})
defer agent.Cleanup()
// The Agent version was 1.14.2 when we added changes to agent introspection
// endpoint feature for the last time.
agent.RequireVersion(">1.14.1")
agent.RequireVersion(">1.20.1")

tdOverrides := make(map[string]string)
tdOverrides["$$$TEST_REGION$$$"] = *ECS.Config.Region
Expand Down Expand Up @@ -663,7 +661,7 @@ func TestTaskMetadataValidator(t *testing.T) {
},
})
defer agent.Cleanup()
agent.RequireVersion(">1.16.2")
agent.RequireVersion(">1.20.1")

tdOverrides := make(map[string]string)
tdOverrides["$$$TEST_REGION$$$"] = *ECS.Config.Region
Expand Down
28 changes: 28 additions & 0 deletions agent/handlers/v1/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ type ContainerResponse struct {
Name string `json:"Name"`
Ports []PortResponse `json:"Ports,omitempty"`
Networks []containermetadata.Network `json:"Networks,omitempty"`
Volumes []VolumeResponse `json:"Volumes,omitempty"`
}

// VolumeResponse is the schema for the volume response JSON object
type VolumeResponse struct {
DockerName string `json:"DockerName,omitempty"`
Source string `json:"Source,omitempty"`
Destination string `json:"Destination,omitempty"`
}

// PortResponse defines the schema for portmapping response JSON
Expand Down Expand Up @@ -101,6 +109,7 @@ func NewContainerResponse(dockerContainer *apicontainer.DockerContainer, eni *ap
}

resp.Ports = NewPortBindingsResponse(dockerContainer, eni)
resp.Volumes = NewVolumesResponse(dockerContainer)

if eni != nil {
resp.Networks = []containermetadata.Network{
Expand Down Expand Up @@ -144,6 +153,25 @@ func NewPortBindingsResponse(dockerContainer *apicontainer.DockerContainer, eni
return resp
}

// NewVolumesResponse creates VolumeResponse for a container
func NewVolumesResponse(dockerContainer *apicontainer.DockerContainer) []VolumeResponse {
container := dockerContainer.Container
var resp []VolumeResponse

volumes := container.GetVolumes()

for _, volume := range volumes {
volResp := VolumeResponse{
DockerName: volume.Name,
Source: volume.Source,
Destination: volume.Destination,
}

resp = append(resp, volResp)
}
return resp
}

// NewTasksResponse creates TasksResponse for all the tasks.
func NewTasksResponse(state dockerstate.TaskEngineState) *TasksResponse {
allTasks := state.AllTasks()
Expand Down
Loading