diff --git a/agent/acs/session/payload_responder.go b/agent/acs/session/payload_responder.go index 7c5a61e2cc4..230e945f1e6 100644 --- a/agent/acs/session/payload_responder.go +++ b/agent/acs/session/payload_responder.go @@ -30,6 +30,7 @@ import ( loggerfield "github.com/aws/amazon-ecs-agent/ecs-agent/logger/field" nlappmesh "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/appmesh" ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface" + "github.com/aws/aws-sdk-go/aws" "github.com/pkg/errors" ) @@ -109,8 +110,8 @@ func (pmHandler *payloadMessageHandler) addPayloadTasks(payload *ecsacs.PayloadM continue } - // Note: If we receive an EBS-backed task, we'll also received an incomplete volume configuration in the list of Volumes - // To accomodate this, we'll first check if the task IS EBS-backed then we'll mark the corresponding Volume object to be + // Note: If we receive an EBS-backed task, we'll also receive an incomplete volume configuration in the list of Volumes + // To accommodate this, we'll first check if the task IS EBS-backed then we'll mark the corresponding Volume object to be // of type "attachment". This volume object will be replaced by the newly created EBS volume configuration when we parse // through the task attachments. volName, ok := hasEBSAttachment(task) diff --git a/agent/acs/session/payload_responder_test.go b/agent/acs/session/payload_responder_test.go index 96a9817c71b..5cf12924437 100644 --- a/agent/acs/session/payload_responder_test.go +++ b/agent/acs/session/payload_responder_test.go @@ -39,6 +39,7 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/credentials" ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface" "github.com/aws/amazon-ecs-agent/ecs-agent/wsclient" + "github.com/aws/aws-sdk-go/aws" "github.com/golang/mock/gomock" "github.com/pkg/errors" diff --git a/agent/acs/session/task_stop_verification_ack_responder_test.go b/agent/acs/session/task_stop_verification_ack_responder_test.go index 83127ab2fe7..f7f28826ada 100644 --- a/agent/acs/session/task_stop_verification_ack_responder_test.go +++ b/agent/acs/session/task_stop_verification_ack_responder_test.go @@ -29,6 +29,7 @@ import ( apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status" "github.com/aws/amazon-ecs-agent/ecs-agent/metrics" "github.com/aws/amazon-ecs-agent/ecs-agent/wsclient" + "github.com/aws/aws-sdk-go/aws" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" diff --git a/agent/api/container/container.go b/agent/api/container/container.go index 9984dd1673e..6faed8197cf 100644 --- a/agent/api/container/container.go +++ b/agent/api/container/container.go @@ -66,7 +66,7 @@ const ( // v4 metadata endpoint MetadataURIEnvVarNameV4 = "ECS_CONTAINER_METADATA_URI_V4" - // MetadataURIFormat defines the URI format for v4 metadata endpoint + // MetadataURIFormatV4 defines the URI format for v4 metadata endpoint MetadataURIFormatV4 = "http://169.254.170.2/v4/%s" // AgentURIEnvVarName defines the name of the environment variable @@ -85,7 +85,7 @@ const ( // SecretTypeEnv is to show secret type being ENVIRONMENT_VARIABLE SecretTypeEnv = "ENVIRONMENT_VARIABLE" - // TargetLogDriver is to show secret target being "LOG_DRIVER", the default will be "CONTAINER" + // SecretTargetLogDriver is to show secret target being "LOG_DRIVER", the default will be "CONTAINER" SecretTargetLogDriver = "LOG_DRIVER" // neuronVisibleDevicesEnvVar is the env which indicates that the container wants to use inferentia devices. @@ -237,7 +237,7 @@ type Container struct { // NOTE: Do not access DesiredStatusUnsafe directly. Instead, use `GetDesiredStatus` // and `SetDesiredStatus`. // TODO DesiredStatusUnsafe should probably be private with appropriately written - // setter/getter. When this is done, we need to ensure that the UnmarshalJSON + // setter/getter. When this is done, we need to ensure that the UnmarshalJSON // is handled properly so that the state storage continues to work. DesiredStatusUnsafe apicontainerstatus.ContainerStatus `json:"desiredStatus"` @@ -245,7 +245,7 @@ type Container struct { // NOTE: Do not access `KnownStatusUnsafe` directly. Instead, use `GetKnownStatus` // and `SetKnownStatus`. // TODO KnownStatusUnsafe should probably be private with appropriately written - // setter/getter. When this is done, we need to ensure that the UnmarshalJSON + // setter/getter. When this is done, we need to ensure that the UnmarshalJSON // is handled properly so that the state storage continues to work. KnownStatusUnsafe apicontainerstatus.ContainerStatus `json:"KnownStatus"` @@ -291,7 +291,7 @@ type Container struct { MetadataFileUpdated bool `json:"metadataFileUpdated"` // KnownExitCodeUnsafe specifies the exit code for the container. - // It is exposed outside of the package so that it's marshalled/unmarshalled in + // It is exposed outside the package so that it's marshalled/unmarshalled in // the JSON body while saving the state. // NOTE: Do not access KnownExitCodeUnsafe directly. Instead, use `GetKnownExitCode` // and `SetKnownExitCode`. @@ -312,8 +312,8 @@ type Container struct { // SteadyStateStatusUnsafe specifies the steady state status for the container // If uninitialized, it's assumed to be set to 'ContainerRunning'. Even though // it's not only supposed to be set when the container is being created, it's - // exposed outside of the package so that it's marshalled/unmarshalled in the - // the JSON body while saving the state + // exposed outside the package so that it's marshalled/unmarshalled in the + // JSON body while saving the state SteadyStateStatusUnsafe *apicontainerstatus.ContainerStatus `json:"SteadyStateStatus,omitempty"` // ContainerArn is the Arn of this container. @@ -420,7 +420,7 @@ func (s *Secret) GetSecretResourceCacheKey() string { return s.ValueFrom + "_" + s.Region } -// String returns a human readable string representation of DockerContainer +// String returns a human-readable string representation of DockerContainer func (dc *DockerContainer) String() string { if dc == nil { return "nil" @@ -533,7 +533,7 @@ func (c *Container) ShouldPullWithExecutionRole() bool { c.RegistryAuthentication.ECRAuthData.UseExecutionRole } -// String returns a human readable string representation of this object +// String returns a human-readable string representation of this object func (c *Container) String() string { ret := fmt.Sprintf("%s(%s) (%s->%s)", c.Name, c.Image, c.GetKnownStatus().String(), c.GetDesiredStatus().String()) @@ -561,7 +561,7 @@ func (c *Container) Fields() logger.Fields { // Container.steadyState is not initialized, the default steady state status // defined by `defaultContainerSteadyStateStatus` is returned. In awsvpc, the 'pause' // container's steady state differs from that of other containers, as the -// 'pause' container can reach its teady state once networking resources +// 'pause' container can reach its steady state once networking resources // have been provisioned for it, which is done in the `ContainerResourcesProvisioned` // state. In bridge mode, pause containers are currently used exclusively for // supporting service-connect tasks. Those pause containers will have steady state @@ -1114,7 +1114,7 @@ func (c *Container) MergeEnvironmentVariablesFromEnvfiles(envVarsList []map[stri c.lock.Lock() defer c.lock.Unlock() - // create map if does not exist + // create map if it does not exist if c.Environment == nil { c.Environment = make(map[string]string) } diff --git a/agent/api/task/task.go b/agent/api/task/task.go index 95664bf785f..959bbba473d 100644 --- a/agent/api/task/task.go +++ b/agent/api/task/task.go @@ -96,7 +96,7 @@ const ( // container's option (network, ipc, or pid) to that of another existing container dockerMappingContainerPrefix = "container:" - // awslogsCredsEndpointOpt is the awslogs option that is used to pass in an + // awslogsCredsEndpointOpt is the awslogs option that is used to pass in a // http endpoint for authentication awslogsCredsEndpointOpt = "awslogs-credentials-endpoint" // These contants identify the docker flag options @@ -143,13 +143,13 @@ const ( // ec2ExecutionEnv specifies the ec2 execution environment. ec2ExecutionEnv = "AWS_ECS_EC2" - // specifies bridge type mode for a task + // BridgeNetworkMode specifies bridge type mode for a task BridgeNetworkMode = "bridge" - // specifies awsvpc type mode for a task + // AWSVPCNetworkMode specifies awsvpc type mode for a task AWSVPCNetworkMode = "awsvpc" - // specifies host type mode for a task + // HostNetworkMode specifies host type mode for a task HostNetworkMode = "host" // disableIPv6SysctlKey specifies the setting that controls whether ipv6 is disabled. @@ -198,8 +198,8 @@ type Task struct { // the desired status is informed by the ECS backend as a result of either // API calls made to ECS or decisions made by the ECS service scheduler. // The DesiredStatusUnsafe is almost always either apitaskstatus.TaskRunning or apitaskstatus.TaskStopped. - // NOTE: Do not access DesiredStatusUnsafe directly. Instead, use `UpdateStatus`, - // `UpdateDesiredStatus`, `SetDesiredStatus`, and `SetDesiredStatus`. + // NOTE: Do not access DesiredStatusUnsafe directly. + // Instead, use `UpdateStatus`, `UpdateDesiredStatus`, and `SetDesiredStatus`. // TODO DesiredStatusUnsafe should probably be private with appropriately written // setter/getter. When this is done, we need to ensure that the UnmarshalJSON // is handled properly so that the state storage continues to work. @@ -226,11 +226,10 @@ type Task struct { // it won't be set if the pull never happens PullStoppedAtUnsafe time.Time `json:"PullStoppedAt"` // ExecutionStoppedAtUnsafe is the timestamp when the task desired status moved to stopped, - // which is when the any of the essential containers stopped + // which is when any of the essential containers stopped ExecutionStoppedAtUnsafe time.Time `json:"ExecutionStoppedAt"` // SentStatusUnsafe represents the last KnownStatusUnsafe that was sent to the ECS SubmitTaskStateChange API. - // TODO(samuelkarp) SentStatusUnsafe needs a lock and setters/getters. // TODO SentStatusUnsafe should probably be private with appropriately written // setter/getter. When this is done, we need to ensure that the UnmarshalJSON // is handled properly so that the state storage continues to work. @@ -261,8 +260,7 @@ type Task struct { PlatformFields PlatformFields `json:"PlatformFields,omitempty"` // terminalReason should be used when we explicitly move a task to stopped. - // This ensures the task object carries some context for why it was explicitly - // stoppped. + // This ensures the task object carries some context for why it was explicitly stopped. terminalReason string terminalReasonOnce sync.Once @@ -301,7 +299,7 @@ type Task struct { } // TaskFromACS translates ecsacs.Task to apitask.Task by first marshaling the received -// ecsacs.Task to json and unmarshaling it as apitask.Task +// ecsacs.Task to json and unmarshalling it as apitask.Task func TaskFromACS(acsTask *ecsacs.Task, envelope *ecsacs.PayloadMessage) (*Task, error) { data, err := jsonutil.BuildJSON(acsTask) if err != nil { @@ -1004,7 +1002,7 @@ func (task *Task) initializeCredentialsEndpoint(credentialsManager credentials.M task.SetCredentialsRelativeURI(credentialsEndpointRelativeURI) } -// initializeContainersV3MetadataEndpoint generates an v3 endpoint id for each container, constructs the +// initializeContainersV3MetadataEndpoint generates a v3 endpoint id for each container, constructs the // v3 metadata endpoint, and injects it as an environment variable func (task *Task) initializeContainersV3MetadataEndpoint(uuidProvider utils.UUIDProvider) { task.initializeV3EndpointIDForAllContainers(uuidProvider) @@ -1013,7 +1011,7 @@ func (task *Task) initializeContainersV3MetadataEndpoint(uuidProvider utils.UUID } } -// initializeContainersV4MetadataEndpoint generates an v4 endpoint id which we reuse the v3 container id +// initializeContainersV4MetadataEndpoint generates a v4 endpoint id which we reuse the v3 container id // (they are the same) for each container, constructs the v4 metadata endpoint, // and injects it as an environment variable func (task *Task) initializeContainersV4MetadataEndpoint(uuidProvider utils.UUIDProvider) { @@ -1109,7 +1107,7 @@ func (task *Task) initializeSSMSecretResource(credentialsManager credentials.Man apicontainerstatus.ContainerCreated) } - // Firelens container needs to depends on secret if other containers use secret log options. + // Firelens container needs to depend on secret if other containers use secret log options. if container.GetFirelensConfig() != nil && task.firelensDependsOnSecretResource(apicontainer.SecretProviderSSM) { container.BuildResourceDependency(ssmSecretResource.GetName(), resourcestatus.ResourceStatus(ssmsecret.SSMSecretCreated), @@ -1118,7 +1116,7 @@ func (task *Task) initializeSSMSecretResource(credentialsManager credentials.Man } } -// firelensDependsOnSecret checks whether the firelens container needs to depends on a secret resource of +// firelensDependsOnSecret checks whether the firelens container needs to depend on a secret resource of // a certain provider type. func (task *Task) firelensDependsOnSecretResource(secretProvider string) bool { isLogDriverSecretWithGivenProvider := func(s apicontainer.Secret) bool { @@ -1177,7 +1175,7 @@ func (task *Task) initializeASMSecretResource(credentialsManager credentials.Man apicontainerstatus.ContainerCreated) } - // Firelens container needs to depends on secret if other containers use secret log options. + // Firelens container needs to depend on secret if other containers use secret log options. if container.GetFirelensConfig() != nil && task.firelensDependsOnSecretResource(apicontainer.SecretProviderASM) { container.BuildResourceDependency(asmSecretResource.GetName(), resourcestatus.ResourceStatus(asmsecret.ASMSecretCreated), @@ -1306,7 +1304,7 @@ func (task *Task) addFirelensContainerDependency() error { if hostConfig.LogConfig.Type == firelensDriverName { // If there's no dependency between the app container and the firelens container, make firelens container - // start first to be the default behavior by adding a START container depdendency. + // start first to be the default behavior by adding a START container dependency. if !container.DependsOnContainer(firelensContainer.Name) { logger.Info("Adding a START container dependency on firelens for container", logger.Fields{ field.TaskID: task.GetID(), @@ -1354,7 +1352,7 @@ func (task *Task) collectFirelensLogOptions(containerToLogOptions map[string]map // collectFirelensLogEnvOptions collects all the log secret options. Each secret log option will have a value // of a config file variable (e.g. "${config_var_name}") and we will pass the secret value as env to the firelens -// container and it will resolve the config file variable from the env. +// container, and it will resolve the config file variable from the env. // Each config variable name has a format of log-option-key_container-name. We need the container name because options // from different containers using awsfirelens log driver in a task will be presented in the same firelens config file. func (task *Task) collectFirelensLogEnvOptions(containerToLogOptions map[string]map[string]string, firelensConfigType string) error { @@ -1668,7 +1666,7 @@ func (task *Task) updateTaskKnownStatus() (newStatus apitaskstatus.TaskStatus) { } } if earliestKnownStatusContainer == nil { - logger.Critical("Impossible state found while updating tasks's known status", logger.Fields{ + logger.Critical("Impossible state found while updating tasks' known status", logger.Fields{ field.TaskID: task.GetID(), "earliestKnownStatus": containerEarliestKnownStatus.String(), }) @@ -1691,7 +1689,7 @@ func (task *Task) updateTaskKnownStatus() (newStatus apitaskstatus.TaskStatus) { } // We can't rely on earliest container known status alone for determining if the // task state needs to be updated as containers can have different steady states - // defined. Instead we should get the task status for all containers' known + // defined. Instead, we should get the task status for all containers' known // statuses and compute the min of this earliestKnownTaskStatus := task.getEarliestKnownTaskStatusForContainers() if task.GetKnownStatus() < earliestKnownTaskStatus { @@ -1861,7 +1859,7 @@ func (task *Task) DockerHostConfig(container *apicontainer.Container, dockerCont } // ApplyExecutionRoleLogsAuth will check whether the task has execution role -// credentials, and add the genereated credentials endpoint to the associated HostConfig +// credentials, and add the generated credentials endpoint to the associated HostConfig func (task *Task) ApplyExecutionRoleLogsAuth(hostConfig *dockercontainer.HostConfig, credentialsManager credentials.Manager) *apierrors.HostConfigError { id := task.GetExecutionCredentialsID() if id == "" { @@ -2270,7 +2268,7 @@ func (task *Task) ipcModeOverride(container *apicontainer.Container, dockerConta } pauseDockerID, ok := dockerContainerMap[pauseCont.Name] if !ok || pauseDockerID == nil { - // Docker container shouldn't be nill or not exist if the Container definition within task exists; implies code-bug + // Docker container shouldn't be nil or not exist if the Container definition within task exists; implies code-bug logger.Critical("Namespace Pause container not found; stopping task", logger.Fields{ field.TaskID: task.GetID(), }) @@ -2318,7 +2316,7 @@ func (task *Task) initializeContainerOrdering() error { for _, link := range container.Links { linkParts := strings.Split(link, ":") if len(linkParts) > 2 { - return fmt.Errorf("Invalid link format") + return fmt.Errorf("invalid link format") } linkName := linkParts[0] if _, ok := task.ContainerByName(linkName); !ok { @@ -2385,7 +2383,7 @@ func (task *Task) buildPortMapWithSCIngressConfig(dynamicHostPortRange string) ( if ic.HostPort != nil { // For non-default bridge mode service connect experience, a host port is specified by customers // Note that service connect ingress config has been validated in service_connect_validator.go, - // where host ports will be validated to ensure user-definied ports are within a valid port range (1 to 65535) + // where host ports will be validated to ensure user-defined ports are within a valid port range (1 to 65535) // and do not have port collisions. hostPortStr = strconv.Itoa(int(*ic.HostPort)) } else { @@ -2413,7 +2411,7 @@ func (task *Task) buildPortMapWithSCIngressConfig(dynamicHostPortRange string) ( // (2) Port mapping configured by customers in the task definition. // // For service connect bridge mode task, we will create port bindings for customers' application containers -// and service connect AppNet container, and let them to be published by the associated pause containers. +// and service connect AppNet container, and let them be published by the associated pause containers. // (a) For default bridge service connect experience, ECS Agent will assign a host port within the // default/user-specified dynamic host port range for the ingress listener. If no available host port can be // found by ECS Agent, an error will be returned. @@ -2434,7 +2432,7 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo containerPortRangeMap := make(map[string]string) // For service connect bridge network mode task, we will create port bindings for task containers, - // including both application containers and service connect AppNet container, and let them to be published + // including both application containers and service connect AppNet container, and let them be published // by the associated pause containers. if task.IsServiceConnectEnabled() && task.IsNetworkModeBridge() { if container.Type == apicontainer.ContainerCNIPause { @@ -2464,7 +2462,7 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo } // If the associated task container to this pause container is NOT the service connect AppNet container, // we will continue to update the dockerPortMap for the pause container using the port bindings - // configured for the application container since port bindings will be published by the pasue container. + // configured for the application container since port bindings will be published by the pause container. containerToCheck = taskContainer } else { // If the container is not a pause container, then it is a regular customers' application container @@ -2487,9 +2485,9 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo dockerPort := nat.Port(strconv.Itoa(containerPort) + "/" + protocolStr) if portBinding.HostPort != 0 { - // An user-specified host port exists. + // A user-specified host port exists. // Note that the host port value has been validated by ECS front end service; - // thus only an valid host port value will be streamed down to ECS Agent. + // thus only a valid host port value will be streamed down to ECS Agent. hostPortStr = strconv.Itoa(int(portBinding.HostPort)) } else { // If there is no user-specified host port, ECS Agent will find an available host port @@ -2564,7 +2562,8 @@ func (task *Task) dockerPortMap(container *apicontainer.Container, dynamicHostPo return dockerPortMap, nil } -func (task *Task) dockerVolumesFrom(container *apicontainer.Container, dockerContainerMap map[string]*apicontainer.DockerContainer) ([]string, error) { +func (task *Task) dockerVolumesFrom(container *apicontainer.Container, + dockerContainerMap map[string]*apicontainer.DockerContainer) ([]string, error) { volumesFrom := make([]string, len(container.VolumesFrom)) for i, volume := range container.VolumesFrom { targetContainer, ok := dockerContainerMap[volume.SourceContainer] @@ -2616,9 +2615,8 @@ func (task *Task) dockerHostBinds(container *apicontainer.Container) ([]string, return binds, nil } -// UpdateStatus updates a task's known and desired statuses to be compatible -// with all of its containers -// It will return a bool indicating if there was a change +// UpdateStatus updates a task's known and desired statuses to be compatible with all of its containers. +// It will return a bool indicating if there was a change in the task's known status. func (task *Task) UpdateStatus() bool { change := task.updateTaskKnownStatus() if change != apitaskstatus.TaskStatusNone { @@ -2629,7 +2627,7 @@ func (task *Task) UpdateStatus() bool { return change != apitaskstatus.TaskStatusNone } -// UpdateDesiredStatus sets the known status of the task +// UpdateDesiredStatus sets the desired status of the task, and its containers and resources. func (task *Task) UpdateDesiredStatus() { task.lock.Lock() defer task.lock.Unlock() @@ -2651,26 +2649,26 @@ func (task *Task) updateTaskDesiredStatusUnsafe() { } if cont.Essential && (cont.KnownTerminal() || cont.DesiredTerminal()) { task.DesiredStatusUnsafe = apitaskstatus.TaskStopped - logger.Info("Essential container stopped; updated task desired status to stopped", task.fieldsUnsafe()) + logger.Info("Essential container stopped; updated task desired status to stopped", + task.fieldsUnsafe()) } } } -// updateContainerDesiredStatusUnsafe sets all container's desired status's to the -// task's desired status +// updateContainerDesiredStatusUnsafe sets all containers' desired statuses to the task's desired status // Invariant: container desired status is <= task desired status converted to container status // Note: task desired status and container desired status is typically only RUNNING or STOPPED func (task *Task) updateContainerDesiredStatusUnsafe(taskDesiredStatus apitaskstatus.TaskStatus) { for _, container := range task.Containers { - taskDesiredStatusToContainerStatus := apitaskstatus.MapTaskToContainerStatus(taskDesiredStatus, container.GetSteadyStateStatus()) + taskDesiredStatusToContainerStatus := apitaskstatus.MapTaskToContainerStatus(taskDesiredStatus, + container.GetSteadyStateStatus()) if container.GetDesiredStatus() < taskDesiredStatusToContainerStatus { container.SetDesiredStatus(taskDesiredStatusToContainerStatus) } } } -// updateResourceDesiredStatusUnsafe sets all resources' desired status depending on the -// task's desired status +// updateResourceDesiredStatusUnsafe sets all resources' desired status depending on the task's desired status // TODO: Create a mapping of resource status to the corresponding task status and use it here func (task *Task) updateResourceDesiredStatusUnsafe(taskDesiredStatus apitaskstatus.TaskStatus) { resources := task.getResourcesUnsafe() @@ -2910,12 +2908,12 @@ func (task *Task) GetExecutionStoppedAt() time.Time { return task.ExecutionStoppedAtUnsafe } -// String returns a human readable string representation of this object +// String returns a human-readable string representation of this object func (task *Task) String() string { return task.stringUnsafe() } -// stringUnsafe returns a human readable string representation of this object +// stringUnsafe returns a human-readable string representation of this object func (task *Task) stringUnsafe() string { return fmt.Sprintf("%s:%s %s, TaskStatus: (%s->%s) N Containers: %d, N ENIs %d", task.Family, task.Version, task.Arn, @@ -3030,7 +3028,7 @@ func (task *Task) GetCredentialSpecResource() ([]taskresource.TaskResource, bool return res, ok } -// getAllCredentialSpecRequirements is used to build all the credential spec requirements for the task +// GetAllCredentialSpecRequirements is used to build all the credential spec requirements for the task func (task *Task) GetAllCredentialSpecRequirements() map[string]string { reqsContainerMap := make(map[string]string) for _, container := range task.Containers { @@ -3300,7 +3298,7 @@ func (task *Task) getIPCMode() string { return task.IPCMode } -// AssociationByTypeAndContainer gets a list of names of all the associations associated with a container and of a +// AssociationsByTypeAndContainer gets a list of names of all the associations associated with a container and of a // certain type func (task *Task) AssociationsByTypeAndContainer(associationType, containerName string) []string { task.lock.RLock() @@ -3466,8 +3464,8 @@ func (task *Task) IsServiceConnectEnabled() bool { return task.GetServiceConnectContainer() != nil } -// Is EBS Task Attach enabled returns true if this task has EBS volume configuration in its ACS payload. -// TODO as more daemons come online, we'll want a generic handler these bool checks and payload handling +// IsEBSTaskAttachEnabled returns true if this task has EBS volume configuration in its ACS payload. +// TODO: as more daemons come online, we'll want a generic handler these bool checks and payload handling func (task *Task) IsEBSTaskAttachEnabled() bool { task.lock.RLock() defer task.lock.RUnlock() @@ -3523,7 +3521,7 @@ func (task *Task) PopulateServiceConnectRuntimeConfig(serviceConnectConfig servi task.ServiceConnectConfig.RuntimeConfig = serviceConnectConfig } -// PopulateServiceConnectPauseIPConfig is called once we've started SC pause container and retrieved its container IPs. +// PopulateServiceConnectNetworkConfig is called once we've started SC pause container and retrieved its container IPs. func (task *Task) PopulateServiceConnectNetworkConfig(ipv4Addr string, ipv6Addr string) { task.lock.Lock() defer task.lock.Unlock() @@ -3735,7 +3733,7 @@ func (task *Task) IsRunning() bool { return taskStatus == apitaskstatus.TaskRunning } -// Checks if the task has at least one container with a successfully +// HasAContainerWithResolvedDigest checks if the task has at least one container with a successfully // resolved image manifest digest. func (task *Task) HasAContainerWithResolvedDigest() bool { for _, c := range task.Containers { diff --git a/agent/api/task/task_test.go b/agent/api/task/task_test.go index c611cf449e1..d3ccaefd798 100644 --- a/agent/api/task/task_test.go +++ b/agent/api/task/task_test.go @@ -1528,12 +1528,13 @@ func TestPostUnmarshalTaskWithPIDSharing(t *testing.T) { seqNum := int64(42) task, err := TaskFromACS(&testTaskFromACS, &ecsacs.PayloadMessage{SeqNum: &seqNum}) - assert.Nil(t, err, "Should be able to handle acs task") + assert.NoError(t, err, "Should be able to handle acs task") assert.Equal(t, aTest.PIDMode, task.getPIDMode()) assert.Equal(t, aTest.IPCMode, task.getIPCMode()) assert.Equal(t, 2, len(task.Containers)) // before PostUnmarshalTask cfg := config.Config{} - task.PostUnmarshalTask(&cfg, nil, nil, nil, nil) + err = task.PostUnmarshalTask(&cfg, nil, nil, nil, nil) + assert.NoError(t, err) if aTest.ShouldProvision { assert.Equal(t, 3, len(task.Containers), "Namespace Pause Container should be created.") } else { @@ -1567,7 +1568,8 @@ func TestNamespaceProvisionDependencyAndHostConfig(t *testing.T) { assert.Equal(t, aTest.IPCMode, task.getIPCMode()) assert.Equal(t, 2, len(task.Containers)) // before PostUnmarshalTask cfg := config.Config{} - task.PostUnmarshalTask(&cfg, nil, nil, nil, nil) + err = task.PostUnmarshalTask(&cfg, nil, nil, nil, nil) + assert.NoError(t, err) if !aTest.ShouldProvision { assert.Equal(t, 2, len(task.Containers), "Namespace Pause Container should NOT be created.") docMaps := dockerMap(task) diff --git a/agent/api/testutils/task_equal_test.go b/agent/api/testutils/task_equal_test.go index ffb81ddba20..beebefa6745 100644 --- a/agent/api/testutils/task_equal_test.go +++ b/agent/api/testutils/task_equal_test.go @@ -53,8 +53,8 @@ func TestTaskEqual(t *testing.T) { for index, tc := range testCases { t.Run(fmt.Sprintf("index %d expected %t", index, tc.shouldBeEqual), func(t *testing.T) { assert.Equal(t, TasksEqual(&tc.lhs, &tc.rhs), tc.shouldBeEqual, "TasksEqual not working as expected. Check index failure.") - // Symetric - assert.Equal(t, TasksEqual(&tc.rhs, &tc.lhs), tc.shouldBeEqual, "Symetric equality check failed. Check index failure.") + // Symmetric + assert.Equal(t, TasksEqual(&tc.rhs, &tc.lhs), tc.shouldBeEqual, "Symmetric equality check failed. Check index failure.") }) } } diff --git a/agent/engine/common_integ_test.go b/agent/engine/common_integ_test.go index eeac803a941..4831a7cbb17 100644 --- a/agent/engine/common_integ_test.go +++ b/agent/engine/common_integ_test.go @@ -43,6 +43,7 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/credentials" "github.com/aws/amazon-ecs-agent/ecs-agent/ec2" "github.com/aws/amazon-ecs-agent/ecs-agent/eventstream" + log "github.com/cihub/seelog" "github.com/stretchr/testify/assert" ) diff --git a/agent/engine/common_test.go b/agent/engine/common_test.go index b8bf2d93113..b4123cfbea1 100644 --- a/agent/engine/common_test.go +++ b/agent/engine/common_test.go @@ -44,6 +44,7 @@ import ( apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status" "github.com/aws/amazon-ecs-agent/ecs-agent/credentials" mock_ttime "github.com/aws/amazon-ecs-agent/ecs-agent/utils/ttime/mocks" + "github.com/aws/aws-sdk-go/aws" "github.com/cihub/seelog" dockercontainer "github.com/docker/docker/api/types/container" diff --git a/agent/engine/data_test.go b/agent/engine/data_test.go index 064cd405f71..30adb3467a3 100644 --- a/agent/engine/data_test.go +++ b/agent/engine/data_test.go @@ -19,8 +19,6 @@ package engine import ( "testing" - ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface" - apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" apitask "github.com/aws/amazon-ecs-agent/agent/api/task" "github.com/aws/amazon-ecs-agent/agent/data" @@ -29,6 +27,8 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment" apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status" apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status" + ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) diff --git a/agent/engine/docker_image_manager.go b/agent/engine/docker_image_manager.go index a52f67558a1..4c981ab0b40 100644 --- a/agent/engine/docker_image_manager.go +++ b/agent/engine/docker_image_manager.go @@ -21,11 +21,6 @@ import ( "sync" "time" - "github.com/aws/amazon-ecs-agent/ecs-agent/logger" - "github.com/aws/amazon-ecs-agent/ecs-agent/logger/field" - - "github.com/docker/docker/api/types" - apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" "github.com/aws/amazon-ecs-agent/agent/config" "github.com/aws/amazon-ecs-agent/agent/data" @@ -33,6 +28,10 @@ import ( "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi" "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate" "github.com/aws/amazon-ecs-agent/agent/engine/image" + "github.com/aws/amazon-ecs-agent/ecs-agent/logger" + "github.com/aws/amazon-ecs-agent/ecs-agent/logger/field" + + "github.com/docker/docker/api/types" ) const ( diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index 1900e35f005..a94dcd5b901 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -1007,7 +1007,7 @@ func (engine *DockerTaskEngine) EmitTaskEvent(task *apitask.Task, reason string) // else and should exit quickly to allow AddTask to do more work. func (engine *DockerTaskEngine) startTask(task *apitask.Task) { // Create a channel that may be used to communicate with this task, survey - // what tasks need to be waited for for this one to start, and then spin off + // what tasks need to be waited for this one to start, and then spin off // a goroutine to oversee this task thisTask := engine.newManagedTask(task) @@ -1781,8 +1781,7 @@ func (engine *DockerTaskEngine) createContainer(task *apitask.Task, container *a // Resolve HostConfig // we have to do this in create, not start, because docker no longer handles - // merging create config with start hostconfig the same; e.g. memory limits - // get lost + // merging create config with start host-config the same; e.g. memory limits get lost dockerClientVersion, versionErr := client.APIVersion() if versionErr != nil { return dockerapi.DockerContainerMetadata{Error: CannotGetDockerClientVersionError{versionErr}} @@ -2671,13 +2670,13 @@ func (engine *DockerTaskEngine) removeContainer(task *apitask.Task, container *a func (engine *DockerTaskEngine) updateTaskUnsafe(task *apitask.Task, update *apitask.Task) { managedTask, ok := engine.managedTasks[task.Arn] if !ok { - logger.Critical("ACS message for a task we thought we managed, but don't! Aborting.", logger.Fields{ + logger.Critical("ACS message for a task we thought we managed, but don't! Aborting.", logger.Fields{ field.TaskARN: task.Arn, }) return } // Keep the lock because sequence numbers cannot be correct unless they are - // also read in the order addtask was called + // also read in the order AddTask was called // This does block the engine's ability to ingest any new events (including // stops for past tasks, ack!), but this is necessary for correctness updateDesiredStatus := update.GetDesiredStatus() @@ -2756,7 +2755,7 @@ func (engine *DockerTaskEngine) transitionFunctionMap() map[apicontainerstatus.C return engine.containerStatusToTransitionFunction } -type transitionApplyFunc (func(*apitask.Task, *apicontainer.Container) dockerapi.DockerContainerMetadata) +type transitionApplyFunc func(*apitask.Task, *apicontainer.Container) dockerapi.DockerContainerMetadata // State is a function primarily meant for testing usage; it is explicitly not // part of the TaskEngine interface and should not be relied upon. diff --git a/agent/engine/docker_task_engine_linux_test.go b/agent/engine/docker_task_engine_linux_test.go index abea4639bc6..7ca628e5d54 100644 --- a/agent/engine/docker_task_engine_linux_test.go +++ b/agent/engine/docker_task_engine_linux_test.go @@ -58,7 +58,6 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/credentials" "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/appmesh" ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/aws/aws-sdk-go/aws" cniTypesCurrent "github.com/containernetworking/cni/pkg/types/100" @@ -67,14 +66,14 @@ import ( "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/registry" "github.com/golang/mock/gomock" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) const ( - cgroupMountPath = "/sys/fs/cgroup" - + cgroupMountPath = "/sys/fs/cgroup" testTaskDefFamily = "testFamily" testTaskDefVersion = "1" containerNetNS = "none" @@ -1236,7 +1235,7 @@ func TestContainersWithServiceConnect_BridgeMode(t *testing.T) { // if we create a dockercontainer.Config.Healthcheck variable and marshal it, dockercontainer.Config.Env gets set to empty // and will later override the internal env vars that Agent populates for the container. - // In real world, the container env vars in task def are marshaled into container.Environment isntead of docker Config.Env. + // In real world, the container env vars in task def are marshaled into container.Environment instead of docker Config.Env. // it gets merged with internal env vars, and eventually get assigned to docker Config.Env healthCheckString := "{\"Healthcheck\":{\"Test\":[\"echo\",\"ok\"],\"Interval\":1000000,\"Timeout\":1000000000,\"Retries\":1}}" sleepTask.Containers = append(sleepTask.Containers, &apicontainer.Container{ @@ -1489,15 +1488,15 @@ func TestWatchAppNetImage(t *testing.T) { ctrl, _, _, taskEngine, _, _, _, serviceConnectManager := mocks(t, ctx, &defaultConfig) defer ctrl.Finish() - tempServiceConnectAppnetAgenTarballDir := t.TempDir() + tempServiceConnectAppnetAgentTarballDir := t.TempDir() - serviceConnectManager.EXPECT().GetAppnetContainerTarballDir().Return(tempServiceConnectAppnetAgenTarballDir).AnyTimes() + serviceConnectManager.EXPECT().GetAppnetContainerTarballDir().Return(tempServiceConnectAppnetAgentTarballDir).AnyTimes() serviceConnectManager.EXPECT().LoadImage(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() watcherCtx, watcherCancel := context.WithTimeout(context.Background(), time.Second) defer watcherCancel() go taskEngine.(*DockerTaskEngine).watchAppNetImage(watcherCtx) - _, err := os.CreateTemp(tempServiceConnectAppnetAgenTarballDir, "agent.tar") + _, err := os.CreateTemp(tempServiceConnectAppnetAgentTarballDir, "agent.tar") assert.NoError(t, err) <-watcherCtx.Done() diff --git a/agent/engine/docker_task_engine_test.go b/agent/engine/docker_task_engine_test.go index f242b43d8fd..97937a3f83b 100644 --- a/agent/engine/docker_task_engine_test.go +++ b/agent/engine/docker_task_engine_test.go @@ -67,8 +67,6 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/appmesh" ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface" mock_ttime "github.com/aws/amazon-ecs-agent/ecs-agent/utils/ttime/mocks" - "github.com/opencontainers/go-digest" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/secretsmanager" @@ -79,6 +77,8 @@ import ( "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/registry" "github.com/golang/mock/gomock" + "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pborman/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -92,7 +92,6 @@ const ( ipv6 = "f0:234:23" dockerContainerName = "docker-container-name" containerPid = 123 - containerPid2 = 456 taskIP = "169.254.170.3" exitCode = 1 labelsTaskARN = "arn:aws:ecs:us-east-1:012345678910:task/c09f0188-7f87-4b0f-bfc3-16296622b6fe" @@ -920,7 +919,7 @@ func TestTaskTransitionWhenStopContainerReturnsUnretriableError(t *testing.T) { // If there's a delay in task engine's processing of the ContainerRunning // event, StopContainer will be invoked again as the engine considers it // as a stopped container coming back. MinTimes() should guarantee that - // StopContainer is invoked at least once and in protecting agasint a test + // StopContainer is invoked at least once and in protecting against a test // failure when there's a delay in task engine processing the ContainerRunning // event. client.EXPECT().StopContainer(gomock.Any(), containerID, gomock.Any()).Return(dockerapi.DockerContainerMetadata{ @@ -2090,7 +2089,7 @@ func TestNewTaskTransitionOnRestart(t *testing.T) { testTask.SetDesiredStatus(apitaskstatus.TaskStopped) dockerTaskEngine.synchronizeState() _, ok := dockerTaskEngine.managedTasks[testTask.Arn] - assert.True(t, ok, "task wasnot started") + assert.True(t, ok, "task was not started") } // TestPullStartedStoppedAtWasSetCorrectly tests the PullStartedAt and PullStoppedAt @@ -2176,7 +2175,7 @@ func TestPullStoppedAtWasSetCorrectlyWhenPullFail(t *testing.T) { mockTime.EXPECT().Now().Return(startTime2), mockTime.EXPECT().Now().Return(startTime3), - // threre container pull stop timestamp + // three container pull stop timestamp mockTime.EXPECT().Now().Return(stopTime1), mockTime.EXPECT().Now().Return(stopTime2), mockTime.EXPECT().Now().Return(stopTime3), @@ -2634,7 +2633,7 @@ func TestSynchronizeResource(t *testing.T) { cgroupResource.EXPECT().GetName().AnyTimes().Return("cgroup") cgroupResource.EXPECT().StatusString(gomock.Any()).AnyTimes() - // Set the task to be stopped so that the process can done quickly + // Set the task to be stopped so that the process can be done quickly testTask.SetDesiredStatus(apitaskstatus.TaskStopped) dockerTaskEngine.synchronizeState() } @@ -2936,7 +2935,7 @@ func TestTaskSecretsEnvironmentVariables(t *testing.T) { client.EXPECT().APIVersion().Return(defaultDockerClientAPIVersion, nil).AnyTimes() // test validates that the expectedConfig includes secrets are appended as - // environment varibles + // environment variables client.EXPECT().CreateContainer(gomock.Any(), expectedConfig, gomock.Any(), gomock.Any(), gomock.Any()) ret := taskEngine.(*DockerTaskEngine).createContainer(testTask, testTask.Containers[0]) assert.Nil(t, ret.Error) diff --git a/agent/engine/docker_task_engine_windows_test.go b/agent/engine/docker_task_engine_windows_test.go index 85f826b9cb6..248b5df8a97 100644 --- a/agent/engine/docker_task_engine_windows_test.go +++ b/agent/engine/docker_task_engine_windows_test.go @@ -24,11 +24,10 @@ import ( "testing" "time" - mock_asm_factory "github.com/aws/amazon-ecs-agent/agent/asm/factory/mocks" - "github.com/aws/amazon-ecs-agent/agent/dockerclient" - apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" apitask "github.com/aws/amazon-ecs-agent/agent/api/task" + mock_asm_factory "github.com/aws/amazon-ecs-agent/agent/asm/factory/mocks" + "github.com/aws/amazon-ecs-agent/agent/dockerclient" "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi" mock_dockerapi "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi/mocks" "github.com/aws/amazon-ecs-agent/agent/ecscni" @@ -42,6 +41,7 @@ import ( apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status" apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status" "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/appmesh" + "github.com/aws/aws-sdk-go/aws" "github.com/docker/docker/api/types" dockercontainer "github.com/docker/docker/api/types/container" diff --git a/agent/engine/engine_integ_test.go b/agent/engine/engine_integ_test.go index 7851fb67bca..1c5b46f754c 100644 --- a/agent/engine/engine_integ_test.go +++ b/agent/engine/engine_integ_test.go @@ -38,6 +38,7 @@ import ( apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status" apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status" "github.com/aws/amazon-ecs-agent/ecs-agent/credentials" + "github.com/aws/aws-sdk-go/aws" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" diff --git a/agent/engine/engine_sudo_linux_integ_test.go b/agent/engine/engine_sudo_linux_integ_test.go index 6a41960eafb..0f773b4114e 100644 --- a/agent/engine/engine_sudo_linux_integ_test.go +++ b/agent/engine/engine_sudo_linux_integ_test.go @@ -33,20 +33,6 @@ import ( "testing" "time" - "github.com/cihub/seelog" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/cloudwatchlogs" - "github.com/docker/docker/api/types" - dockercontainer "github.com/docker/docker/api/types/container" - sdkClient "github.com/docker/docker/client" - "github.com/pborman/uuid" - "github.com/pkg/errors" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" apitask "github.com/aws/amazon-ecs-agent/agent/api/task" "github.com/aws/amazon-ecs-agent/agent/config" @@ -69,6 +55,19 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/credentials" "github.com/aws/amazon-ecs-agent/ecs-agent/ec2" "github.com/aws/amazon-ecs-agent/ecs-agent/eventstream" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/service/cloudwatchlogs" + "github.com/cihub/seelog" + "github.com/docker/docker/api/types" + dockercontainer "github.com/docker/docker/api/types/container" + sdkClient "github.com/docker/docker/client" + "github.com/pborman/uuid" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ( @@ -77,15 +76,13 @@ var ( const ( testLogSenderImage = "public.ecr.aws/amazonlinux/amazonlinux:2.0.20210126.0" - testFluentbitImage = "public.ecr.aws/aws-observability/aws-for-fluent-bit:2.10.1" + testFluentBitImage = "public.ecr.aws/aws-observability/aws-for-fluent-bit:2.10.1" testVolumeImage = "127.0.0.1:51670/amazon/amazon-ecs-volumes-test:latest" testCluster = "testCluster" validTaskArnPrefix = "arn:aws:ecs:region:account-id:task/" testDataDir = "/var/lib/ecs/data/" testDataDirOnHost = "/var/lib/ecs/" testInstanceID = "testInstanceID" - testTaskDefFamily = "testFamily" - testTaskDefVersion = "1" testECSRegion = "us-east-1" testLogGroupName = "test-fluentbit" testLogGroupPrefix = "firelens-fluentbit-" @@ -362,7 +359,7 @@ func createFirelensTask(t *testing.T) *apitask.Task { }, { Name: "firelens", - Image: testFluentbitImage, + Image: testFluentBitImage, Essential: true, FirelensConfig: &apicontainer.FirelensConfig{ Type: firelens.FirelensConfigTypeFluentbit, diff --git a/agent/engine/engine_unix_integ_test.go b/agent/engine/engine_unix_integ_test.go index e6f743051e7..a0a252fca3f 100644 --- a/agent/engine/engine_unix_integ_test.go +++ b/agent/engine/engine_unix_integ_test.go @@ -31,9 +31,6 @@ import ( "testing" "time" - "github.com/cihub/seelog" - "github.com/docker/docker/api/types" - "github.com/aws/amazon-ecs-agent/agent/api" apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" apitask "github.com/aws/amazon-ecs-agent/agent/api/task" @@ -47,9 +44,11 @@ import ( apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status" apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status" "github.com/aws/amazon-ecs-agent/ecs-agent/utils/ttime" - "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws" + "github.com/cihub/seelog" "github.com/containerd/cgroups/v3" + "github.com/docker/docker/api/types" sdkClient "github.com/docker/docker/client" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -58,21 +57,15 @@ import ( const ( testRegistryHost = "127.0.0.1:51670" testBusyboxImage = testRegistryHost + "/busybox:latest" - testVolumeImage = testRegistryHost + "/amazon/amazon-ecs-volumes-test:latest" testFluentdImage = testRegistryHost + "/amazon/fluentd:latest" ) -var ( - endpoint = utils.DefaultIfBlank(os.Getenv(DockerEndpointEnvVariable), DockerDefaultEndpoint) - TestGPUInstanceType = []string{"p2", "p3", "g3", "g4dn", "p4d"} -) - // Starting from Docker version 20.10.6, a behavioral change was introduced in docker container port bindings, // where both IPv4 and IPv6 port bindings will be exposed. // To mitigate this issue, Agent introduced an environment variable ECS_EXCLUDE_IPV6_PORTBINDING, // which is true by default in the [PR#3025](https://github.com/aws/amazon-ecs-agent/pull/3025). // However, the PR does not modify port bindings in the container state change object, it only filters out IPv6 port -// bindings while building the container state change payload. Thus the invalid IPv6 port bindings still exists +// bindings while building the container state change payload. Thus, the invalid IPv6 port bindings still exists // in ContainerStateChange, and need to be filtered out in this integration test. // // The getValidPortBinding function and the ECS_EXCLUDE_IPV6_PORTBINDING environment variable should be removed once @@ -574,7 +567,7 @@ func TestMultipleDynamicPortForward(t *testing.T) { // TestLinking ensures that container linking does allow networking to go // through to a linked container. this test specifically starts a server that // prints "hello linker" and then links a container that proxies that data to -// a publicly exposed port, where the tests reads it +// a publicly exposed port, where the tests read it func TestLinking(t *testing.T) { taskEngine, done, _ := setupWithDefaultConfig(t) defer done() diff --git a/agent/engine/host_resource_manager.go b/agent/engine/host_resource_manager.go index a6a94efde6a..bd00c164303 100644 --- a/agent/engine/host_resource_manager.go +++ b/agent/engine/host_resource_manager.go @@ -23,6 +23,7 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/api/ecs/model/ecs" "github.com/aws/amazon-ecs-agent/ecs-agent/logger" "github.com/aws/amazon-ecs-agent/ecs-agent/logger/field" + "github.com/aws/aws-sdk-go/aws" ) diff --git a/agent/engine/host_resource_manager_test.go b/agent/engine/host_resource_manager_test.go index 3b32d29f437..21715386333 100644 --- a/agent/engine/host_resource_manager_test.go +++ b/agent/engine/host_resource_manager_test.go @@ -22,6 +22,7 @@ import ( "github.com/aws/amazon-ecs-agent/agent/utils" "github.com/aws/amazon-ecs-agent/ecs-agent/api/ecs/model/ecs" commonutils "github.com/aws/amazon-ecs-agent/ecs-agent/utils" + "github.com/aws/aws-sdk-go/aws" "github.com/stretchr/testify/assert" ) diff --git a/agent/engine/interface.go b/agent/engine/interface.go index 70d83f9d671..50f66ceb1f3 100644 --- a/agent/engine/interface.go +++ b/agent/engine/interface.go @@ -14,9 +14,8 @@ package engine import ( - "encoding/json" - "context" + "encoding/json" apitask "github.com/aws/amazon-ecs-agent/agent/api/task" "github.com/aws/amazon-ecs-agent/agent/data" diff --git a/agent/engine/ordering_integ_test.go b/agent/engine/ordering_integ_test.go index d4a2e008f56..e8f472fa466 100644 --- a/agent/engine/ordering_integ_test.go +++ b/agent/engine/ordering_integ_test.go @@ -24,6 +24,7 @@ import ( apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" "github.com/aws/amazon-ecs-agent/agent/config" "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status" + "github.com/aws/aws-sdk-go/aws" "github.com/stretchr/testify/assert" ) diff --git a/agent/engine/task_manager.go b/agent/engine/task_manager.go index d6565097d4a..848c099b313 100644 --- a/agent/engine/task_manager.go +++ b/agent/engine/task_manager.go @@ -122,7 +122,7 @@ type resourceTransition struct { // This design is chosen to allow a safe level if isolation and avoid any race // conditions around the state of a task. // The data sources (e.g. docker, acs) that write to the task's channels may -// block and it is expected that the managedTask listen to those channels +// block, and it is expected that the managedTask listen to those channels // almost constantly. // The general operation should be: // 1. Listen to the channels @@ -166,7 +166,7 @@ type managedTask struct { _timeOnce sync.Once // steadyStatePollInterval is the duration that a managed task waits - // once the task gets into steady state before polling the state of all of + // once the task gets into steady state before polling the state of all // the task's containers to re-evaluate if the task is still in steady state // This is set to defaultTaskSteadyStatePollInterval in production code. // This can be used by tests that are looking to ensure that the steady state @@ -206,13 +206,13 @@ func (engine *DockerTaskEngine) newManagedTask(task *apitask.Task) *managedTask // loop of receiving messages and attempting to take action based on those // messages. func (mtask *managedTask) overseeTask() { - // Do a single updatestatus at the beginning to create the container - // `desiredstatus`es which are a construct of the engine used only here, + // Do a single UpdateStatus at the beginning to create the container + // `DesiredStatus`es which are a construct of the engine used only here, // not present on the backend mtask.UpdateStatus() // Wait here until enough resources are available on host for the task to progress - // - Waits until host resource manager succesfully 'consume's task resources and returns + // - Waits until host resource manager successfully 'consume's task resources and returns // - For tasks which have crossed this stage before (on agent restarts), resources are pre-consumed - returns immediately // - If the task is already stopped (knownStatus is STOPPED), does not attempt to consume resources - returns immediately // - If an ACS StopTask arrives, host resources manager returns immediately. Host resource manager does not consume resources @@ -238,7 +238,7 @@ func (mtask *managedTask) overseeTask() { } if !mtask.GetKnownStatus().Terminal() { - // If we aren't terminal and we aren't steady state, we should be + // If we aren't terminal, and we aren't steady state, we should be // able to move some containers along. logger.Debug("Task not steady state or terminal; progressing it", logger.Fields{ field.TaskID: mtask.GetID(), @@ -265,7 +265,8 @@ func (mtask *managedTask) overseeTask() { go mtask.releaseIPInIPAM() if mtask.Task.IsEBSTaskAttachEnabled() { - csiClient := csiclient.NewCSIClient(filepath.Join(csiclient.DefaultSocketHostPath, csiclient.DefaultImageName, csiclient.DefaultSocketName)) + csiClient := csiclient.NewCSIClient(filepath.Join(csiclient.DefaultSocketHostPath, csiclient.DefaultImageName, + csiclient.DefaultSocketName)) errors := mtask.UnstageVolumes(&csiClient) for _, err := range errors { logger.Error(fmt.Sprintf("Unable to unstage volumes: %v", err)) @@ -299,7 +300,7 @@ func (mtask *managedTask) emitCurrentStatus() { // channel from monitorQueuedTasks routine to wake up func (mtask *managedTask) waitForHostResources() { if mtask.GetKnownStatus().Terminal() { - // Task's known status is STOPPED. No need to wait in this case and proceed to cleanup + // Task's known status is STOPPED. No need to wait in this case and proceed to clean up // This is relevant when agent restarts and a task has stopped - do not attempt // to consume resources in host resource manager return @@ -328,7 +329,8 @@ func (mtask *managedTask) waitSteady() { field.KnownStatus: mtask.GetKnownStatus().String(), }) - timeoutCtx, cancel := context.WithTimeout(mtask.ctx, retry.AddJitter(mtask.steadyStatePollInterval, mtask.steadyStatePollIntervalJitter)) + timeoutCtx, cancel := context.WithTimeout(mtask.ctx, retry.AddJitter(mtask.steadyStatePollInterval, + mtask.steadyStatePollIntervalJitter)) defer cancel() timedOut := mtask.waitEvent(timeoutCtx.Done()) if mtask.shouldExit() { @@ -454,7 +456,8 @@ func (mtask *managedTask) handleContainerChange(containerChange dockerContainerC } if event.Status.String() == apicontainerstatus.ContainerStatusNone.String() || - (event.Status == apicontainerstatus.ContainerRunning && containerKnownStatus == apicontainerstatus.ContainerResourcesProvisioned) { + (event.Status == apicontainerstatus.ContainerRunning && + containerKnownStatus == apicontainerstatus.ContainerResourcesProvisioned) { logger.Debug("Handling container change event", eventLogFields) } else if event.Status != containerKnownStatus { // This prevents noisy docker event logs for pause container logger.Info("Handling container change event", eventLogFields) @@ -466,7 +469,7 @@ func (mtask *managedTask) handleContainerChange(containerChange dockerContainerC } // If this is a backwards transition stopped->running, the first time set it - // to be known running so it will be stopped. Subsequently ignore these backward transitions + // to be known running so it will be stopped. Subsequently, ignore these backward transitions mtask.handleStoppedToRunningContainerTransition(event.Status, container) if event.Status <= containerKnownStatus { logger.Debug("Container change is redundant", eventLogFields) @@ -485,7 +488,8 @@ func (mtask *managedTask) handleContainerChange(containerChange dockerContainerC // the stop workflow and restart the container. if event.Status == apicontainerstatus.ContainerStopped && container.RestartPolicyEnabled() { exitCode := event.DockerContainerMetadata.ExitCode - shouldRestart, reason := container.RestartTracker.ShouldRestart(exitCode, container.GetStartedAt(), container.GetDesiredStatus()) + shouldRestart, reason := container.RestartTracker.ShouldRestart(exitCode, container.GetStartedAt(), + container.GetDesiredStatus()) if shouldRestart { container.RestartTracker.RecordRestart() resp := mtask.engine.startContainer(mtask.Task, container) @@ -495,7 +499,7 @@ func (mtask *managedTask) handleContainerChange(containerChange dockerContainerC "restartCount": container.RestartTracker.GetRestartCount(), "lastRestartAt": container.RestartTracker.GetLastRestartAt().UTC().Format(time.RFC3339), }) - // return here because we have now restarted the container and we don't + // return here because we have now restarted the container, and we don't // want to complete the rest of the "container stop" workflow return } @@ -650,7 +654,8 @@ func (mtask *managedTask) emitTaskEvent(task *apitask.Task, reason string) { resourcesToRelease := mtask.ToHostResources() err := mtask.engine.hostResourceManager.release(mtask.Arn, resourcesToRelease) if err != nil { - logger.Critical("Failed to release resources after tast stopped", logger.Fields{field.TaskARN: mtask.Arn}) + logger.Critical("Failed to release resources after task stopped", + logger.Fields{field.TaskARN: mtask.Arn}) } } if taskKnownStatus != apitaskstatus.TaskManifestPulled && !taskKnownStatus.BackendRecognized() { @@ -699,7 +704,8 @@ func (mtask *managedTask) emitTaskEvent(task *apitask.Task, reason string) { // emitManagedAgentEvent passes a special task event up through the taskEvents channel if there are managed // agent changes in the container passed as parameter. // It will omit events the backend would not process -func (mtask *managedTask) emitManagedAgentEvent(task *apitask.Task, cont *apicontainer.Container, managedAgentName string, reason string) { +func (mtask *managedTask) emitManagedAgentEvent(task *apitask.Task, cont *apicontainer.Container, + managedAgentName string, reason string) { event, err := api.NewManagedAgentChangeEvent(task, cont, managedAgentName, reason) if err != nil { logger.Error("Skipping emitting ManagedAgent event for task", logger.Fields{ @@ -755,9 +761,10 @@ func (mtask *managedTask) doEmitContainerEvent(event api.ContainerStateChange) { }) select { case <-mtask.ctx.Done(): - logger.Info("Unable to send container change event due to exit", getContainerEventLogFields(event), logger.Fields{ - field.TaskID: mtask.GetID(), - }) + logger.Info("Unable to send container change event due to exit", getContainerEventLogFields(event), + logger.Fields{ + field.TaskID: mtask.GetID(), + }) case mtask.stateChangeEvents <- event: } logger.Debug("Sent container change event", getContainerEventLogFields(event), logger.Fields{ @@ -837,7 +844,8 @@ func (mtask *managedTask) releaseIPInIPAM() { // handleStoppedToRunningContainerTransition detects a "backwards" container // transition where a known-stopped container is found to be running again and // handles it. -func (mtask *managedTask) handleStoppedToRunningContainerTransition(status apicontainerstatus.ContainerStatus, container *apicontainer.Container) { +func (mtask *managedTask) handleStoppedToRunningContainerTransition(status apicontainerstatus.ContainerStatus, + container *apicontainer.Container) { containerKnownStatus := container.GetKnownStatus() if status > containerKnownStatus { // Event status is greater than container's known status. @@ -854,7 +862,7 @@ func (mtask *managedTask) handleStoppedToRunningContainerTransition(status apico return } // If the container becomes running after we've stopped it (possibly - // because we got an error running it and it ran anyways), the first time + // because we got an error running it, and it ran anyway), the first time // update it to 'known running' so that it will be driven back to stopped mtask.unexpectedStart.Do(func() { logger.Warn("Stopped container came back; re-stopping it once", logger.Fields{ @@ -869,7 +877,7 @@ func (mtask *managedTask) handleStoppedToRunningContainerTransition(status apico // handleManagedAgentStoppedTransition handles a container change event which has a managed agent status // we should emit ManagedAgent events for certain container events. func (mtask *managedTask) handleManagedAgentStoppedTransition(container *apicontainer.Container, managedAgentName string) { - //for now we only have the ExecuteCommandAgent + //for now, we only have the ExecuteCommandAgent switch managedAgentName { case execcmd.ExecuteCommandAgentName: if !container.UpdateManagedAgentStatus(managedAgentName, apicontainerstatus.ManagedAgentStopped) { @@ -882,17 +890,19 @@ func (mtask *managedTask) handleManagedAgentStoppedTransition(container *apicont } mtask.emitManagedAgentEvent(mtask.Task, container, managedAgentName, "Received Container Stopped event") default: - logger.Warn("Unexpected ManagedAgent in container; unable to process ManagedAgent transition event", logger.Fields{ - field.TaskID: mtask.GetID(), - field.Container: container.Name, - field.ManagedAgent: managedAgentName, - }) + logger.Warn("Unexpected ManagedAgent in container; unable to process ManagedAgent transition event", + logger.Fields{ + field.TaskID: mtask.GetID(), + field.Container: container.Name, + field.ManagedAgent: managedAgentName, + }) } } // handleEventError handles a container change event error and decides whether // we should proceed to transition the container -func (mtask *managedTask) handleEventError(containerChange dockerContainerChange, currentKnownStatus apicontainerstatus.ContainerStatus) bool { +func (mtask *managedTask) handleEventError(containerChange dockerContainerChange, + currentKnownStatus apicontainerstatus.ContainerStatus) bool { container := containerChange.container event := containerChange.event if container.ApplyingError == nil { @@ -996,14 +1006,15 @@ func (mtask *managedTask) handleContainerStoppedTransitionError(event dockerapi. pr := mtask.dockerClient.SystemPing(mtask.ctx, systemPingTimeout) if pr.Error != nil { - logger.Info("Error stopping the container, but docker seems to be unresponsive; ignoring state change", logger.Fields{ - field.TaskID: mtask.GetID(), - field.Container: container.Name, - field.RuntimeID: container.GetRuntimeID(), - "ErrorName": event.Error.ErrorName(), - field.Error: event.Error.Error(), - "SystemPingError": pr.Error, - }) + logger.Info("Error stopping the container, but docker seems to be unresponsive; ignoring state change", + logger.Fields{ + field.TaskID: mtask.GetID(), + field.Container: container.Name, + field.RuntimeID: container.GetRuntimeID(), + "ErrorName": event.Error.ErrorName(), + field.Error: event.Error.Error(), + "SystemPingError": pr.Error, + }) container.SetKnownStatus(currentKnownStatus) return false } @@ -1037,7 +1048,7 @@ func (mtask *managedTask) progressTask() { field.TaskID: mtask.GetID(), }) // max number of transitions length to ensure writes will never block on - // these and if we exit early transitions can exit the goroutine and it'll + // these and if we exit early transitions can exit the goroutine, and it'll // get GC'd eventually resources := mtask.GetResources() transitionChange := make(chan struct{}, len(mtask.Containers)+len(resources)) @@ -1066,7 +1077,7 @@ func (mtask *managedTask) progressTask() { blockedByOrderingDependencies := len(blockedDependencies) > 0 - // If no transitions happened and we aren't blocked by ordering dependencies, then we are possibly in a state where + // If no transitions happened, and we aren't blocked by ordering dependencies, then we are possibly in a state where // its impossible for containers to move forward. We will do an additional check to see if we are waiting for ACS // execution credentials. If not, then we will abort the task progression. if !atLeastOneTransitionStarted && !blockedByOrderingDependencies { @@ -1098,9 +1109,8 @@ func (mtask *managedTask) progressTask() { transitions[k] = v.String() } - // We've kicked off one or more transitions, wait for them to - // complete, but keep reading events as we do. in fact, we have to for - // transitions to complete + // We've kicked off one or more transitions, wait for them to complete, but keep reading events as we do. + // In fact, we have to for transitions to complete. mtask.waitForTransition(transitions, transitionChange, transitionChangeEntity) // update the task status if mtask.UpdateStatus() { @@ -1143,7 +1153,8 @@ func (mtask *managedTask) isWaitingForACSExecutionCredentials(reasons []error) b // startContainerTransitions steps through each container in the task and calls // the passed transition function when a transition should occur. -func (mtask *managedTask) startContainerTransitions(transitionFunc containerTransitionFunc) (bool, map[string]apicontainer.DependsOn, map[string]apicontainerstatus.ContainerStatus, []error) { +func (mtask *managedTask) startContainerTransitions(transitionFunc containerTransitionFunc) (bool, + map[string]apicontainer.DependsOn, map[string]apicontainerstatus.ContainerStatus, []error) { anyCanTransition := false var reasons []error blocked := make(map[string]apicontainer.DependsOn) @@ -1164,7 +1175,7 @@ func (mtask *managedTask) startContainerTransitions(transitionFunc containerTran // If the container is already in a transition, skip if transition.actionRequired && !cont.SetAppliedStatus(transition.nextState) { - // At least one container is able to be moved forwards, so we're not deadlocked + // At least one container is able to be moved forward, so we're not deadlocked anyCanTransition = true continue } @@ -1194,7 +1205,8 @@ func (mtask *managedTask) startContainerTransitions(transitionFunc containerTran return anyCanTransition, blocked, transitions, reasons } -func (mtask *managedTask) handleTerminalDependencyError(container *apicontainer.Container, error dependencygraph.DependencyError) { +func (mtask *managedTask) handleTerminalDependencyError(container *apicontainer.Container, + error dependencygraph.DependencyError) { logger.Error("Terminal error detected during transition; marking container as stopped", logger.Fields{ field.Container: container.Name, field.Error: error.Error(), @@ -1312,7 +1324,7 @@ type resourceTransitionFunc func(resource taskresource.TaskResource, nextStatus // * an error indicating why this transition can't happen // // 'Stopped, false, ""' -> "You can move it to known stopped, but you don't have to call a transition function" -// 'Running, true, ""' -> "You can move it to running and you need to call the transition function" +// 'Running, true, ""' -> "You can move it to running, and you need to call the transition function" // 'None, false, containerDependencyNotResolved' -> "This should not be moved; it has unresolved dependencies;" // // Next status is determined by whether the known and desired statuses are @@ -1483,9 +1495,10 @@ func (mtask *managedTask) handleContainersUnableToTransitionState() { } if stopTask { - logger.Critical("Task in a bad state; it's not steady state but no containers want to transition", logger.Fields{ - field.TaskID: mtask.GetID(), - }) + logger.Critical("Task in a bad state; it's not steady state but no containers want to transition", + logger.Fields{ + field.TaskID: mtask.GetID(), + }) mtask.handleDesiredStatusChange(apitaskstatus.TaskStopped, 0) } } diff --git a/agent/engine/task_manager_test.go b/agent/engine/task_manager_test.go index 5ce53f955dd..afa895329cd 100644 --- a/agent/engine/task_manager_test.go +++ b/agent/engine/task_manager_test.go @@ -24,18 +24,12 @@ import ( "testing" "time" - "github.com/aws/aws-sdk-go/aws" - - "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi" - "github.com/aws/amazon-ecs-agent/agent/taskresource" - mock_taskresource "github.com/aws/amazon-ecs-agent/agent/taskresource/mocks" - resourcestatus "github.com/aws/amazon-ecs-agent/agent/taskresource/status" - "github.com/aws/amazon-ecs-agent/agent/api" apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" 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/data" + "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi" mock_dockerapi "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi/mocks" "github.com/aws/amazon-ecs-agent/agent/engine/dependencygraph" mock_dockerstate "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate/mocks" @@ -43,6 +37,9 @@ import ( "github.com/aws/amazon-ecs-agent/agent/engine/testdata" "github.com/aws/amazon-ecs-agent/agent/sighandlers/exitcodes" "github.com/aws/amazon-ecs-agent/agent/statechange" + "github.com/aws/amazon-ecs-agent/agent/taskresource" + mock_taskresource "github.com/aws/amazon-ecs-agent/agent/taskresource/mocks" + resourcestatus "github.com/aws/amazon-ecs-agent/agent/taskresource/status" "github.com/aws/amazon-ecs-agent/agent/taskresource/volume" taskresourcevolume "github.com/aws/amazon-ecs-agent/agent/taskresource/volume" apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachment/resource" @@ -56,9 +53,10 @@ import ( "github.com/aws/amazon-ecs-agent/ecs-agent/eventstream" ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface" mock_ttime "github.com/aws/amazon-ecs-agent/ecs-agent/utils/ttime/mocks" - "github.com/stretchr/testify/assert" + "github.com/aws/aws-sdk-go/aws" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" ) func TestHandleEventError(t *testing.T) { @@ -144,7 +142,7 @@ func TestHandleEventError(t *testing.T) { ExpectedOK: true, }, { - Name: "Container vanished betweeen pull and running", + Name: "Container vanished between pull and running", EventStatus: apicontainerstatus.ContainerRunning, CurrentContainerKnownStatus: apicontainerstatus.ContainerPulled, Error: &ContainerVanishedError{}, @@ -282,7 +280,7 @@ func TestContainerNextState(t *testing.T) { // The expected next status is MANIFEST_PULLED {apicontainerstatus.ContainerStatusNone, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerManifestPulled, true, nil}, // NONE -> RESOURCES_PROVISIONED transition is allowed and actionable, when desired - // is Running. The exptected next status is MANIFEST_PULLED + // is Running. The expected next status is MANIFEST_PULLED {apicontainerstatus.ContainerStatusNone, apicontainerstatus.ContainerResourcesProvisioned, apicontainerstatus.ContainerManifestPulled, true, nil}, // NONE -> NONE transition is not be allowed and is not actionable, // when desired is Running @@ -301,10 +299,10 @@ func TestContainerNextState(t *testing.T) { // MANIFEST_PULLED -> STOPPED transition will result in STOPPED and is allowed, but not actionable {apicontainerstatus.ContainerManifestPulled, apicontainerstatus.ContainerStopped, apicontainerstatus.ContainerStopped, false, nil}, // PULLED -> RUNNING transition is allowed and actionable, when desired is Running - // The exptected next status is Created + // The expected next status is Created {apicontainerstatus.ContainerPulled, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerCreated, true, nil}, // PULLED -> RESOURCES_PROVISIONED transition is allowed and actionable, when desired - // is Running. The exptected next status is Created + // is Running. The expected next status is Created {apicontainerstatus.ContainerPulled, apicontainerstatus.ContainerResourcesProvisioned, apicontainerstatus.ContainerCreated, true, nil}, // PULLED -> PULLED transition is not allowed and not actionable, // when desired is Running @@ -319,7 +317,7 @@ func TestContainerNextState(t *testing.T) { // The expected next status is Running {apicontainerstatus.ContainerCreated, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning, true, nil}, // CREATED -> RESOURCES_PROVISIONED transition is allowed and actionable, when desired - // is Running. The exptected next status is Running + // is Running. The expected next status is Running {apicontainerstatus.ContainerCreated, apicontainerstatus.ContainerResourcesProvisioned, apicontainerstatus.ContainerRunning, true, nil}, // CREATED -> CREATED transition is not allowed and not actionable, // when desired is Running @@ -359,7 +357,7 @@ func TestContainerNextState(t *testing.T) { // when desired is Running {apicontainerstatus.ContainerResourcesProvisioned, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerStatusNone, false, dependencygraph.ContainerPastDesiredStatusErr}, // RESOURCES_PROVISIONED -> STOPPED transition is allowed and actionable, when desired - // is Running. The exptected next status is STOPPED + // is Running. The expected next status is STOPPED {apicontainerstatus.ContainerResourcesProvisioned, apicontainerstatus.ContainerStopped, apicontainerstatus.ContainerStopped, true, nil}, // RESOURCES_PROVISIONED -> NONE transition is not allowed and not actionable, // when desired is Running @@ -848,16 +846,16 @@ func TestStartContainerTransitionsWithTerminalError(t *testing.T) { dockerMessages: dockerMessagesChan, } - canTransition, _, transitions, errors := task.startContainerTransitions( + canTransition, _, transitions, errs := task.startContainerTransitions( func(cont *apicontainer.Container, nextStatus apicontainerstatus.ContainerStatus) { t.Error("Transition function should not be called when no transitions are possible") }) assert.False(t, canTransition) assert.Empty(t, transitions) - assert.Equal(t, 3, len(errors)) // first error is just indicating container1 is at desired status, following errors should be terminal - assert.False(t, errors[0].(dependencygraph.DependencyError).IsTerminal(), "Error should NOT be terminal") - assert.True(t, errors[1].(dependencygraph.DependencyError).IsTerminal(), "Error should be terminal") - assert.True(t, errors[2].(dependencygraph.DependencyError).IsTerminal(), "Error should be terminal") + assert.Equal(t, 3, len(errs)) // first error is just indicating container1 is at desired status, following errors should be terminal + assert.False(t, errs[0].(dependencygraph.DependencyError).IsTerminal(), "Error should NOT be terminal") + assert.True(t, errs[1].(dependencygraph.DependencyError).IsTerminal(), "Error should be terminal") + assert.True(t, errs[2].(dependencygraph.DependencyError).IsTerminal(), "Error should be terminal") stoppedMessages := make(map[string]dockerContainerChange) // verify we are sending STOPPED message @@ -886,7 +884,7 @@ func TestStartContainerTransitionsInvokesHandleContainerChange(t *testing.T) { eventStreamName := "TESTTASKENGINE" // Create a container with the intent to do - // CREATERD -> STOPPED transition. This triggers + // CREATED -> STOPPED transition. This triggers // `managedTask.handleContainerChange()` and generates the following // events: // 1. container state change event for Submit* API @@ -2060,7 +2058,7 @@ func TestHandleContainerChangeStopped_WithRestartPolicy_DesiredStopped(t *testin }, } - // Set desired status of container to stopped, this similates what would happen + // Set desired status of container to stopped, this simulates what would happen // if user called the ecs.StopTask API. container.SetDesiredStatus(apicontainerstatus.ContainerStopped) diff --git a/agent/engine/task_manager_unix_test.go b/agent/engine/task_manager_unix_test.go index 520629fcd75..125f770d21a 100644 --- a/agent/engine/task_manager_unix_test.go +++ b/agent/engine/task_manager_unix_test.go @@ -26,6 +26,7 @@ import ( "time" apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" + apitask "github.com/aws/amazon-ecs-agent/agent/api/task" "github.com/aws/amazon-ecs-agent/agent/data" mock_dockerapi "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi/mocks" "github.com/aws/amazon-ecs-agent/agent/engine/dependencygraph" @@ -37,16 +38,15 @@ import ( resourcestatus "github.com/aws/amazon-ecs-agent/agent/taskresource/status" "github.com/aws/amazon-ecs-agent/agent/taskresource/volume" apicontainerstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/container/status" + apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status" mock_ttime "github.com/aws/amazon-ecs-agent/ecs-agent/utils/ttime/mocks" - "github.com/golang/mock/gomock" - apitask "github.com/aws/amazon-ecs-agent/agent/api/task" - apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" ) // These tests use cgroup resource, which is linux specific. -// generic resource's(eg.volume) tests should be added to common test file. +// generic resources (e.g. volume) tests should be added to common test file. func TestHandleResourceStateChangeAndSave(t *testing.T) { testCases := []struct { Name string diff --git a/agent/taskresource/interface.go b/agent/taskresource/interface.go index 379637bfce6..dfb8042d5ce 100644 --- a/agent/taskresource/interface.go +++ b/agent/taskresource/interface.go @@ -43,7 +43,7 @@ type TaskResource interface { Cleanup() error // GetName returns the unique name of the resource GetName() string - // DesiredTeminal returns true if remove is in terminal state + // DesiredTerminal returns true if remove is in terminal state DesiredTerminal() bool // KnownCreated returns true if resource state is CREATED KnownCreated() bool @@ -72,7 +72,7 @@ type TaskResource interface { // BuildContainerDependency adds a new dependency container and its satisfied status BuildContainerDependency(containerName string, satisfied apicontainerstatus.ContainerStatus, dependent resourcestatus.ResourceStatus) - // Initialize will initialze the resource fields of the resource + // Initialize will initialize the resource fields of the resource Initialize(res *ResourceFields, taskKnownStatus apitaskstatus.TaskStatus, taskDesiredStatus apitaskstatus.TaskStatus)