Remove unused code, add staticcheck to TravisCI (#2465)

* Remove unused code found via staticcheck tool

* Add staticcheck unused code check to CI

Makefile

@@ -277,6 +277,9 @@ importcheck:
 
 .PHONY: static-check
 static-check: gocyclo govet importcheck
+	# check for unused code using staticcheck binary
+	# https://github.com/dominikh/go-tools/tree/master/cmd/staticcheck
+	staticcheck -tests=false -checks "U1000" ./agent/...
 
 .PHONY: goimports
 goimports:
@@ -287,6 +290,7 @@ goimports:
 	go get github.com/golang/mock/mockgen
 	go get golang.org/x/tools/cmd/goimports
 	go get github.com/fzipp/gocyclo
+	go get honnef.co/go/tools/cmd/staticcheck
 	touch .get-deps-stamp
 
 get-deps: .get-deps-stamp

agent/acs/handler/payload_handler.go

@@ -277,8 +277,7 @@ func (payloadHandler *payloadRequestHandler) addPayloadTasks(payload *ecsacs.Pay
 	}
 
 	// Construct a slice with credentials acks from all tasks
-	var credentialsAcks []*ecsacs.IAMRoleCredentialsAckRequest
-	credentialsAcks = append(stoppedTasksCredentialsAcks, newTasksCredentialsAcks...)
+	credentialsAcks := append(stoppedTasksCredentialsAcks, newTasksCredentialsAcks...)
 	return credentialsAcks, allTasksOK
 }
 

agent/api/container/container.go

@@ -497,11 +497,7 @@ func (c *Container) GetNextKnownStateProgression() apicontainerstatus.ContainerS
 // IsInternal returns true if the container type is `ContainerCNIPause`
 // or `ContainerNamespacePause`. It returns false otherwise
 func (c *Container) IsInternal() bool {
-	if c.Type == ContainerNormal {
-		return false
-	}
-
-	return true
+	return c.Type != ContainerNormal
 }
 
 // IsRunning returns true if the container's known status is either RUNNING

agent/api/task/task.go

@@ -2586,15 +2586,6 @@ func (task *Task) GetContainerIndex(containerName string) int {
 	return -1
 }
 
-func (task *Task) requireEnvfiles() bool {
-	for _, container := range task.Containers {
-		if container.ShouldCreateWithEnvFiles() {
-			return true
-		}
-	}
-	return false
-}
-
 func (task *Task) initializeEnvfilesResource(config *config.Config, credentialsManager credentials.Manager) error {
 
 	for _, container := range task.Containers {

agent/api/task/task_linux.go

@@ -33,7 +33,6 @@ import (
 )
 
 const (
-	defaultCPUPeriod = 100 * time.Millisecond // 100ms
 	// With a 100ms CPU period, we can express 0.01 vCPU to 10 vCPUs
 	maxTaskVCPULimit = 10
 	// Reference: http://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html
@@ -231,11 +230,6 @@ func (task *Task) initializeCredentialSpecResource(config *config.Config, creden
 	return errors.New("task credentialspec is only supported on windows")
 }
 
-// getAllCredentialSpecRequirements is used to build all the credential spec requirements for the task
-func (task *Task) getAllCredentialSpecRequirements() []string {
-	return nil
-}
-
 // GetCredentialSpecResource retrieves credentialspec resource from resource map
 func (task *Task) GetCredentialSpecResource() ([]taskresource.TaskResource, bool) {
 	return []taskresource.TaskResource{}, false

agent/api/task/task_linux_test.go

@@ -61,6 +61,8 @@ const (
 	testTaskDefVersion         = "1"
 	testRegion                 = "testRegion"
 	testExecutionCredentialsID = "testExecutionCredentialsID"
+
+	defaultCPUPeriod = 100 * time.Millisecond // 100ms
 )
 
 func TestAddNetworkResourceProvisioningDependencyNop(t *testing.T) {

agent/api/task/task_test.go

@@ -3434,5 +3434,14 @@ func TestRequiresEnvfiles(t *testing.T) {
 		Containers:         []*apicontainer.Container{container},
 	}
 
-	assert.Equal(t, true, task.requireEnvfiles())
+	assert.Equal(t, true, requireEnvfiles(task))
+}
+
+func requireEnvfiles(task *Task) bool {
+	for _, container := range task.Containers {
+		if container.ShouldCreateWithEnvFiles() {
+			return true
+		}
+	}
+	return false
 }

agent/api/task/task_unsupported.go

@@ -27,15 +27,7 @@ import (
 )
 
 const (
-	defaultCPUPeriod = 100 * time.Millisecond // 100ms
-
-	// With a 100ms CPU period, we can express 0.01 vCPU to 10 vCPUs
-	maxTaskVCPULimit = 10
-	// Reference: http://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html
-	minimumCPUShare = 2
-
 	minimumCPUPercent = 0
-	bytesPerMegabyte  = 1024 * 1024
 )
 
 // PlatformFields consists of fields specific to Linux for a task
@@ -80,11 +72,6 @@ func (task *Task) initializeCredentialSpecResource(config *config.Config, creden
 	return errors.New("task credentialspec is only supported on windows")
 }
 
-// getAllCredentialSpecRequirements is used to build all the credential spec requirements for the task
-func (task *Task) getAllCredentialSpecRequirements() []string {
-	return nil
-}
-
 // GetCredentialSpecResource retrieves credentialspec resource from resource map
 func (task *Task) GetCredentialSpecResource() ([]taskresource.TaskResource, bool) {
 	return []taskresource.TaskResource{}, false

agent/api/task/taskvolume.go

@@ -29,8 +29,6 @@ const (
 	HostVolumeType   = "host"
 	DockerVolumeType = "docker"
 	EFSVolumeType    = "efs"
-
-	efsVolumePluginCapability = "efsAuth"
 )
 
 // TaskVolume is a definition of all the volumes available for containers to

agent/app/agent_test.go

@@ -579,7 +579,7 @@ func TestNewTaskEngineRestoreFromCheckpointClusterIDMismatch(t *testing.T) {
 	_, _, err := agent.newTaskEngine(eventstream.NewEventStream("events", ctx),
 		credentialsManager, state, imageManager)
 	assert.Error(t, err)
-	assert.True(t, isClusterMismatch(err))
+	assert.IsType(t, clusterMismatchError{}, err)
 }
 
 func TestNewTaskEngineRestoreFromCheckpointNewStateManagerError(t *testing.T) {

agent/app/errors.go

@@ -29,8 +29,3 @@ func isTransient(err error) bool {
 type clusterMismatchError struct {
 	error
 }
-
-func isClusterMismatch(err error) bool {
-	_, ok := err.(clusterMismatchError)
-	return ok
-}

agent/containermetadata/manager.go

@@ -22,7 +22,6 @@ import (
 	apitask "github.com/aws/amazon-ecs-agent/agent/api/task"
 	"github.com/aws/amazon-ecs-agent/agent/config"
 	"github.com/aws/amazon-ecs-agent/agent/dockerclient"
-	"github.com/aws/amazon-ecs-agent/agent/utils/ioutilwrapper"
 	dockercontainer "github.com/docker/docker/api/types/container"
 )
 
@@ -61,8 +60,6 @@ type metadataManager struct {
 	dataDirOnHost string
 	// containerInstanceARN is the Container Instance ARN registered for this agent
 	containerInstanceARN string
-	// ioutilWrap is a wrapper for 'ioutil' package operations
-	ioutilWrap ioutilwrapper.IOUtil
 	// availabilityZone is the availabiltyZone where task is in
 	availabilityZone string
 	// hostPrivateIPv4Address is the private IPv4 address associated with the EC2 instance

agent/containermetadata/types.go

@@ -105,7 +105,6 @@ type DockerContainerMetadata struct {
 	dockerContainerName string
 	imageID             string
 	imageName           string
-	networkMode         string
 	ports               []apicontainer.PortBinding
 	networkInfo         NetworkMetadata
 }

agent/dockerclient/dockerapi/docker_client.go

@@ -54,10 +54,6 @@ import (
 
 const (
 	dockerDefaultTag = "latest"
-	// imageNameFormat is the name of a image may look like: repo:tag
-	imageNameFormat = "%s:%s"
-	// the buffer size will ensure agent doesn't miss any event from docker
-	dockerEventBufferSize = 100
 	// healthCheckStarting is the initial status returned from docker container health check
 	healthCheckStarting = "starting"
 	// healthCheckHealthy is the healthy status returned from docker container health check
@@ -243,9 +239,6 @@ func (dg *dockerGoClient) WithVersion(version dockerclient.DockerVersion) Docker
 	}
 }
 
-// scratchCreateLock guards against multiple 'scratch' image creations at once
-var scratchCreateLock sync.Mutex
-
 // NewDockerGoClient creates a new DockerGoClient
 // TODO Remove clientfactory parameter once migration to Docker SDK is complete.
 func NewDockerGoClient(sdkclientFactory sdkclientfactory.Factory,
@@ -1035,15 +1028,13 @@ func (dg *dockerGoClient) listImages(ctx context.Context) ListImagesResponse {
 	if err != nil {
 		return ListImagesResponse{Error: err}
 	}
-	var imagesRepoTag []string
+	var imageRepoTags []string
 	imageIDs := make([]string, len(images))
 	for i, image := range images {
 		imageIDs[i] = image.ID
-		for _, imageRepoTag := range image.RepoTags {
-			imagesRepoTag = append(imagesRepoTag, imageRepoTag)
-		}
+		imageRepoTags = append(imageRepoTags, image.RepoTags...)
 	}
-	return ListImagesResponse{ImageIDs: imageIDs, RepoTags: imagesRepoTag, Error: nil}
+	return ListImagesResponse{ImageIDs: imageIDs, RepoTags: imageRepoTags, Error: nil}
 }
 
 func (dg *dockerGoClient) SupportedVersions() []dockerclient.DockerVersion {

agent/dockerclient/dockerapi/docker_client_test.go

@@ -74,6 +74,7 @@ const (
 	xMaximumPullRetryDelay     = 100 * time.Microsecond
 	xPullRetryDelayMultiplier  = 2
 	xPullRetryJitterMultiplier = 0.2
+	dockerEventBufferSize      = 100
 )
 
 func defaultTestConfig() *config.Config {

agent/dockerclient/dockerapi/docker_events_buffer.go

@@ -42,7 +42,6 @@ type InfiniteBuffer struct {
 	events       []*events.Message
 	empty        bool
 	waitForEvent sync.WaitGroup
-	count        int
 	lock         sync.RWMutex
 }
 

agent/dockerclient/sdkclientfactory/sdkclientfactory.go

@@ -23,15 +23,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-const (
-	// minAPIVersionKey is the docker.Env key for min API version
-	// This is supported in Docker API versions >=1.25
-	// https://docs.docker.com/engine/api/version-history/#v125-api-changes
-	minAPIVersionKey = "MinAPIVersion"
-	// apiVersionKey is the docker.Env key for API version
-	apiVersionKey = "ApiVersion"
-)
-
 // Factory provides a collection of docker remote clients that include a
 // recommended client version as well as a set of alternative supported
 // docker clients.

agent/ecscni/plugin.go

@@ -90,11 +90,6 @@ func (client *cniClient) SetupNS(
 	timeout time.Duration) (*current.Result, error) {
 	ctx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
-
-	type output struct {
-		result *current.Result
-		err    error
-	}
 	return client.setupNS(ctx, cfg)
 }
 

agent/engine/dependencygraph/graph.go

@@ -201,15 +201,6 @@ func verifyContainerDependenciesResolvedForResource(target taskresource.TaskReso
 	return true
 }
 
-func linksToContainerNames(links []string) []string {
-	names := make([]string, 0, len(links))
-	for _, link := range links {
-		name := strings.Split(link, ":")[0]
-		names = append(names, name)
-	}
-	return names
-}
-
 func executionCredentialsResolved(target *apicontainer.Container, id string, manager credentials.Manager) bool {
 	if target.GetKnownStatus() >= apicontainerstatus.ContainerPulled ||
 		!target.ShouldPullWithExecutionRole() ||

agent/engine/task_manager.go

@@ -41,7 +41,6 @@ import (
 	"github.com/aws/amazon-ecs-agent/agent/utils/ttime"
 
 	"github.com/cihub/seelog"
-	"github.com/pkg/errors"
 )
 
 const (
@@ -58,15 +57,10 @@ const (
 )
 
 var (
-	_stoppedSentWaitInterval       = stoppedSentWaitInterval
-	_maxStoppedWaitTimes           = int(maxStoppedWaitTimes)
-	taskNotWaitForSteadyStateError = errors.New("managed task: steady state check context is nil")
+	_stoppedSentWaitInterval = stoppedSentWaitInterval
+	_maxStoppedWaitTimes     = int(maxStoppedWaitTimes)
 )
 
-type acsTaskUpdate struct {
-	apitaskstatus.TaskStatus
-}
-
 type dockerContainerChange struct {
 	container *apicontainer.Container
 	event     dockerapi.DockerContainerChangeEvent

agent/eventhandler/task_handler.go

@@ -390,15 +390,6 @@ func (taskEvents *taskSendableEvents) toStringUnsafe() string {
 		taskEvents.taskARN, taskEvents.sending, taskEvents.createdAt.String())
 }
 
-// getTasksToEventsLen returns the length of the tasksToEvents map. It is
-// used only in the test code to ascertain that map has been cleaned up
-func (handler *TaskHandler) getTasksToEventsLen() int {
-	handler.lock.RLock()
-	defer handler.lock.RUnlock()
-
-	return len(handler.tasksToEvents)
-}
-
 // handleInvalidParamException removes the event from event queue when its parameters are
 // invalid to reduce redundant API call
 func handleInvalidParamException(err error, events *list.List, eventToSubmit *list.Element) {

agent/eventhandler/task_handler_test.go

@@ -335,13 +335,22 @@ func TestCleanupTaskEventAfterSubmit(t *testing.T) {
 
 	// Wait for task events to be removed from the tasksToEvents map
 	for {
-		if handler.getTasksToEventsLen() == 0 {
+		if getTasksToEventsLen(handler) == 0 {
 			break
 		}
 		time.Sleep(time.Millisecond)
 	}
 }
 
+// getTasksToEventsLen returns the length of the tasksToEvents map. It is
+// used only in the test code to ascertain that map has been cleaned up
+func getTasksToEventsLen(handler *TaskHandler) int {
+	handler.lock.RLock()
+	defer handler.lock.RUnlock()
+
+	return len(handler.tasksToEvents)
+}
+
 func containerEvent(arn string) statechange.Event {
 	return api.ContainerStateChange{TaskArn: arn, ContainerName: "containerName", Status: apicontainerstatus.ContainerRunning, Container: &apicontainer.Container{}}
 }

agent/eventhandler/task_handler_types.go

@@ -40,14 +40,6 @@ type sendableEvent struct {
 	lock sync.RWMutex
 }
 
-func newSendableContainerEvent(event api.ContainerStateChange) *sendableEvent {
-	return &sendableEvent{
-		isContainerEvent: true,
-		containerSent:    false,
-		containerChange:  event,
-	}
-}
-
 func newSendableTaskEvent(event api.TaskStateChange) *sendableEvent {
 	return &sendableEvent{
 		isContainerEvent: false,

agent/eventhandler/task_handler_types_test.go

@@ -29,6 +29,14 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func newSendableContainerEvent(event api.ContainerStateChange) *sendableEvent {
+	return &sendableEvent{
+		isContainerEvent: true,
+		containerSent:    false,
+		containerChange:  event,
+	}
+}
+
 func TestShouldContainerEventBeSent(t *testing.T) {
 	event := newSendableContainerEvent(api.ContainerStateChange{
 		Status: apicontainerstatus.ContainerStopped,

agent/httpclient/httpclient.go

@@ -27,8 +27,6 @@ import (
 	"github.com/aws/amazon-ecs-agent/agent/version"
 )
 
-const defaultTimeout = 10 * time.Minute
-
 // Taken from the default http.Client behavior
 const defaultDialTimeout = 30 * time.Second
 const defaultDialKeepalive = 30 * time.Second

agent/metrics/generic_metrics_client.go

@@ -115,12 +115,6 @@ func (gm *GenericMetrics) FireCallEnd(callHash, callName string, timestamp time.
 	}
 }
 
-// Simple Timeout function
-func startTimeout(timeout chan bool) {
-	time.Sleep(callTimeout)
-	timeout <- true
-}
-
 // This function increments the call count for a specific API call
 // This is invoked at the API call's start, whereas the duration metrics
 // are updated at the API call's end.