Skip to content

Commit

Permalink
Merge pull request #946 from samuelkarp/ecs
Browse files Browse the repository at this point in the history
  • Loading branch information
samuelkarp authored Jul 27, 2020
2 parents 9c46dd2 + 8025807 commit 988c0de
Show file tree
Hide file tree
Showing 42 changed files with 1,055 additions and 23 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jobs:
continue-on-error: ${{ matrix.supported }}
strategy:
matrix:
variant: [aws-k8s-1.15, aws-k8s-1.16, aws-k8s-1.17]
variant: [aws-k8s-1.15, aws-k8s-1.16, aws-k8s-1.17, aws-ecs-1]
arch: [x86_64, aarch64]
supported: [true]
include:
Expand Down
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ ENV PACKAGE=${PACKAGE} ARCH=${ARCH}
COPY ./macros/${ARCH} ./macros/shared ./macros/rust ./macros/cargo ./packages/${PACKAGE}/ .
RUN rpmdev-setuptree \
&& cat ${ARCH} shared rust cargo > .rpmmacros \
&& echo "%_cross_variant ${VARIANT}" >> .rpmmacros \
&& rm ${ARCH} shared rust cargo \
&& mv *.spec rpmbuild/SPECS \
&& find . -maxdepth 1 -not -path '*/\.*' -type f -exec mv {} rpmbuild/SOURCES/ \; \
Expand Down
7 changes: 7 additions & 0 deletions packages/Cargo.lock

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

1 change: 1 addition & 0 deletions packages/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ members = [
"docker-engine",
"docker-init",
"docker-proxy",
"ecs-agent",
"filesystem",
"findutils",
"glibc",
Expand Down
3 changes: 2 additions & 1 deletion packages/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ Requires: %{name}

Macros start with `%`.
If the macro is specific to Bottlerocket, it will include the `cross` token.
The definitions for these can be found in [macros](../macros).
The definitions for most of these can be found in [macros](../macros).
The definition for `%{_cross_variant}` is the Bottlerocket variant being built.

When developing a package on an RPM-based system, you can expand the macros with a command like this.
```
Expand Down
14 changes: 14 additions & 0 deletions packages/containerd/containerd-config-toml_aws-ecs-1
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
version = 2
root = "/var/lib/containerd"
state = "/run/containerd"
disabled_plugins = [
"io.containerd.internal.v1.opt",
"io.containerd.snapshotter.v1.aufs",
"io.containerd.snapshotter.v1.devmapper",
"io.containerd.snapshotter.v1.native",
"io.containerd.snapshotter.v1.zfs",
"io.containerd.grpc.v1.cri",
]

[grpc]
address = "/run/containerd/containerd.sock"
7 changes: 4 additions & 3 deletions packages/containerd/containerd.spec
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ Source0: https://%{goimport}/archive/v%{gover}/%{gorepo}-%{gover}.tar.gz
Source1: containerd.service
Source2: containerd-config-toml_aws-k8s
Source3: containerd-config-toml_aws-dev
Source4: containerd-tmpfiles.conf
Source4: containerd-config-toml_aws-ecs-1
Source5: containerd-tmpfiles.conf
Source1000: clarify.toml

# Upstream patch; can drop when we move to v1.4.0.
Expand Down Expand Up @@ -77,10 +78,10 @@ install -p -m 0644 %{S:1} %{buildroot}%{_cross_unitdir}/containerd.service

install -d %{buildroot}%{_cross_templatedir}
install -d %{buildroot}%{_cross_factorydir}%{_cross_sysconfdir}/containerd
install -p -m 0644 %{S:2} %{S:3} %{buildroot}%{_cross_templatedir}
install -p -m 0644 %{S:2} %{S:3} %{S:4} %{buildroot}%{_cross_templatedir}

install -d %{buildroot}%{_cross_tmpfilesdir}
install -p -m 0644 %{S:4} %{buildroot}%{_cross_tmpfilesdir}/containerd.conf
install -p -m 0644 %{S:5} %{buildroot}%{_cross_tmpfilesdir}/containerd.conf

%cross_scan_attribution --clarify %{S:1000} go-vendor vendor

Expand Down
3 changes: 1 addition & 2 deletions packages/docker-engine/daemon.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
{
"bridge": "none",
"log-driver": "journald",
"live-restore": true,
"max-concurrent-downloads": 10,
"storage-driver": "overlay2",
"exec-opts": ["native.cgroupdriver=systemd"],
"exec-opts": ["native.cgroupdriver=cgroupfs"],
"data-root": "/var/lib/docker",
"selinux-enabled": true
}
145 changes: 145 additions & 0 deletions packages/ecs-agent/0001-engine-move-default-image-exclusions.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
From c2a7cea2edeae59cdc6ca31696e36f7e922ae71f Mon Sep 17 00:00:00 2001
From: Samuel Karp <[email protected]>
Date: Wed, 8 Jul 2020 10:05:12 -0700
Subject: [PATCH] engine: move default image exclusions

This commit moves the logic adding default excluded images from the
cleanup list to the engine package, away from the config package, and in
doing so fixes two bugs:

1. Prior to this change, any value for ImageCleanupExclusionList
provided by a config mechanism *other* than environmentConfig would
be ignored. This is because environmentConfig has the highest
precedence, the defaults were added to environmentConfig by
parseImageCleanupExclusionList, and config.Merge will only merge a
new value when the left config's field is its zero value. Since the
default excluded images were populated, environmentConfig's
ImageCleanupExclusionList field was never zero.
2. CachedImageNamePauseContainer hard-coded a name for the pause
container image that was used to populate the exclusion list, but the
actual name of the pause container is a value populated at link-time
into the DefaultPauseContainerImageName and DefaultPauseContainerTag
variables. If the value was set to anything other than what was
defined in CachedImageNamePauseContainer, the pause container image
would not be correctly excluded from image cleanup.

Signed-off-by: Samuel Karp <[email protected]>
---
agent/config/config.go | 1 -
agent/config/config_test.go | 2 +-
agent/config/parse.go | 7 +------
agent/engine/docker_image_manager.go | 15 ++++++++++++++-
agent/engine/docker_image_manager_test.go | 17 +++++++++++++++++
5 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/agent/config/config.go b/agent/config/config.go
index 1079684b..20f2f684 100644
--- a/agent/config/config.go
+++ b/agent/config/config.go
@@ -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
diff --git a/agent/config/config_test.go b/agent/config/config_test.go
index e4ae71ab..3ac2d9e2 100644
--- a/agent/config/config_test.go
+++ b/agent/config/config_test.go
@@ -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")
}

diff --git a/agent/config/parse.go b/agent/config/parse.go
index 186f126d..ef1165af 100644
--- a/agent/config/parse.go
+++ b/agent/config/parse.go
@@ -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
}

diff --git a/agent/engine/docker_image_manager.go b/agent/engine/docker_image_manager.go
index 2d575457..22815db5 100644
--- a/agent/engine/docker_image_manager.go
+++ b/agent/engine/docker_image_manager.go
@@ -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,
@@ -89,6 +89,19 @@ func NewImageManager(cfg *config.Config, client dockerapi.DockerClient, state do
}
}

+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) SetSaver(stateManager statemanager.Saver) {
imageManager.saver = stateManager
}
diff --git a/agent/engine/docker_image_manager_test.go b/agent/engine/docker_image_manager_test.go
index 6fbacba4..8a014dc3 100644
--- a/agent/engine/docker_image_manager_test.go
+++ b/agent/engine/docker_image_manager_test.go
@@ -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) {
--
2.27.0

Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
From f3e8123fc244d256e95f0dac0a12ee8cf383d92a Mon Sep 17 00:00:00 2001
From: Samuel Karp <[email protected]>
Date: Wed, 8 Jul 2020 11:18:35 -0700
Subject: [PATCH 1/2] bottlerocket: default filesystem locations

---
agent/config/config.go | 6 +++---
agent/config/config_unix.go | 4 ++--
agent/utils/license.go | 6 +++---
3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/agent/config/config.go b/agent/config/config.go
index 1079684b..b11c0153 100644
--- a/agent/config/config.go
+++ b/agent/config/config.go
@@ -51,7 +51,7 @@ const (
AgentPrometheusExpositionPort = 51680

// defaultConfigFileName is the default (json-formatted) config file
- defaultConfigFileName = "/etc/ecs_container_agent/config.json"
+ defaultConfigFileName = "/etc/ecs/ecs.config.json"

// DefaultClusterName is the name of the default cluster.
DefaultClusterName = "default"
@@ -115,13 +115,13 @@ const (
minimumNumImagesToDeletePerCycle = 1

// defaultCNIPluginsPath is the default path where cni binaries are located
- defaultCNIPluginsPath = "/amazon-ecs-cni-plugins"
+ defaultCNIPluginsPath = "/usr/lib/amazon-ecs-agent/cni-plugins"

// DefaultMinSupportedCNIVersion denotes the minimum version of cni spec required
DefaultMinSupportedCNIVersion = "0.3.0"

// pauseContainerTarball is the path to the pause container tarball
- pauseContainerTarballPath = "/images/amazon-ecs-pause.tar"
+ pauseContainerTarballPath = "/usr/lib/amazon-ecs-agent/amazon-ecs-pause.tar"

// DefaultTaskMetadataSteadyStateRate is set as 40. This is arrived from our benchmarking
// results where task endpoint can handle 4000 rps effectively. Here, 100 containers
diff --git a/agent/config/config_unix.go b/agent/config/config_unix.go
index ac5f5d0f..86984db4 100644
--- a/agent/config/config_unix.go
+++ b/agent/config/config_unix.go
@@ -27,7 +27,7 @@ const (
// AgentCredentialsAddress is used to serve the credentials for tasks.
AgentCredentialsAddress = "" // this is left blank right now for net=bridge
// defaultAuditLogFile specifies the default audit log filename
- defaultCredentialsAuditLogFile = "/log/audit.log"
+ defaultCredentialsAuditLogFile = "/var/log/ecs/audit.log"
// DefaultTaskCgroupPrefix is default cgroup prefix for ECS tasks
DefaultTaskCgroupPrefix = "/ecs"

@@ -48,7 +48,7 @@ func DefaultConfig() Config {
DockerEndpoint: "unix:///var/run/docker.sock",
ReservedPorts: []uint16{SSHPort, DockerReservedPort, DockerReservedSSLPort, AgentIntrospectionPort, AgentCredentialsPort},
ReservedPortsUDP: []uint16{},
- DataDir: "/data/",
+ DataDir: "/var/lib/ecs/data",
DataDirOnHost: "/var/lib/ecs",
DisableMetrics: false,
ReservedMemory: 0,
diff --git a/agent/utils/license.go b/agent/utils/license.go
index 6eccff6a..324307cd 100644
--- a/agent/utils/license.go
+++ b/agent/utils/license.go
@@ -24,9 +24,9 @@ type LicenseProvider interface {
type licenseProvider struct{}

const (
- licenseFile = "LICENSE"
- noticeFile = "NOTICE"
- thirdPartyFile = "THIRD-PARTY"
+ licenseFile = "/usr/share/licenses/ecs-agent/LICENSE"
+ noticeFile = "/usr/share/licenses/ecs-agent/NOTICE"
+ thirdPartyFile = "/usr/share/licenses/ecs-agent/THIRD-PARTY"
)

var readFile = ioutil.ReadFile
--
2.27.0

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
From 648041259821e73e79965f3de3864109c3a5a816 Mon Sep 17 00:00:00 2001
From: Samuel Karp <[email protected]>
Date: Thu, 16 Jul 2020 10:52:01 -0700
Subject: [PATCH] bottlerocket: version values settable with linker

---
agent/version/version.go | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/agent/version/version.go b/agent/version/version.go
index 4417a7cf..6fcb4832 100644
--- a/agent/version/version.go
+++ b/agent/version/version.go
@@ -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.41.0"
+var Version = "1.41.0"

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

// GitShortHash is the short hash of this agent build
-const GitShortHash = "33c15e8b"
+var GitShortHash = "33c15e8b"
--
2.27.0

22 changes: 22 additions & 0 deletions packages/ecs-agent/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[package]
name = "ecs-agent"
version = "0.1.0"
edition = "2018"
publish = false
build = "build.rs"

[lib]
path = "pkg.rs"

# ECS agent
[[package.metadata.build-package.external-files]]
url = "https://github.com/aws/amazon-ecs-agent/archive/v1.41.0.tar.gz"
sha512 = "d53d1a47eb41d39ffd541b6e416b42276f1d2207fdf5f031be173a8737655dab71ca0c88732716b6fee0761f7fede474cee08a21fd2db5d33b460d6aa547630e"

# TODO: Package the CNI plugins
# The ECS agent repository includes two CNI plugins as git submodules. git
# archive does not include submodules, so the tarball above does not include
# the source of those plugins.

[build-dependencies]
glibc = { path = "../glibc" }
Loading

0 comments on commit 988c0de

Please sign in to comment.