containermetadata: use asserts for tests

README.md

@@ -179,9 +179,6 @@ configure them as something other than the defaults.
 | `ECS_CNI_PLUGINS_PATH` | `/ecs/cni` | The path where the cni binary file is located | `/amazon-ecs-cni-plugins` | Not applicable |
 | `ECS_AWSVPC_BLOCK_IMDS` | `true` | Whether to block access to [Instance Metdata](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-metadata.html) for Tasks started with `awsvpc` network mode | `false` | Not applicable |
 | `ECS_AWSVPC_ADDITIONAL_LOCAL_ROUTES` | `["10.0.15.0/24"]` | In `awsvpc` network mode, traffic to these prefixes will be routed via the host bridge instead of the task ENI | `[]` | Not applicable |
-| `ECS_ENABLE_TASK_ENI` | `false` | Whether to enable task networking for task to be launched with its own network interface | `false` | `false` |
-| `ECS_CNI_PLUGINS_PATH` | `/ecs/cni` | The path where the cni binary file is located | `/amazon-ecs-cni-plugins` | `/amazon-ecs-cni-plugins` |
-| `ECS_AWSVPC_BLOCK_IMDS` | `true` | Whether to block access to [Instance Metdata](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-metadata.html) for Tasks started with `awsvpc` network mode | `false` | `false`|
 | `ECS_ENABLE_CONTAINER_METADATA` | `<true|false>` | Whether to enable creation of a metadata file in the containers. If enabled the agent will inject a file with the container's various metadata and the user can access the container's metadata internally by accessing the environment variable $ECS_CONTAINER_METADATA_FILE. | `false` | `false` |
 | `ECS_HOST_DATA_DIR` | `/var/lib/ecs` | The source directory on the host from which ECS_DATADIR is mounted. We use this to determine the source mount path for container metadata files in the case the Agent is run in a container. We do not use this value in Windows because the Agent is not run on a container in Windows. | `/var/lib/ecs` | `Not used` |
 

agent/api/container.go

@@ -112,7 +112,7 @@ type Container struct {
 	// handled properly so that the state storage continues to work.
 	SentStatusUnsafe ContainerStatus `json:"SentStatus"`
 
-	// MetadataUpdated is set to true when we have completed updating the
+	// MetadataFileUpdated is set to true when we have completed updating the
 	// metadata file
 	MetadataFileUpdated bool `json:"metadataFileUpdated"`
 

agent/config/config_test.go

@@ -151,7 +151,8 @@ func TestEnvironmentConfig(t *testing.T) {
 	assert.NoError(t, err, "should marshal additional local routes")
 	assert.Equal(t, additionalLocalRoutesJSON, string(serializedAdditionalLocalRoutesJSON))
 	assert.Equal(t, (90 * time.Second), conf.TaskCleanupWaitDuration)
-	assert.Equal(t, "/etc/ecs/", conf.DataDirOnHost)
+	assert.Equal(t, "/etc/ecs/", conf.DataDirOnHost, "Wrong value for DataDirOnHost")
+	assert.True(t, conf.ContainerMetadataEnabled, "Wrong value for ContainerMetadataEnabled")
 }
 
 func TestTrimWhitespace(t *testing.T) {

agent/containermetadata/manager.go

@@ -21,7 +21,6 @@ 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"
 
 	docker "github.com/fsouza/go-dockerclient"

agent/containermetadata/manager_test.go

@@ -25,6 +25,7 @@ import (
 
 	docker "github.com/fsouza/go-dockerclient"
 	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
 )
 
 const (
@@ -54,9 +55,7 @@ func TestSetContainerInstanceARN(t *testing.T) {
 
 	newManager := &metadataManager{}
 	newManager.SetContainerInstanceARN(mockARN)
-	if newManager.containerInstanceARN != mockARN {
-		t.Error("Got unexpected container instance ARN: " + newManager.containerInstanceARN)
-	}
+	assert.Equal(t, mockARN, newManager.containerInstanceARN)
 }
 
 // TestCreateMalformedFilepath checks case when taskARN is invalid resulting in an invalid file path
@@ -71,9 +70,7 @@ func TestCreateMalformedFilepath(t *testing.T) {
 	err := newManager.Create(nil, nil, mockTaskARN, mockContainerName)
 	expectErrorMessage := fmt.Sprintf("container metadata create for task %s container %s: get metdata file path of task %s container %s: get task ARN: invalid TaskARN %s", mockTaskARN, mockContainerName, mockTaskARN, mockContainerName, mockTaskARN)
 
-	if err.Error() != expectErrorMessage {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+	assert.Equal(t, expectErrorMessage, err.Error())
 }
 
 // TestCreateMkdirAllFail checks case when MkdirAll call fails
@@ -94,9 +91,7 @@ func TestCreateMkdirAllFail(t *testing.T) {
 	err := newManager.Create(nil, nil, mockTaskARN, mockContainerName)
 	expectErrorMessage := fmt.Sprintf("creating metadata directory for task %s: err", mockTaskARN)
 
-	if err.Error() != expectErrorMessage {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+	assert.Equal(t, expectErrorMessage, err.Error())
 }
 
 // TestUpdateInspectFail checks case when Inspect call fails
@@ -115,11 +110,7 @@ func TestUpdateInspectFail(t *testing.T) {
 	mockClient.EXPECT().InspectContainer(mockDockerID, inspectContainerTimeout).Return(nil, errors.New("Inspect fail"))
 	err := newManager.Update(mockDockerID, mockTaskARN, mockContainerName)
 
-	if err == nil {
-		t.Error("Expected inspect error to result in update fail")
-	} else if err.Error() != "Inspect fail" {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+	assert.Error(t, err, "Expected inspect error to result in update fail")
 }
 
 // TestUpdateNotRunningFail checks case where container is not running
@@ -145,13 +136,11 @@ func TestUpdateNotRunningFail(t *testing.T) {
 	err := newManager.Update(mockDockerID, mockTaskARN, mockContainerName)
 	expectErrorMessage := fmt.Sprintf("container metadata update for task %s container %s: container not running or invalid", mockTaskARN, mockContainerName)
 
-	if err.Error() != expectErrorMessage {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+	assert.Equal(t, expectErrorMessage, err.Error())
 }
 
-// TestCleanMalformedFilepath checks case where ARN is invalid
-func TestCleanMalformedFilepath(t *testing.T) {
+// TestMalformedFilepath checks case where ARN is invalid
+func TestMalformedFilepath(t *testing.T) {
 	_, _, _, _, done := managerSetup(t)
 	defer done()
 
@@ -161,13 +150,11 @@ func TestCleanMalformedFilepath(t *testing.T) {
 	err := newManager.Clean(mockTaskARN)
 	expectErrorMessage := fmt.Sprintf("clean task %s: get task ARN: invalid TaskARN invalidARN", mockTaskARN)
 
-	if err.Error() != expectErrorMessage {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+	assert.Equal(t, expectErrorMessage, err.Error())
 }
 
-// TestClean is the mainline case for metadata create
-func TestClean(t *testing.T) {
+// TestHappyPath is the mainline case for metadata create
+func TestHappyPath(t *testing.T) {
 	_, _, mockOS, _, done := managerSetup(t)
 	defer done()
 
@@ -181,7 +168,6 @@ func TestClean(t *testing.T) {
 		mockOS.EXPECT().RemoveAll(gomock.Any()).Return(nil),
 	)
 	err := newManager.Clean(mockTaskARN)
-	if err != nil {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+
+	assert.NoError(t, err)
 }

agent/containermetadata/manager_unix_test.go

@@ -19,6 +19,7 @@ import (
 
 	docker "github.com/fsouza/go-dockerclient"
 	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
 )
 
 // TestCreate is the mainline case for metadata create
@@ -47,16 +48,9 @@ func TestCreate(t *testing.T) {
 	}
 	err := newManager.Create(mockConfig, mockHostConfig, mockTaskARN, mockContainerName)
 
-	if err != nil {
-		t.Error("Got unexpected error: " + err.Error())
-	}
-
-	if len(mockConfig.Env) != 1 {
-		t.Error("Unexpected number of environment variables in config: ", len(mockConfig.Env))
-	}
-	if len(mockHostConfig.Binds) != 1 {
-		t.Error("Unexpected number of binds in host config: ", len(mockHostConfig.Binds))
-	}
+	assert.NoError(t, err)
+	assert.Equal(t, 1, len(mockConfig.Env), "Unexpected number of environment variables in config")
+	assert.Equal(t, 1, len(mockHostConfig.Binds), "Unexpected number of binds in host config")
 }
 
 // TestUpdate is mainline case for metadata update
@@ -90,7 +84,6 @@ func TestUpdate(t *testing.T) {
 		mockFile.EXPECT().Close().Return(nil),
 	)
 	err := newManager.Update(mockDockerID, mockTaskARN, mockContainerName)
-	if err != nil {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+
+	assert.NoError(t, err)
 }

agent/containermetadata/manager_windows_test.go

@@ -47,16 +47,9 @@ func TestCreate(t *testing.T) {
 	}
 	err := newManager.Create(mockConfig, mockHostConfig, mockTaskARN, mockContainerName)
 
-	if err != nil {
-		t.Error("Got unexpected error: " + err.Error())
-	}
-
-	if len(mockConfig.Env) != 1 {
-		t.Error("Unexpected number of environment variables in config: ", len(mockConfig.Env))
-	}
-	if len(mockHostConfig.Binds) != 1 {
-		t.Error("Unexpected number of binds in host config: ", len(mockHostConfig.Binds))
-	}
+	assert.NoError(t, err)
+	assert.Equal(t, 1, len(mockConfig.Env), "Unexpected number of environment variables in config")
+	assert.Equal(t, 1, len(mockHostConfig.Binds), "Unexpected number of binds in host config")
 }
 
 // TestUpdate is mainline case for metadata update
@@ -87,7 +80,6 @@ func TestUpdate(t *testing.T) {
 		mockFile.EXPECT().Close().Return(nil),
 	)
 	err := newManager.Update(mockDockerID, mockTaskARN, mockContainerName)
-	if err != nil {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+
+	assert.NoError(t, err)
 }

agent/containermetadata/utils_test.go

@@ -16,6 +16,7 @@ package containermetadata
 
 import (
 	"fmt"
+	"github.com/stretchr/testify/assert"
 	"testing"
 )
 
@@ -30,7 +31,5 @@ func TestGetTaskIDFailDueToInvalidID(t *testing.T) {
 	_, err := getTaskIDfromARN(mockTaskARN)
 	expectErrorMessage := fmt.Sprintf("get task ARN: cannot find TaskID for TaskARN %s", mockTaskARN)
 
-	if err.Error() != expectErrorMessage {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+	assert.Equal(t, expectErrorMessage, err.Error())
 }

agent/containermetadata/write_metadata_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/aws/amazon-ecs-agent/agent/utils/oswrapper/mocks"
 
 	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
 )
 
 func writeSetup(t *testing.T) (*mock_ioutilwrapper.MockIOUtil, *mock_oswrapper.MockOS, *mock_oswrapper.MockFile, func()) {
@@ -44,7 +45,5 @@ func TestWriteInvalidARN(t *testing.T) {
 	expectErrorMessage := fmt.Sprintf("write to metadata file for task %s container %s: get metdata file path of task %s container %s: get task ARN: invalid TaskARN %s", mockTaskARN, mockContainerName, mockTaskARN, mockContainerName, mockTaskARN)
 
 	err := writeToMetadataFile(nil, nil, mockData, mockTaskARN, mockContainerName, mockDataDir)
-	if err.Error() != expectErrorMessage {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+	assert.Equal(t, expectErrorMessage, err.Error())
 }

agent/containermetadata/write_metadata_unix_test.go

@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
 )
 
 // TestWriteTempFileFail checks case where temp file cannot be made
@@ -36,12 +37,10 @@ func TestWriteTempFileFail(t *testing.T) {
 	)
 
 	err := writeToMetadataFile(nil, mockIOUtil, mockData, mockTaskARN, mockContainerName, mockDataDir)
-	if err == nil {
-		t.Error("Expected error to be returned")
-	}
-	if err.Error() != "temp file fail" {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+	expectErrorMessage := "temp file fail"
+
+	assert.Error(t, err)
+	assert.Equal(t, expectErrorMessage, err.Error())
 }
 
 // TestWriteFileWriteFail checks case where write to file fails
@@ -61,12 +60,10 @@ func TestWriteFileWriteFail(t *testing.T) {
 	)
 
 	err := writeToMetadataFile(nil, mockIOUtil, mockData, mockTaskARN, mockContainerName, mockDataDir)
-	if err == nil {
-		t.Error("Expected error to be returned")
-	}
-	if err.Error() != "write fail" {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+	expectErrorMessage := "write fail"
+
+	assert.Error(t, err)
+	assert.Equal(t, expectErrorMessage, err.Error())
 }
 
 // TestWriteChmodFail checks case where chmod fails
@@ -87,10 +84,8 @@ func TestWriteChmodFail(t *testing.T) {
 	)
 
 	err := writeToMetadataFile(nil, mockIOUtil, mockData, mockTaskARN, mockContainerName, mockDataDir)
-	if err == nil {
-		t.Error("Expected error to be returned")
-	}
-	if err.Error() != "chmod fail" {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+	expectErrorMessage := "chmod fail"
+
+	assert.Error(t, err)
+	assert.Equal(t, expectErrorMessage, err.Error())
 }

agent/containermetadata/write_metadata_windows_test.go

@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
 )
 
 // TestWriteOpenFileFail checks case where open file fails and does not return a NotExist error
@@ -38,12 +39,11 @@ func TestWriteOpenFileFail(t *testing.T) {
 	)
 
 	err := writeToMetadataFile(mockOS, nil, mockData, mockTaskARN, mockContainerName, mockDataDir)
-	if err == nil {
-		t.Error("Expected error to be returned")
-	}
-	if err.Error() != "does exist" {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+
+	expectErrorMessage := "does exist"
+
+	assert.Error(t, err)
+	assert.Equal(t, expectErrorMessage, err.Error())
 }
 
 // TestWriteFileWrtieFail checks case where we fail to write to file
@@ -63,10 +63,9 @@ func TestWriteFileWriteFail(t *testing.T) {
 	)
 
 	err := writeToMetadataFile(mockOS, nil, mockData, mockTaskARN, mockContainerName, mockDataDir)
-	if err == nil {
-		t.Error("Expected error to be returned")
-	}
-	if err.Error() != "write fail" {
-		t.Error("Got unexpected error: " + err.Error())
-	}
+
+	expectErrorMessage := "write fail"
+
+	assert.Error(t, err)
+	assert.Equal(t, expectErrorMessage, err.Error())
 }

agent/engine/default.go

@@ -27,6 +27,10 @@ import (
 var log = logger.ForModule("TaskEngine")
 
 // NewTaskEngine returns a default TaskEngine
-func NewTaskEngine(cfg *config.Config, client DockerClient, credentialsManager credentials.Manager, containerChangeEventStream *eventstream.EventStream, imageManager ImageManager, state dockerstate.TaskEngineState, metadataManager containermetadata.Manager) TaskEngine {
+func NewTaskEngine(cfg *config.Config, client DockerClient,
+	credentialsManager credentials.Manager,
+	containerChangeEventStream *eventstream.EventStream,
+	imageManager ImageManager, state dockerstate.TaskEngineState,
+	metadataManager containermetadata.Manager) TaskEngine {
 	return NewDockerTaskEngine(cfg, client, credentialsManager, containerChangeEventStream, imageManager, state, metadataManager)
 }

agent/engine/docker_task_engine.go

@@ -100,7 +100,10 @@ type DockerTaskEngine struct {
 // The distinction between created and initialized is that when created it may
 // be serialized/deserialized, but it will not communicate with docker until it
 // is also initialized.
-func NewDockerTaskEngine(cfg *config.Config, client DockerClient, credentialsManager credentials.Manager, containerChangeEventStream *eventstream.EventStream, imageManager ImageManager, state dockerstate.TaskEngineState, metadataManager containermetadata.Manager) *DockerTaskEngine {
+func NewDockerTaskEngine(cfg *config.Config, client DockerClient,
+	credentialsManager credentials.Manager, containerChangeEventStream *eventstream.EventStream,
+	imageManager ImageManager, state dockerstate.TaskEngineState,
+	metadataManager containermetadata.Manager) *DockerTaskEngine {
 	dockerTaskEngine := &DockerTaskEngine{
 		cfg:    cfg,
 		client: client,