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

Merge branch 'dev' into boltdb #2532

Merged
merged 14 commits into from
Jul 21, 2020
Merged
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,5 @@ _bin/
/misc/taskmetadata-validator/taskmetadata-validator
/bin
*.iml
cover.out
coverprofile.out
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ matrix:
- os: linux
script:
- make get-deps
- make test
- make static-check
- make test
- make analyze-cover-profile
- make xplatform-build
- os: windows
script:
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## 1.41.1
* Bug - Fixed a bug [#2476](https://github.com/aws/amazon-ecs-agent/issues/2476) where HostPort is not present in ECS Task Metadata Endpoint response with bridge network type [#2495](https://github.com/aws/amazon-ecs-agent/pull/2495)

## 1.41.0
* Feature - Add inferentia support [#2458](https://github.com/aws/amazon-ecs-agent/pull/2458)
* Bug - fixes a bug where env file feature would not accept "=", which is the delimiter in the values of a env var [#2487](https://github.com/aws/amazon-ecs-agent/pull/2487)

## 1.40.0
* Enhancement - Agent's default stats gathering is changing from docker streaming stats to polling. This should not affect the metrics that customers ultimately see in cloudwatch, but it does affect how the agent gathers the underlying metrics from docker. This change was made for considerable performance gains. Customers with high CPU loads may see their cluster utilization increase; this is a good thing because it means the containers are utilizing more of the cluster, and agent/dockerd/containerd are utilizing less [#2452](https://github.com/aws/amazon-ecs-agent/pull/2452)
* Enhancement - Adds a jitter to this so that we don't query docker for every container's state all at the same time [#2444](https://github.com/aws/amazon-ecs-agent/pull/2444)
Expand Down
13 changes: 11 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ VERBOSE=-v -cover
# -count=1 runs the test without using the build cache. The build cache can
# provide false positives when running integ tests, so we err on the side of
# caution. See `go help test`
# unit tests include the coverage profile
GOTEST=${GO_EXECUTABLE} test -count=1 ${VERBOSE}

# -race sometimes causes compile issues on Arm
Expand All @@ -119,11 +120,17 @@ ifneq (${BUILD_PLATFORM},aarch64)
endif

test:
${GOTEST} -tags unit -timeout=60s ./agent/...
${GOTEST} -tags unit -coverprofile cover.out -timeout=60s ./agent/...
go tool cover -func cover.out > coverprofile.out

test-silent:
$(eval VERBOSE=)
${GOTEST} -tags unit -timeout=60s ./agent/...
${GOTEST} -tags unit -coverprofile cover.out -timeout=60s ./agent/...
go tool cover -func cover.out > coverprofile.out

.PHONY: analyze-cover-profile
analyze-cover-profile: coverprofile.out
./scripts/analyze-cover-profile

run-integ-tests: test-registry gremlin container-health-check-image run-sudo-tests
ECS_LOGLEVEL=debug ${GOTEST} -tags integration -timeout=30m ./agent/...
Expand Down Expand Up @@ -331,4 +338,6 @@ clean:
-rm -f .builder-image-stamp
-rm -f .out-stamp
-rm -rf $(PWD)/bin
-rm -rf cover.out
-rm -rf coverprofile.out

2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.40.0
1.41.1
18 changes: 6 additions & 12 deletions agent/api/task/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ const (
ContainerOrderingCreateCondition = "CREATE"
ContainerOrderingStartCondition = "START"

arnResourceSections = 2
arnResourceDelimiter = "/"
// networkModeNone specifies the string used to define the `none` docker networking mode
networkModeNone = "none"
Expand Down Expand Up @@ -1095,9 +1094,10 @@ func (task *Task) collectFirelensLogEnvOptions(containerToLogOptions map[string]
// container's host config.
func (task *Task) AddFirelensContainerBindMounts(firelensConfig *apicontainer.FirelensConfig, hostConfig *dockercontainer.HostConfig,
config *config.Config) *apierrors.HostConfigError {
// TODO: fix task.GetID(). It's currently incorrect when opted in task long arn format.
fields := strings.Split(task.Arn, "/")
taskID := fields[len(fields)-1]
taskID, err := task.GetID()
if err != nil {
return &apierrors.HostConfigError{Msg: err.Error()}
}

var configBind, s3ConfigBind, socketBind string
switch firelensConfig.Type {
Expand Down Expand Up @@ -2259,14 +2259,8 @@ func (task *Task) GetID() (string, error) {
return "", errors.Errorf("task get-id: malformed task resource: %s", resource)
}

resourceSplit := strings.SplitN(resource, arnResourceDelimiter, arnResourceSections)
if len(resourceSplit) != arnResourceSections {
return "", errors.Errorf(
"task get-id: invalid task resource split: %s, expected=%d, actual=%d",
resource, arnResourceSections, len(resourceSplit))
}

return resourceSplit[1], nil
resourceSplit := strings.Split(resource, arnResourceDelimiter)
return resourceSplit[len(resourceSplit)-1], nil
}

// RecordExecutionStoppedAt checks if this is an essential container stopped
Expand Down
15 changes: 12 additions & 3 deletions agent/api/task/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1595,7 +1595,7 @@ func TestGetIDErrorPaths(t *testing.T) {
{"", "EmptyString"},
{"invalidArn", "InvalidARN"},
{"arn:aws:ecs:region:account-id:task:task-id", "IncorrectSections"},
{"arn:aws:ecs:region:account-id:task", "IncorrectResouceSections"},
{"arn:aws:ecs:region:account-id:task", "IncorrectResourceSections"},
}

task := Task{}
Expand All @@ -1612,12 +1612,21 @@ func TestGetIDErrorPaths(t *testing.T) {

// TestGetIDHappyPath validates the happy path of GetID
func TestGetIDHappyPath(t *testing.T) {
task := Task{
taskNormalARN := Task{
Arn: "arn:aws:ecs:region:account-id:task/task-id",
}
taskID, err := task.GetID()
taskLongARN := Task{
Arn: "arn:aws:ecs:region:account-id:task/cluster-name/task-id",
}

taskID, err := taskNormalARN.GetID()
assert.NoError(t, err)
assert.Equal(t, "task-id", taskID)

taskID, err = taskLongARN.GetID()
assert.NoError(t, err)
assert.Equal(t, "task-id", taskID)

}

// TestTaskGetPrimaryENI tests the eni can be correctly acquired by calling GetTaskPrimaryENI
Expand Down
1 change: 0 additions & 1 deletion agent/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ const (
DefaultTaskMetadataBurstRate = 60

//Known cached image names
CachedImageNamePauseContainer = "amazon/amazon-ecs-pause:0.1.0"
CachedImageNameAgentContainer = "amazon/amazon-ecs-agent:latest"

// DefaultNvidiaRuntime is the name of the runtime to pass Nvidia GPUs to containers
Expand Down
2 changes: 1 addition & 1 deletion agent/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ func TestValidForImagesCleanupExclusion(t *testing.T) {
defer setTestRegion()()
defer setTestEnv("ECS_EXCLUDE_UNTRACKED_IMAGE", "amazonlinux:2,amazonlinux:3")()
imagesNotDelete := parseImageCleanupExclusionList("ECS_EXCLUDE_UNTRACKED_IMAGE")
expectedImages := []string{"amazonlinux:2", "amazonlinux:3", CachedImageNameAgentContainer, CachedImageNamePauseContainer}
expectedImages := []string{"amazonlinux:2", "amazonlinux:3"}
assert.Equal(t, expectedImages, imagesNotDelete, "unexpected imageCleanupExclusionList")
}

Expand Down
7 changes: 1 addition & 6 deletions agent/config/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,16 +309,11 @@ func parseImageCleanupExclusionList(envVar string) []string {
var imageCleanupExclusionList []string
if imageEnv == "" {
seelog.Debugf("Environment variable empty: %s", imageEnv)
return nil
} else {
imageCleanupExclusionList = strings.Split(imageEnv, ",")
}

// append known cached internal images to imageCleanupExclusionLis
imageCleanupExclusionList = append(imageCleanupExclusionList, CachedImageNameAgentContainer, CachedImageNamePauseContainer)

for _, image := range imageCleanupExclusionList {
seelog.Infof("Image excluded from cleanup: %s", image)
}
return imageCleanupExclusionList
}

Expand Down
15 changes: 14 additions & 1 deletion agent/engine/docker_image_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func NewImageManager(cfg *config.Config, client dockerapi.DockerClient, state do
numImagesToDelete: cfg.NumImagesToDeletePerCycle,
imageCleanupTimeInterval: cfg.ImageCleanupInterval,
imagePullBehavior: cfg.ImagePullBehavior,
imageCleanupExclusionList: cfg.ImageCleanupExclusionList,
imageCleanupExclusionList: buildImageCleanupExclusionList(cfg),
deleteNonECSImagesEnabled: cfg.DeleteNonECSImagesEnabled,
nonECSContainerCleanupWaitDuration: cfg.TaskCleanupWaitDuration,
numNonECSContainersToDelete: cfg.NumNonECSContainersToDeletePerCycle,
Expand All @@ -94,6 +94,19 @@ func (imageManager *dockerImageManager) SetDataClient(dataClient data.Client) {
imageManager.dataClient = dataClient
}

func buildImageCleanupExclusionList(cfg *config.Config) []string {
// append known cached internal images to imageCleanupExclusionList
excludedImages := append(cfg.ImageCleanupExclusionList,
cfg.PauseContainerImageName+":"+cfg.PauseContainerTag,
config.DefaultPauseContainerImageName+":"+config.DefaultPauseContainerTag,
config.CachedImageNameAgentContainer,
)
for _, image := range excludedImages {
seelog.Infof("Image excluded from cleanup: %s", image)
}
return excludedImages
}

func (imageManager *dockerImageManager) AddAllImageStates(imageStates []*image.ImageState) {
imageManager.updateLock.Lock()
defer imageManager.updateLock.Unlock()
Expand Down
17 changes: 17 additions & 0 deletions agent/engine/docker_image_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,23 @@ func defaultTestConfig() *config.Config {
return cfg
}

func TestNewImageManagerExcludesCachedImages(t *testing.T) {
cfg := defaultTestConfig()
cfg.PauseContainerImageName = "pause-name"
cfg.PauseContainerTag = "pause-tag"
cfg.ImageCleanupExclusionList = []string{"excluded:1"}
expected := []string{
"excluded:1",
"pause-name:pause-tag",
config.DefaultPauseContainerImageName + ":" + config.DefaultPauseContainerTag,
config.CachedImageNameAgentContainer,
}
imageManager := NewImageManager(cfg, nil, nil)
dockerImageManager, ok := imageManager.(*dockerImageManager)
require.True(t, ok, "imageManager must be *dockerImageManager")
assert.ElementsMatch(t, expected, dockerImageManager.imageCleanupExclusionList)
}

// TestImagePullRemoveDeadlock tests if there's a deadlock when trying to
// pull an image while image clean up is in progress
func TestImagePullRemoveDeadlock(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion agent/engine/docker_task_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2966,7 +2966,7 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) {

func TestCreateFirelensContainerSetFluentdUID(t *testing.T) {
testTask := &apitask.Task{
Arn: "test-task-arn",
Arn: "arn:aws:ecs:region:account-id:task/test-task-arn",
Containers: []*apicontainer.Container{
{
Name: "test-container",
Expand Down
4 changes: 2 additions & 2 deletions agent/handlers/task_server_setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ var (
CPU: cpu,
Memory: memory,
Type: apicontainer.ContainerNormal,
Ports: []apicontainer.PortBinding{
KnownPortBindingsUnsafe: []apicontainer.PortBinding{
{
ContainerPort: containerPort,
Protocol: apicontainer.TransportProtocolTCP,
Expand Down Expand Up @@ -229,7 +229,7 @@ var (
CPU: cpu,
Memory: memory,
Type: apicontainer.ContainerNormal,
Ports: []apicontainer.PortBinding{
KnownPortBindingsUnsafe: []apicontainer.PortBinding{
{
ContainerPort: containerPort,
Protocol: apicontainer.TransportProtocolTCP,
Expand Down
2 changes: 1 addition & 1 deletion agent/handlers/v2/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func newContainerResponse(dockerContainer *apicontainer.DockerContainer,
resp.FinishedAt = &finishedAt
}

for _, binding := range container.Ports {
for _, binding := range container.GetKnownPortBindings() {
port := v1.PortResponse{
ContainerPort: binding.ContainerPort,
Protocol: binding.Protocol.String(),
Expand Down
4 changes: 2 additions & 2 deletions agent/handlers/v2/response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ func TestTaskResponseMarshal(t *testing.T) {
V3EndpointID: "",
Image: imageName,
ImageID: imageID,
Ports: []apicontainer.PortBinding{
KnownPortBindingsUnsafe: []apicontainer.PortBinding{
{
ContainerPort: 80,
Protocol: apicontainer.TransportProtocolTCP,
Expand Down Expand Up @@ -414,7 +414,7 @@ func TestContainerResponseMarshal(t *testing.T) {
Status: apicontainerstatus.ContainerHealthy,
Since: aws.Time(timeRFC3339),
},
Ports: []apicontainer.PortBinding{
KnownPortBindingsUnsafe: []apicontainer.PortBinding{
{
ContainerPort: 80,
Protocol: apicontainer.TransportProtocolTCP,
Expand Down
2 changes: 1 addition & 1 deletion agent/taskresource/envFiles/envfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ func (envfile *EnvironmentFileResource) readEnvVarsFromFile(envfilePath string)
}
// only read the line that has "="
if strings.Contains(line, envVariableDelimiter) {
variables := strings.Split(line, "=")
variables := strings.SplitN(line, envVariableDelimiter, 2)
// verify that there is at least a character on each side
if len(variables[0]) > 0 && len(variables[1]) > 0 {
envVars[variables[0]] = variables[1]
Expand Down
10 changes: 7 additions & 3 deletions agent/taskresource/envFiles/envfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@ func TestReadEnvVarsFromEnvfiles(t *testing.T) {
envfileResource := newMockEnvfileResource(envfiles, nil, nil, mockIOUtil)
envfileResource.bufio = mockBufio

envfileContent := "key=value"
envfileContentLine1 := "key1=value"
envFileContentLine2 := "key2=val1=val2"

tempOpen := open
open = func(name string) (oswrapper.File, error) {
Expand All @@ -325,7 +326,9 @@ func TestReadEnvVarsFromEnvfiles(t *testing.T) {
gomock.InOrder(
mockBufio.EXPECT().NewScanner(mockFile).Return(mockScanner),
mockScanner.EXPECT().Scan().Return(true),
mockScanner.EXPECT().Text().Return(envfileContent),
mockScanner.EXPECT().Text().Return(envfileContentLine1),
mockScanner.EXPECT().Scan().Return(true),
mockScanner.EXPECT().Text().Return(envFileContentLine2),
mockScanner.EXPECT().Scan().Return(false),
mockScanner.EXPECT().Err().Return(nil),
)
Expand All @@ -334,7 +337,8 @@ func TestReadEnvVarsFromEnvfiles(t *testing.T) {

assert.Nil(t, err)
assert.Equal(t, 1, len(envVarsList))
assert.Equal(t, "value", envVarsList[0]["key"])
assert.Equal(t, "value", envVarsList[0]["key1"])
assert.Equal(t, "val1=val2", envVarsList[0]["key2"])
}

func TestReadEnvVarsCommentFromEnvfiles(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions agent/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ package version
// repository. Only the 'Version' const should change in checked-in source code

// Version is the version of the Agent
const Version = "1.40.0"
const Version = "1.41.1"

// GitDirty indicates the cleanliness of the git repo when this agent was built
const GitDirty = true

// GitShortHash is the short hash of this agent build
const GitShortHash = "017dc6c0"
const GitShortHash = "0d002a15"
9 changes: 1 addition & 8 deletions misc/awscli/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,8 +1 @@
FROM debian:stable

RUN apt-get update && apt-get install -y \
python2.7 curl

RUN curl -O https://bootstrap.pypa.io/get-pip.py

RUN python2.7 get-pip.py && pip install awscli
FROM amazon/aws-cli:2.0.27
2 changes: 1 addition & 1 deletion misc/container-health/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
# express or implied. See the License for the specific language governing
# permissions and limitations under the License.
FROM busybox:1.29.3
FROM busybox:1.32.0

HEALTHCHECK --interval=1s --timeout=1s --retries=3 CMD echo hello

Expand Down
4 changes: 2 additions & 2 deletions misc/fluentd/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# fluent/fluentd:v1.2.5
FROM fluent/fluentd@sha256:c99d55c78d383df803d49f2f54755e41c654ea8e3c1ef34ba695d3a1af33151c
# fluent/fluentd:v1.11-1
FROM fluent/fluentd@sha256:708cb509799785a40a07c4269e5dded4bfa6df1ae50631f24e4921ae98fda1e5

COPY fluent.conf /fluentd/etc/
2 changes: 1 addition & 1 deletion misc/image-cleanup-test-images/linux0.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
# express or implied. See the License for the specific language governing
# permissions and limitations under the License.
FROM busybox:1.29.3
FROM busybox:1.32.0

MAINTAINER Amazon Web Services, Inc.

Expand Down
2 changes: 1 addition & 1 deletion misc/image-cleanup-test-images/linux1.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
# express or implied. See the License for the specific language governing
# permissions and limitations under the License.
FROM busybox:1.29.3
FROM busybox:1.32.0

MAINTAINER Amazon Web Services, Inc.

Expand Down
2 changes: 1 addition & 1 deletion misc/image-cleanup-test-images/linux2.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
# express or implied. See the License for the specific language governing
# permissions and limitations under the License.
FROM busybox:1.29.3
FROM busybox:1.32.0

MAINTAINER Amazon Web Services, Inc.

Expand Down
2 changes: 1 addition & 1 deletion misc/taskmetadata-validator/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
# express or implied. See the License for the specific language governing
# permissions and limitations under the License.
FROM busybox:1.29.3
FROM busybox:1.32.0

MAINTAINER Amazon Web Services, Inc.
COPY taskmetadata-validator /
Expand Down
Loading