Skip to content

Commit

Permalink
making ecs-init tests os-agnostic
Browse files Browse the repository at this point in the history
  • Loading branch information
singholt committed Jan 25, 2025
1 parent 7f838b7 commit ca3426a
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 24 deletions.
7 changes: 5 additions & 2 deletions ecs-init/config/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ const (
ECSAgentAppArmorDefaultProfileName = "ecs-agent-default"
)

// OsStat is useful to mock in unit tests
var OsStat = os.Stat

// partitionBucketRegion provides the "partitional" bucket region
// suitable for downloading agent from.
var partitionBucketRegion = map[string]string{
Expand Down Expand Up @@ -258,15 +261,15 @@ func MountDirectoryEBS() string {

// HostCertsDirPath() returns the CA store path on the host
func HostCertsDirPath() string {
if _, err := os.Stat(hostCertsDirPath); err != nil {
if _, err := OsStat(hostCertsDirPath); err != nil {
return ""
}
return hostCertsDirPath
}

// HostPKIDirPath() returns the CA store path on the host
func HostPKIDirPath() string {
if _, err := os.Stat(hostPKIDirPath); err != nil {
if _, err := OsStat(hostPKIDirPath); err != nil {
return ""
}
return hostPKIDirPath
Expand Down
10 changes: 7 additions & 3 deletions ecs-init/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ const (
// nvidiaGPUDevicesPresentMaxRetries specifies the maximum number of retries to attempt for checking if NVIDIA
// GPU devices are present.
nvidiaGPUDevicesPresentMaxRetries = 10

// lsblk lists information about block devices. This is used by the ECS agent for the EBS task attach functionality.
// Ref: https://man7.org/linux/man-pages/man8/lsblk.8.html
lsblkDir = "/usr/bin/lsblk"
)

// Do NOT include "CAP_" in capability string
Expand Down Expand Up @@ -495,7 +499,7 @@ func getCredentialsFetcherSocketBind() (string, bool) {
credentialsFetcherUnixSocketHostPath, ok := config.HostCredentialsFetcherPath()
if ok && credentialsFetcherUnixSocketHostPath != "" {
// check whether the path to the credentials fetcher socket exists
_, err := os.Stat(credentialsFetcherUnixSocketHostPath)
_, err := config.OsStat(credentialsFetcherUnixSocketHostPath)
if err != nil {
if os.IsNotExist(err) {
return "", false
Expand All @@ -508,7 +512,7 @@ func getCredentialsFetcherSocketBind() (string, bool) {
}

// getDockerSocketBind returns the bind for Docker socket.
// Value for the bind is as follow:
// Value for the bind is as follows:
// 1. DOCKER_HOST (as in os.Getenv) not set: source /var/run, dest /var/run
// 2. DOCKER_HOST (as in os.Getenv) set: source DOCKER_HOST (as in os.Getenv, trim unix:// prefix),
// dest DOCKER_HOST (as in /etc/ecs/ecs.config, trim unix:// prefix)
Expand Down Expand Up @@ -562,7 +566,7 @@ func getCapabilityBinds() []string {
}

func defaultIsPathValid(path string, shouldBeDirectory bool) bool {
fileInfo, err := os.Stat(path)
fileInfo, err := config.OsStat(path)
if err != nil {
return false
}
Expand Down
2 changes: 1 addition & 1 deletion ecs-init/docker/docker_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func createHostConfig(binds []string) *godocker.HostConfig {
iptablesExecutableHostDir+":"+iptablesExecutableContainerDir+readOnly,
iptablesAltDir+":"+iptablesAltDir+readOnly,
iptablesLegacyDir+":"+iptablesLegacyDir+readOnly,
"/usr/bin/lsblk:/usr/bin/lsblk",
lsblkDir+":"+lsblkDir,
)
binds = append(binds, getNsenterBinds(os.Stat)...)
binds = append(binds, getModInfoBinds(os.Stat)...)
Expand Down
66 changes: 48 additions & 18 deletions ecs-init/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,6 @@ import (
"github.com/stretchr/testify/assert"
)

// defaultExpectedAgentBinds is the total number of agent host config binds.
// Note: Change this value every time when a new bind mount is added to
// agent for the tests to pass
const (
defaultExpectedAgentBinds = 22
)

func TestIsAgentImageLoadedListFailure(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
Expand Down Expand Up @@ -197,7 +190,7 @@ func TestStartAgentNoEnvFile(t *testing.T) {
mockFS.EXPECT().ReadFile(config.InstanceConfigFile()).Return(nil, errors.New("not found")).AnyTimes()
mockFS.EXPECT().ReadFile(config.AgentConfigFile()).Return(nil, errors.New("test error")).AnyTimes()
mockDocker.EXPECT().CreateContainer(gomock.Any()).Do(func(opts godocker.CreateContainerOptions) {
validateCommonCreateContainerOptions(t, opts, defaultExpectedAgentBinds)
validateCommonCreateContainerOptions(t, opts)
}).Return(&godocker.Container{
ID: containerID,
}, nil)
Expand All @@ -218,7 +211,6 @@ func TestStartAgentNoEnvFile(t *testing.T) {
func validateCommonCreateContainerOptions(
t *testing.T,
opts godocker.CreateContainerOptions,
expectedAgentBinds int,
) {
if opts.Name != "ecs-agent" {
t.Errorf("Expected container Name to be %s but was %s", "ecs-agent", opts.Name)
Expand Down Expand Up @@ -250,7 +242,6 @@ func validateCommonCreateContainerOptions(

hostCfg := opts.HostConfig

assert.Len(t, hostCfg.Binds, expectedAgentBinds)
binds := make(map[string]struct{})
for _, binding := range hostCfg.Binds {
binds[binding] = struct{}{}
Expand All @@ -261,6 +252,9 @@ func validateCommonCreateContainerOptions(
expectKey(config.AgentDataDirectory()+":/data", binds, t)
expectKey(config.AgentConfigDirectory()+":"+config.AgentConfigDirectory(), binds, t)
expectKey(config.CacheDirectory()+":"+config.CacheDirectory(), binds, t)
expectKey(config.CgroupMountpoint()+":"+DefaultCgroupMountpoint, binds, t)
expectKey(config.InstanceConfigDirectory()+":"+config.InstanceConfigDirectory(), binds, t)
expectKey(filepath.Join(config.LogDirectory(), execAgentLogRelativePath)+":"+filepath.Join(logDir, execAgentLogRelativePath), binds, t)
expectKey(config.ProcFS+":"+hostProcDir+":ro", binds, t)
expectKey(iptablesUsrLibDir+":"+iptablesUsrLibDir+":ro", binds, t)
expectKey(iptablesLibDir+":"+iptablesLibDir+":ro", binds, t)
Expand All @@ -269,11 +263,25 @@ func validateCommonCreateContainerOptions(
expectKey(iptablesExecutableHostDir+":"+iptablesExecutableContainerDir+":ro", binds, t)
expectKey(iptablesAltDir+":"+iptablesAltDir+":ro", binds, t)
expectKey(iptablesLegacyDir+":"+iptablesLegacyDir+":ro", binds, t)
expectKey(config.HostPKIDirPath()+":"+config.HostPKIDirPath(), binds, t)
expectKey(lsblkDir+":"+lsblkDir, binds, t)
expectKey(config.LogDirectory()+"/exec:/log/exec", binds, t)
for _, pluginDir := range pluginDirs {
expectKey(pluginDir+":"+pluginDir+readOnly, binds, t)
}

// verify nsenter binds are present in the hostConfig
nsEnterBinds := getNsenterBinds(os.Stat)
for _, nsEnterDir := range nsEnterBinds {
expectKey(nsEnterDir+":"+nsEnterDir, binds, t)
}

// verify modInfo binds are present in the hostConfig
modInfoBinds := getModInfoBinds(os.Stat)
for _, modInfoBind := range modInfoBinds {
expectKey(modInfoBind+":"+modInfoBind, binds, t)
}

if hostCfg.NetworkMode != networkMode {
t.Errorf("Expected network mode to be %s, got %s", networkMode, hostCfg.NetworkMode)
}
Expand Down Expand Up @@ -318,6 +326,15 @@ func TestStartAgentEnvFile(t *testing.T) {
isPathValid = defaultIsPathValid
}()

// mock os.Stat so that unit test assertions don't change with a change in the underlying OS versions
// where we run these tests.
config.OsStat = func(name string) (os.FileInfo, error) {
return nil, nil
}
defer func() {
config.OsStat = os.Stat
}()

envFile := "\nAGENT_TEST_VAR=val\nAGENT_TEST_VAR2=val2\n"
containerID := "container id"

Expand All @@ -327,7 +344,7 @@ func TestStartAgentEnvFile(t *testing.T) {
mockFS.EXPECT().ReadFile(config.InstanceConfigFile()).Return(nil, errors.New("not found")).AnyTimes()
mockFS.EXPECT().ReadFile(config.AgentConfigFile()).Return([]byte(envFile), nil).AnyTimes()
mockDocker.EXPECT().CreateContainer(gomock.Any()).Do(func(opts godocker.CreateContainerOptions) {
validateCommonCreateContainerOptions(t, opts, defaultExpectedAgentBinds)
validateCommonCreateContainerOptions(t, opts)
cfg := opts.Config

envVariables := make(map[string]struct{})
Expand Down Expand Up @@ -364,8 +381,6 @@ func TestStartAgentWithGPUConfig(t *testing.T) {

envFile := "\nECS_ENABLE_GPU_SUPPORT=true\n"
containerID := "container id"
expectedAgentBinds := defaultExpectedAgentBinds
expectedAgentBinds += 1

defer func() {
MatchFilePatternForGPU = FilePatternMatchForGPU
Expand All @@ -380,7 +395,7 @@ func TestStartAgentWithGPUConfig(t *testing.T) {
mockFS.EXPECT().ReadFile(config.InstanceConfigFile()).Return([]byte(envFile), nil).AnyTimes()
mockFS.EXPECT().ReadFile(config.AgentConfigFile()).Return(nil, errors.New("not found")).AnyTimes()
mockDocker.EXPECT().CreateContainer(gomock.Any()).Do(func(opts godocker.CreateContainerOptions) {
validateCommonCreateContainerOptions(t, opts, expectedAgentBinds)
validateCommonCreateContainerOptions(t, opts)
var found bool
for _, bind := range opts.HostConfig.Binds {
if bind == gpu.GPUInfoDirPath+":"+gpu.GPUInfoDirPath {
Expand Down Expand Up @@ -421,6 +436,15 @@ func TestStartAgentWithGPUConfigNoDevices(t *testing.T) {
isPathValid = defaultIsPathValid
}()

// mock os.Stat so that unit test assertions don't change with a change in the underlying OS versions
// where we run these tests.
config.OsStat = func(name string) (os.FileInfo, error) {
return nil, nil
}
defer func() {
config.OsStat = os.Stat
}()

envFile := "\nECS_ENABLE_GPU_SUPPORT=true\n"
containerID := "container id"

Expand All @@ -438,7 +462,7 @@ func TestStartAgentWithGPUConfigNoDevices(t *testing.T) {
mockFS.EXPECT().ReadFile(config.InstanceConfigFile()).Return([]byte(envFile), nil).AnyTimes()
mockFS.EXPECT().ReadFile(config.AgentConfigFile()).Return(nil, errors.New("not found")).AnyTimes()
mockDocker.EXPECT().CreateContainer(gomock.Any()).Do(func(opts godocker.CreateContainerOptions) {
validateCommonCreateContainerOptions(t, opts, defaultExpectedAgentBinds)
validateCommonCreateContainerOptions(t, opts)
cfg := opts.Config

envVariables := make(map[string]struct{})
Expand Down Expand Up @@ -864,6 +888,14 @@ func TestStartAgentWithExecBinds(t *testing.T) {
isPathValid = func(path string, isDir bool) bool {
return true
}
// mock os.Stat so that unit test assertions don't change with a change in the underlying OS versions
// where we run these tests.
config.OsStat = func(name string) (os.FileInfo, error) {
return nil, nil
}
defer func() {
config.OsStat = os.Stat
}()
hostCapabilityExecResourcesDir := filepath.Join(hostResourcesRootDir, execCapabilityName)
containerCapabilityExecResourcesDir := filepath.Join(containerResourcesRootDir, execCapabilityName)

Expand All @@ -876,8 +908,6 @@ func TestStartAgentWithExecBinds(t *testing.T) {
hostConfigDir + ":" + containerConfigDir,
}

expectedAgentBinds := defaultExpectedAgentBinds
expectedAgentBinds += len(expectedExecBinds)
// bind mount for the config folder is already included in expectedAgentBinds since it's always added
expectedExecBinds = append(expectedExecBinds, hostConfigDir+":"+containerConfigDir)
defer func() {
Expand All @@ -890,7 +920,7 @@ func TestStartAgentWithExecBinds(t *testing.T) {
mockFS.EXPECT().ReadFile(config.InstanceConfigFile()).Return(nil, errors.New("not found")).AnyTimes()
mockFS.EXPECT().ReadFile(config.AgentConfigFile()).Return(nil, errors.New("not found")).AnyTimes()
mockDocker.EXPECT().CreateContainer(gomock.Any()).Do(func(opts godocker.CreateContainerOptions) {
validateCommonCreateContainerOptions(t, opts, expectedAgentBinds)
validateCommonCreateContainerOptions(t, opts)

// verify that exec binds are added
assert.Subset(t, opts.HostConfig.Binds, expectedExecBinds)
Expand Down

0 comments on commit ca3426a

Please sign in to comment.