From b5a54a0c7b0044330d084dd99165cf35c6e0bb61 Mon Sep 17 00:00:00 2001 From: mye956 Date: Tue, 22 Aug 2023 18:14:56 +0000 Subject: [PATCH 1/3] Adding EBS watcher implementation --- agent/ebs/watcher.go | 175 ++++++++++ agent/ebs/watcher_test.go | 315 ++++++++++++++++++ .../dockerstate/docker_task_engine_state.go | 31 +- agent/engine/dockerstate/dockerstate_test.go | 12 +- .../dockerstate/mocks/dockerstate_mocks.go | 14 + .../ecs-agent/api/resource/ebs_discovery.go | 64 ++++ .../api/resource/ebs_discovery_linux.go | 92 +++++ .../api/resource/ebs_discovery_windows.go | 21 ++ .../ecs-agent/api/resource/generate_mocks.go | 16 + .../ecs-agent/api/resource/interfaces.go | 27 ++ .../ecs-agent/api/resource/mocks/ebs_mocks.go | 62 ++++ .../api/resource/resource_attachment.go | 59 +++- agent/vendor/modules.txt | 1 + ecs-agent/api/resource/ebs_discovery.go | 64 ++++ ecs-agent/api/resource/ebs_discovery_linux.go | 92 +++++ .../api/resource/ebs_discovery_linux_test.go | 64 ++++ .../api/resource/ebs_discovery_windows.go | 21 ++ ecs-agent/api/resource/generate_mocks.go | 16 + ecs-agent/api/resource/interfaces.go | 27 ++ ecs-agent/api/resource/mocks/ebs_mocks.go | 62 ++++ ecs-agent/api/resource/resource_attachment.go | 59 +++- 21 files changed, 1279 insertions(+), 15 deletions(-) create mode 100644 agent/ebs/watcher.go create mode 100644 agent/ebs/watcher_test.go create mode 100644 agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go create mode 100644 agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go create mode 100644 agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_windows.go create mode 100644 agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/generate_mocks.go create mode 100644 agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/interfaces.go create mode 100644 agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/mocks/ebs_mocks.go create mode 100644 ecs-agent/api/resource/ebs_discovery.go create mode 100644 ecs-agent/api/resource/ebs_discovery_linux.go create mode 100644 ecs-agent/api/resource/ebs_discovery_linux_test.go create mode 100644 ecs-agent/api/resource/ebs_discovery_windows.go create mode 100644 ecs-agent/api/resource/generate_mocks.go create mode 100644 ecs-agent/api/resource/interfaces.go create mode 100644 ecs-agent/api/resource/mocks/ebs_mocks.go diff --git a/agent/ebs/watcher.go b/agent/ebs/watcher.go new file mode 100644 index 00000000000..48be77e41f7 --- /dev/null +++ b/agent/ebs/watcher.go @@ -0,0 +1,175 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package ebs + +import ( + "context" + "fmt" + "time" + + "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate" + "github.com/aws/amazon-ecs-agent/agent/statechange" + apiebs "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource" + log "github.com/cihub/seelog" +) + +type EBSWatcher struct { + ctx context.Context + cancel context.CancelFunc + agentState dockerstate.TaskEngineState + // TODO: The ebsChangeEvent will be used to send over the state change event for EBS attachments once it's been found and mounted/resize/format. + ebsChangeEvent chan<- statechange.Event + // TODO: The dataClient will be used to save to agent's data client as well as start the ACK timer. This will be added once the data client functionality have been added + // dataClient data.Client + discoveryClient apiebs.EBSDiscovery + scanTicker *time.Ticker +} + +// NewWatcher is used to return a new instance of the EBSWatcher struct +func NewWatcher(ctx context.Context, + state dockerstate.TaskEngineState, + stateChangeEvents chan<- statechange.Event) *EBSWatcher { + derivedContext, cancel := context.WithCancel(ctx) + discoveryClient := apiebs.NewDiscoveryClient(derivedContext) + return &EBSWatcher{ + ctx: derivedContext, + cancel: cancel, + agentState: state, + ebsChangeEvent: stateChangeEvents, + discoveryClient: discoveryClient, + } +} + +// Start is used to kick off the periodic scanning process of the EBS volume attachments for the EBS watcher. +// It will be start and continue to run whenever there's a pending EBS volume attachment that hasn't been found. +// If there aren't any, the scan ticker will not start up/scan for volumes. +func (w *EBSWatcher) Start() { + log.Info("Starting EBS watcher.") + w.scanTicker = time.NewTicker(apiebs.ScanPeriod) + for { + select { + case <-w.scanTicker.C: + pendingEBS := w.agentState.GetAllPendingEBSAttachmentsWithKey() + if len(pendingEBS) > 0 { + foundVolumes := apiebs.ScanEBSVolumes(pendingEBS, w.discoveryClient) + w.NotifyFound(foundVolumes) + } + case <-w.ctx.Done(): + w.scanTicker.Stop() + log.Info("EBS Watcher Stopped due to agent stop") + return + } + } +} + +// Stop will stop the EBS watcher +func (w *EBSWatcher) Stop() { + log.Info("Stopping EBS watcher.") + w.cancel() +} + +// HandleResourceAttachment processes the resource attachment message. It will: +// 1. Check whether we already have this attachment in state and if so it's a noop. +// 2. Otherwise add the attachment to state, start its ack timer, and save to the agent state. +func (w *EBSWatcher) HandleResourceAttachment(ebs *apiebs.ResourceAttachment) error { + attachmentType := ebs.GetAttachmentProperties(apiebs.ResourceTypeName) + if attachmentType != apiebs.ElasticBlockStorage { + log.Warnf("Resource type not Elastic Block Storage. Skip handling resource attachment with type: %v.", attachmentType) + return nil + } + + volumeId := ebs.GetAttachmentProperties(apiebs.VolumeIdName) + _, ok := w.agentState.GetEBSByVolumeId(volumeId) + if ok { + log.Infof("EBS Volume attachment already exists. Skip handling EBS attachment %v.", ebs.EBSToString()) + return nil + } + + if err := w.addEBSAttachmentToState(ebs); err != nil { + return fmt.Errorf("%w; attach %v message handler: unable to add ebs attachment to engine state: %v", + err, attachmentType, ebs.EBSToString()) + } + + return nil +} + +// NotifyFound will go through the list of found EBS volumes from the scanning process and mark them as found. +func (w *EBSWatcher) NotifyFound(foundVolumes []string) { + for _, volumeId := range foundVolumes { + w.notifyFoundEBS(volumeId) + } +} + +// notifyFoundEBS will mark it as found within the agent state +func (w *EBSWatcher) notifyFoundEBS(volumeId string) { + // TODO: Add the EBS volume to data client + ebs, ok := w.agentState.GetEBSByVolumeId(volumeId) + if !ok { + log.Warnf("Unable to find EBS volume with volume ID: %v.", volumeId) + return + } + + if ebs.HasExpired() { + log.Warnf("EBS status expired, no longer tracking EBS volume: %v.", ebs.EBSToString()) + return + } + + if ebs.IsSent() { + log.Warnf("State change event has already been emitted for EBS volume: %v.", ebs.EBSToString()) + return + } + + if ebs.IsAttached() { + log.Infof("EBS volume: %v, has been found already.", ebs.EBSToString()) + return + } + + ebs.StopAckTimer() + ebs.SetAttachedStatus() + + log.Infof("Successfully found attached EBS volume: %v", ebs.EBSToString()) +} + +// removeEBSAttachment removes a EBS volume with a specific volume ID +func (w *EBSWatcher) removeEBSAttachment(volumeID string) { + // TODO: Remove the EBS volume from the data client. + w.agentState.RemoveEBSAttachment(volumeID) +} + +// addEBSAttachmentToState adds an EBS attachment to state, and start its ack timer +func (w *EBSWatcher) addEBSAttachmentToState(ebs *apiebs.ResourceAttachment) error { + volumeId := ebs.AttachmentProperties[apiebs.VolumeIdName] + err := ebs.StartTimer(func() { + w.handleEBSAckTimeout(volumeId) + }) + if err != nil { + return err + } + + w.agentState.AddEBSAttachment(ebs) + return nil +} + +// handleEBSAckTimeout removes EBS attachment from agent state after the EBS ack timeout +func (w *EBSWatcher) handleEBSAckTimeout(volumeId string) { + ebsAttachment, ok := w.agentState.GetEBSByVolumeId(volumeId) + if !ok { + log.Warnf("Ignoring unmanaged EBS attachment volume ID=%v", volumeId) + return + } + if !ebsAttachment.IsSent() { + log.Warnf("Timed out waiting for EBS ack; removing EBS attachment record %v", ebsAttachment.EBSToString()) + w.removeEBSAttachment(volumeId) + } +} diff --git a/agent/ebs/watcher_test.go b/agent/ebs/watcher_test.go new file mode 100644 index 00000000000..7ee370e97e1 --- /dev/null +++ b/agent/ebs/watcher_test.go @@ -0,0 +1,315 @@ +//go:build unit +// +build unit + +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package ebs + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate" + "github.com/aws/amazon-ecs-agent/agent/statechange" + "github.com/aws/amazon-ecs-agent/ecs-agent/acs/session/testconst" + "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo" + apiebs "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource" + mock_ebs_discovery "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/mocks" + "github.com/aws/amazon-ecs-agent/ecs-agent/api/status" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" +) + +const ( + resourceAttachmentARN = "arn:aws:ecs:us-west-2:123456789012:attachment/a1b2c3d4-5678-90ab-cdef-11111EXAMPLE" + containerInstanceARN = "arn:aws:ecs:us-west-2:123456789012:container-instance/a1b2c3d4-5678-90ab-cdef-11111EXAMPLE" + taskARN = "task1" + taskClusterARN = "arn:aws:ecs:us-west-2:123456789012:cluster/customer-task-cluster" + deviceName = "/dev/xvdba" + volumeID = "vol-1234" +) + +// newTestEBSWatcher creates a new EBSWatcher object for testing +func newTestEBSWatcher(ctx context.Context, agentState dockerstate.TaskEngineState, + ebsChangeEvent chan<- statechange.Event, discoveryClient apiebs.EBSDiscovery) *EBSWatcher { + derivedContext, cancel := context.WithCancel(ctx) + return &EBSWatcher{ + ctx: derivedContext, + cancel: cancel, + agentState: agentState, + ebsChangeEvent: ebsChangeEvent, + discoveryClient: discoveryClient, + } +} + +// TestHandleEBSAttachmentHappyCase tests handling a new resource attachment of type Elastic Block Stores +// The expected behavior is for the resource attachment to be added to the agent state and be able to be marked as attached. +func TestHandleEBSAttachmentHappyCase(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + ctx := context.Background() + taskEngineState := dockerstate.NewTaskEngineState() + eventChannel := make(chan statechange.Event) + mockDiscoveryClient := mock_ebs_discovery.NewMockEBSDiscovery(mockCtrl) + + testAttachmentProperties := map[string]string{ + apiebs.ResourceTypeName: apiebs.ElasticBlockStorage, + apiebs.DeviceName: deviceName, + apiebs.VolumeIdName: volumeID, + } + + expiresAt := time.Now().Add(time.Millisecond * testconst.WaitTimeoutMillis) + ebsAttachment := &apiebs.ResourceAttachment{ + AttachmentInfo: attachmentinfo.AttachmentInfo{ + TaskARN: taskARN, + TaskClusterARN: taskClusterARN, + ContainerInstanceARN: containerInstanceARN, + ExpiresAt: expiresAt, + Status: status.AttachmentNone, + AttachmentARN: resourceAttachmentARN, + }, + AttachmentProperties: testAttachmentProperties, + } + watcher := newTestEBSWatcher(ctx, taskEngineState, eventChannel, mockDiscoveryClient) + var wg sync.WaitGroup + wg.Add(1) + mockDiscoveryClient.EXPECT().ConfirmEBSVolumeIsAttached(deviceName, volumeID). + Do(func(deviceName, volumeID string) { + wg.Done() + }). + Return(nil). + MinTimes(1) + + err := watcher.HandleResourceAttachment(ebsAttachment) + assert.NoError(t, err) + + // Instead of starting the EBS watcher, we'll be mocking a tick of the EBS watcher's scan ticker. + // Otherwise, the watcher will continue to run forever and the test will panic. + wg.Add(1) + go func() { + defer wg.Done() + pendingEBS := watcher.agentState.GetAllPendingEBSAttachmentsWithKey() + if len(pendingEBS) > 0 { + foundVolumes := apiebs.ScanEBSVolumes(pendingEBS, watcher.discoveryClient) + watcher.NotifyFound(foundVolumes) + } + }() + + wg.Wait() + + assert.Len(t, taskEngineState.(*dockerstate.DockerTaskEngineState).GetAllEBSAttachments(), 1) + ebsAttachment, ok := taskEngineState.(*dockerstate.DockerTaskEngineState).GetEBSByVolumeId(volumeID) + assert.True(t, ok) + assert.True(t, ebsAttachment.IsAttached()) +} + +// TestHandleExpiredEBSAttachment tests acknowledging an expired resource attachment of type Elastic Block Stores +// The resource attachment object should not be saved to the agent state since the expiration date is in the past. +func TestHandleExpiredEBSAttachment(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + ctx := context.Background() + taskEngineState := dockerstate.NewTaskEngineState() + eventChannel := make(chan statechange.Event) + mockDiscoveryClient := mock_ebs_discovery.NewMockEBSDiscovery(mockCtrl) + + testAttachmentProperties := map[string]string{ + apiebs.ResourceTypeName: apiebs.ElasticBlockStorage, + apiebs.DeviceName: deviceName, + apiebs.VolumeIdName: volumeID, + } + + expiresAt := time.Now().Add(-1 * time.Millisecond) + ebsAttachment := &apiebs.ResourceAttachment{ + AttachmentInfo: attachmentinfo.AttachmentInfo{ + TaskARN: taskARN, + TaskClusterARN: taskClusterARN, + ContainerInstanceARN: containerInstanceARN, + ExpiresAt: expiresAt, + Status: status.AttachmentNone, + AttachmentARN: resourceAttachmentARN, + }, + AttachmentProperties: testAttachmentProperties, + } + watcher := newTestEBSWatcher(ctx, taskEngineState, eventChannel, mockDiscoveryClient) + + err := watcher.HandleResourceAttachment(ebsAttachment) + assert.Error(t, err) + assert.Len(t, taskEngineState.(*dockerstate.DockerTaskEngineState).GetAllEBSAttachments(), 0) + _, ok := taskEngineState.(*dockerstate.DockerTaskEngineState).GetEBSByVolumeId(volumeID) + assert.False(t, ok) +} + +// TestHandleDuplicateEBSAttachment tests handling duplicate resource attachment of type Elastic Block Store +// The expected behavior is for only of the resource attachment object to be added to the agent state. +func TestHandleDuplicateEBSAttachment(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + ctx := context.Background() + taskEngineState := dockerstate.NewTaskEngineState() + eventChannel := make(chan statechange.Event) + mockDiscoveryClient := mock_ebs_discovery.NewMockEBSDiscovery(mockCtrl) + + expiresAt := time.Now().Add(time.Millisecond * testconst.WaitTimeoutMillis) + + testAttachmentProperties1 := map[string]string{ + apiebs.ResourceTypeName: apiebs.ElasticBlockStorage, + apiebs.DeviceName: deviceName, + apiebs.VolumeIdName: volumeID, + } + + ebsAttachment1 := &apiebs.ResourceAttachment{ + AttachmentInfo: attachmentinfo.AttachmentInfo{ + TaskARN: taskARN, + TaskClusterARN: taskClusterARN, + ContainerInstanceARN: containerInstanceARN, + ExpiresAt: expiresAt, + Status: status.AttachmentNone, + AttachmentARN: resourceAttachmentARN, + }, + AttachmentProperties: testAttachmentProperties1, + } + + testAttachmentProperties2 := map[string]string{ + apiebs.ResourceTypeName: apiebs.ElasticBlockStorage, + apiebs.DeviceName: deviceName, + apiebs.VolumeIdName: volumeID, + } + + ebsAttachment2 := &apiebs.ResourceAttachment{ + AttachmentInfo: attachmentinfo.AttachmentInfo{ + TaskARN: taskARN, + TaskClusterARN: taskClusterARN, + ContainerInstanceARN: containerInstanceARN, + ExpiresAt: expiresAt, + Status: status.AttachmentNone, + AttachmentARN: resourceAttachmentARN, + }, + AttachmentProperties: testAttachmentProperties2, + } + + watcher := newTestEBSWatcher(ctx, taskEngineState, eventChannel, mockDiscoveryClient) + var wg sync.WaitGroup + wg.Add(1) + mockDiscoveryClient.EXPECT().ConfirmEBSVolumeIsAttached(deviceName, volumeID). + Do(func(deviceName, volumeID string) { + wg.Done() + }). + Return(nil). + MinTimes(1) + + watcher.HandleResourceAttachment(ebsAttachment1) + watcher.HandleResourceAttachment(ebsAttachment2) + + // Instead of starting the EBS watcher, we'll be mocking a tick of the EBS watcher's scan ticker. + // Otherwise, the watcher will continue to run forever and the test will panic. + wg.Add(1) + go func() { + defer wg.Done() + pendingEBS := watcher.agentState.GetAllPendingEBSAttachmentsWithKey() + if len(pendingEBS) > 0 { + foundVolumes := apiebs.ScanEBSVolumes(pendingEBS, watcher.discoveryClient) + watcher.NotifyFound(foundVolumes) + } + }() + + wg.Wait() + + assert.Len(t, taskEngineState.(*dockerstate.DockerTaskEngineState).GetAllEBSAttachments(), 1) + ebsAttachment, ok := taskEngineState.(*dockerstate.DockerTaskEngineState).GetEBSByVolumeId(volumeID) + assert.True(t, ok) + assert.True(t, ebsAttachment.IsAttached()) +} + +// TestHandleInvalidTypeEBSAttachment tests handling a resource attachment that is not of type Elastic Block Stores +// The expected behavior for the EBS watcher is to not add the resource attachment object to the agent state. +func TestHandleInvalidTypeEBSAttachment(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + ctx := context.Background() + taskEngineState := dockerstate.NewTaskEngineState() + eventChannel := make(chan statechange.Event) + mockDiscoveryClient := mock_ebs_discovery.NewMockEBSDiscovery(mockCtrl) + + testAttachmentProperties := map[string]string{ + apiebs.ResourceTypeName: "InvalidResourceType", + apiebs.DeviceName: deviceName, + apiebs.VolumeIdName: volumeID, + } + + expiresAt := time.Now().Add(time.Millisecond * testconst.WaitTimeoutMillis) + ebsAttachment := &apiebs.ResourceAttachment{ + AttachmentInfo: attachmentinfo.AttachmentInfo{ + TaskARN: taskARN, + TaskClusterARN: taskClusterARN, + ContainerInstanceARN: containerInstanceARN, + ExpiresAt: expiresAt, + Status: status.AttachmentNone, + AttachmentARN: resourceAttachmentARN, + }, + AttachmentProperties: testAttachmentProperties, + } + watcher := newTestEBSWatcher(ctx, taskEngineState, eventChannel, mockDiscoveryClient) + + watcher.HandleResourceAttachment(ebsAttachment) + + assert.Len(t, taskEngineState.(*dockerstate.DockerTaskEngineState).GetAllEBSAttachments(), 0) + _, ok := taskEngineState.(*dockerstate.DockerTaskEngineState).GetEBSByVolumeId(volumeID) + assert.False(t, ok) +} + +// TestHandleEBSAckTimeout tests acknowledging the timeout of a EBS-type resource attachment object saved within the agent state. +// The expected behavior is after the timeout duration, the resource attachment object will be removed from agent state. +func TestHandleEBSAckTimeout(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + ctx := context.Background() + taskEngineState := dockerstate.NewTaskEngineState() + eventChannel := make(chan statechange.Event) + mockDiscoveryClient := mock_ebs_discovery.NewMockEBSDiscovery(mockCtrl) + + testAttachmentProperties := map[string]string{ + apiebs.ResourceTypeName: apiebs.ElasticBlockStorage, + apiebs.DeviceName: deviceName, + apiebs.VolumeIdName: volumeID, + } + + expiresAt := time.Now().Add(time.Millisecond * 5) + ebsAttachment := &apiebs.ResourceAttachment{ + AttachmentInfo: attachmentinfo.AttachmentInfo{ + TaskARN: taskARN, + TaskClusterARN: taskClusterARN, + ContainerInstanceARN: containerInstanceARN, + ExpiresAt: expiresAt, + Status: status.AttachmentNone, + AttachmentARN: resourceAttachmentARN, + }, + AttachmentProperties: testAttachmentProperties, + } + watcher := newTestEBSWatcher(ctx, taskEngineState, eventChannel, mockDiscoveryClient) + + watcher.HandleResourceAttachment(ebsAttachment) + time.Sleep(10 * time.Millisecond) + assert.Len(t, taskEngineState.(*dockerstate.DockerTaskEngineState).GetAllEBSAttachments(), 0) + ebsAttachment, ok := taskEngineState.(*dockerstate.DockerTaskEngineState).GetEBSByVolumeId(volumeID) + assert.False(t, ok) +} diff --git a/agent/engine/dockerstate/docker_task_engine_state.go b/agent/engine/dockerstate/docker_task_engine_state.go index 2393eedc992..2e5803f9f8f 100644 --- a/agent/engine/dockerstate/docker_task_engine_state.go +++ b/agent/engine/dockerstate/docker_task_engine_state.go @@ -86,6 +86,8 @@ type TaskEngineState interface { GetAllEBSAttachments() []*apiresource.ResourceAttachment // AllPendingEBSAttachments reutrns all of the ebs attachments that haven't sent a state change GetAllPendingEBSAttachments() []*apiresource.ResourceAttachment + // GetAllPendingEBSAttachmentWithKey returns a map of all pending ebs attachments along with the corresponding volume ID as the key + GetAllPendingEBSAttachmentsWithKey() map[string]*apiresource.ResourceAttachment // AddEBSAttachment adds an ebs attachment from acs to be stored AddEBSAttachment(ebs *apiresource.ResourceAttachment) // RemoveEBSAttachment removes an ebs attachment to stop tracking @@ -287,7 +289,7 @@ func (state *DockerTaskEngineState) allEBSAttachmentsUnsafe() []*apiresource.Res return allEBSAttachments } -// GetAllPendingEBSAttachments returns all the ebs volumes managed by ecs on the instance that haven't sent a state change yet +// GetAllPendingEBSAttachments returns all the ebs volumes managed by ecs on the instance that haven't been found attached on the host func (state *DockerTaskEngineState) GetAllPendingEBSAttachments() []*apiresource.ResourceAttachment { state.lock.RLock() defer state.lock.RUnlock() @@ -298,13 +300,32 @@ func (state *DockerTaskEngineState) GetAllPendingEBSAttachments() []*apiresource func (state *DockerTaskEngineState) allPendingEBSAttachmentsUnsafe() []*apiresource.ResourceAttachment { var pendingEBSAttachments []*apiresource.ResourceAttachment for _, v := range state.ebsAttachments { - if !v.IsSent() { + if !v.IsAttached() { pendingEBSAttachments = append(pendingEBSAttachments, v) } } return pendingEBSAttachments } +// GetAllPendingEBSAttachmentsWithKey returns all the ebs volumes managed by ecs on the instance that haven't been found attached on the host as a map +// with the corresponding volume ID as the key +func (state *DockerTaskEngineState) GetAllPendingEBSAttachmentsWithKey() map[string]*apiresource.ResourceAttachment { + state.lock.RLock() + defer state.lock.RUnlock() + + return state.allPendingEBSAttachmentsWithKeyUnsafe() +} + +func (state *DockerTaskEngineState) allPendingEBSAttachmentsWithKeyUnsafe() map[string]*apiresource.ResourceAttachment { + pendingEBSAttachments := make(map[string]*apiresource.ResourceAttachment) + for k, v := range state.ebsAttachments { + if !v.IsAttached() { + pendingEBSAttachments[k] = v + } + } + return pendingEBSAttachments +} + // AddEBSAttachment adds the ebs volume to state func (state *DockerTaskEngineState) AddEBSAttachment(ebsAttachment *apiresource.ResourceAttachment) { if ebsAttachment == nil { @@ -316,9 +337,9 @@ func (state *DockerTaskEngineState) AddEBSAttachment(ebsAttachment *apiresource. volumeId := ebsAttachment.AttachmentProperties[apiresource.VolumeIdName] if _, ok := state.ebsAttachments[volumeId]; !ok { state.ebsAttachments[volumeId] = ebsAttachment - seelog.Debugf("Successfully added EBS attachment: %v", ebsAttachment) + seelog.Debugf("Successfully added EBS attachment: %v", ebsAttachment.EBSToString()) } else { - seelog.Debugf("Duplicate ebs attachment information: %v", ebsAttachment) + seelog.Debugf("Duplicate ebs attachment information: %v", ebsAttachment.EBSToString()) } } @@ -332,7 +353,7 @@ func (state *DockerTaskEngineState) RemoveEBSAttachment(volumeId string) { defer state.lock.Unlock() if ebs, ok := state.ebsAttachments[volumeId]; ok { delete(state.ebsAttachments, volumeId) - seelog.Debugf("Successfully deleted EBS attachment: %v", ebs) + seelog.Debugf("Successfully deleted EBS attachment: %v", ebs.EBSToString()) } else { seelog.Debugf("RemoveEBSAttachment: The requested EBS attachment with volume ID: %v does not exist", volumeId) } diff --git a/agent/engine/dockerstate/dockerstate_test.go b/agent/engine/dockerstate/dockerstate_test.go index 6c66829d503..02df17bb720 100644 --- a/agent/engine/dockerstate/dockerstate_test.go +++ b/agent/engine/dockerstate/dockerstate_test.go @@ -24,6 +24,7 @@ import ( "github.com/aws/amazon-ecs-agent/agent/engine/image" "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo" apiresource "github.com/aws/amazon-ecs-agent/ecs-agent/api/resource" + "github.com/aws/amazon-ecs-agent/ecs-agent/api/status" ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface" "github.com/stretchr/testify/assert" ) @@ -164,6 +165,7 @@ func TestAddPendingEBSAttachment(t *testing.T) { TaskARN: "taskarn1", AttachmentARN: "ebs1", AttachStatusSent: false, + Status: status.AttachmentNone, }, AttachmentProperties: testAttachmentProperties, } @@ -177,19 +179,25 @@ func TestAddPendingEBSAttachment(t *testing.T) { apiresource.FileSystemTypeName: "testXFS2", } - sentAttachment := &apiresource.ResourceAttachment{ + foundAttachment := &apiresource.ResourceAttachment{ AttachmentInfo: attachmentinfo.AttachmentInfo{ TaskARN: "taskarn2", AttachmentARN: "ebs2", AttachStatusSent: true, + Status: status.AttachmentAttached, }, AttachmentProperties: testSentAttachmentProperties, } state.AddEBSAttachment(pendingAttachment) - state.AddEBSAttachment(sentAttachment) + state.AddEBSAttachment(foundAttachment) assert.Len(t, state.(*DockerTaskEngineState).GetAllPendingEBSAttachments(), 1) + assert.Len(t, state.(*DockerTaskEngineState).GetAllPendingEBSAttachmentsWithKey(), 1) assert.Len(t, state.(*DockerTaskEngineState).GetAllEBSAttachments(), 2) + + _, ok := state.(*DockerTaskEngineState).GetAllPendingEBSAttachmentsWithKey()["vol-123"] + assert.True(t, ok) + } func TestTwophaseAddContainer(t *testing.T) { diff --git a/agent/engine/dockerstate/mocks/dockerstate_mocks.go b/agent/engine/dockerstate/mocks/dockerstate_mocks.go index ee74d6b1fe2..73a577dc1a8 100644 --- a/agent/engine/dockerstate/mocks/dockerstate_mocks.go +++ b/agent/engine/dockerstate/mocks/dockerstate_mocks.go @@ -294,6 +294,20 @@ func (mr *MockTaskEngineStateMockRecorder) GetAllPendingEBSAttachments() *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllPendingEBSAttachments", reflect.TypeOf((*MockTaskEngineState)(nil).GetAllPendingEBSAttachments)) } +// GetAllPendingEBSAttachmentsWithKey mocks base method. +func (m *MockTaskEngineState) GetAllPendingEBSAttachmentsWithKey() map[string]*resource.ResourceAttachment { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAllPendingEBSAttachmentsWithKey") + ret0, _ := ret[0].(map[string]*resource.ResourceAttachment) + return ret0 +} + +// GetAllPendingEBSAttachmentsWithKey indicates an expected call of GetAllPendingEBSAttachmentsWithKey. +func (mr *MockTaskEngineStateMockRecorder) GetAllPendingEBSAttachmentsWithKey() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllPendingEBSAttachmentsWithKey", reflect.TypeOf((*MockTaskEngineState)(nil).GetAllPendingEBSAttachmentsWithKey)) +} + // GetEBSByVolumeId mocks base method. func (m *MockTaskEngineState) GetEBSByVolumeId(arg0 string) (*resource.ResourceAttachment, bool) { m.ctrl.T.Helper() diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go new file mode 100644 index 00000000000..77bf30b9456 --- /dev/null +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go @@ -0,0 +1,64 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package resource + +import ( + "context" + "errors" + "fmt" + "strings" + "time" +) + +const ( + ebsnvmeIDTimeoutDuration = 5 * time.Second + ebsResourceKeyPrefix = "ebs-volume:" + ScanPeriod = 500 * time.Millisecond +) + +var ( + ErrInvalidVolumeID = errors.New("EBS volume IDs do not match") +) + +// EBSDiscoveryClient is used to scan and validate if an EBS volume is attached on the host instance. +type EBSDiscoveryClient struct { + ctx context.Context +} + +// NewDiscoveryClient creates a new EBSDiscoveryClient object +func NewDiscoveryClient(ctx context.Context) EBSDiscovery { + return &EBSDiscoveryClient{ + ctx: ctx, + } +} + +// ScanEBSVolumes will iterate through the entire list of pending EBS volume attachments within the agent state and checks if it's attached on the host. +func ScanEBSVolumes[T GenericEBSAttachmentObject](pendingAttachments map[string]T, dc EBSDiscovery) []string { + var err error + var foundVolumes []string + for key, ebs := range pendingAttachments { + volumeId := strings.TrimPrefix(key, ebsResourceKeyPrefix) + deviceName := ebs.GetAttachmentProperties(DeviceName) + err = dc.ConfirmEBSVolumeIsAttached(deviceName, volumeId) + if err != nil { + if !errors.Is(err, ErrInvalidVolumeID) { + err = fmt.Errorf("%w; failed to confirm if EBS volume is attached to the host", err) + } + ebs.SetError(err) + continue + } + foundVolumes = append(foundVolumes, key) + } + return foundVolumes +} diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go new file mode 100644 index 00000000000..c8da7cd4d9b --- /dev/null +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go @@ -0,0 +1,92 @@ +//go:build linux +// +build linux + +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package resource + +import ( + "context" + "encoding/json" + "fmt" + "os/exec" + "strings" +) + +// LsblkOutput is used to manage and track the output of `lsblk` +type LsblkOutput struct { + BlockDevies []BD `json:"blockdevices"` +} +type BD struct { + Name string `json:"name"` + Serial string `json:"serial"` + Children []BDChild `json:"children"` +} +type BDChild struct { + Name string `json:"name"` + Serial string `json:"serial"` +} + +// ConfirmEBSVolumeIsAttached is used to scan for an EBS volume that's on the host with a specific device name and/or volume ID. +// There are two cases: +// 1. On nitro-based instance we check both device name and volume ID. +// 2. On xen-based instance we only check by the device name. +func (api *EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error { + var lsblkOut LsblkOutput + ctxWithTimeout, cancel := context.WithTimeout(api.ctx, ebsnvmeIDTimeoutDuration) + defer cancel() + + // The lsblk command will output the name and volume ID of all block devices on the host in JSON format + output, err := exec.CommandContext(ctxWithTimeout, "lsblk", "-o", "NAME,SERIAL", "-J").CombinedOutput() + if err != nil { + err = fmt.Errorf("%w; failed to run lsblk %v", err, string(output)) + return err + } + err = json.Unmarshal(output, &lsblkOut) + if err != nil { + err = fmt.Errorf("%w; failed to unmarshal string: %v", err, string(output)) + return err + } + + actualVolumeId, err := parseLsblkOutput(&lsblkOut, deviceName) + if err != nil { + return err + } + expectedVolumeId := strings.ReplaceAll(volumeID, "-", "") + + // On Xen-based instances, the volume ID can't be obtained and so we don't need to check by volume ID. + if actualVolumeId != "" && expectedVolumeId != actualVolumeId { + err = fmt.Errorf("%w; expected EBS volume %v but found %v", ErrInvalidVolumeID, volumeID, actualVolumeId) + return err + } + + return nil +} + +// parseLsblkOutput will parse the `lsblk` output and search for a EBS volume with a specific device name. +// Once found we return the volume ID, otherwise we return an empty string along with an error +// The output of the "lsblk -o +SERIAL" command looks like the following: +// NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT SERIAL +// nvme0n1 259:0 0 30G 0 disk vol123 +// ├─nvme0n1p1 259:1 0 30G 0 part / +// └─nvme0n1p128 259:2 0 1M 0 part +func parseLsblkOutput(output *LsblkOutput, deviceName string) (string, error) { + actualDeviceName := deviceName[strings.LastIndex(deviceName, "/")+1:] + for _, block := range output.BlockDevies { + if block.Name == actualDeviceName { + return block.Serial, nil + } + } + return "", fmt.Errorf("cannot find EBS volume with device name: %v", actualDeviceName) +} diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_windows.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_windows.go new file mode 100644 index 00000000000..425844610c1 --- /dev/null +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_windows.go @@ -0,0 +1,21 @@ +//go:build windows +// +build windows + +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package resource + +func (api EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error { + return nil +} diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/generate_mocks.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/generate_mocks.go new file mode 100644 index 00000000000..a66369fc121 --- /dev/null +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/generate_mocks.go @@ -0,0 +1,16 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +//go:generate mockgen -destination=mocks/ebs_mocks.go -copyright_file=../../../scripts/copyright_file github.com/aws/amazon-ecs-agent/ecs-agent/api/resource EBSDiscovery + +package resource diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/interfaces.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/interfaces.go new file mode 100644 index 00000000000..9be152f1b3f --- /dev/null +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/interfaces.go @@ -0,0 +1,27 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package resource + +// EBSDiscovery is an interface used to find EBS volumes that are attached onto the host instance. It is implemented by +// EBSDiscoveryClient +type EBSDiscovery interface { + ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error +} + +// GenericEBSAttachmentObject is an interface used to implement the Resource attachment objects that's saved within the agent state +type GenericEBSAttachmentObject interface { + GetAttachmentProperties(key string) string + EBSToString() string + SetError(err error) +} diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/mocks/ebs_mocks.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/mocks/ebs_mocks.go new file mode 100644 index 00000000000..7e8eb5918ce --- /dev/null +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/mocks/ebs_mocks.go @@ -0,0 +1,62 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. +// + +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/aws/amazon-ecs-agent/ecs-agent/api/resource (interfaces: EBSDiscovery) + +// Package mock_resource is a generated GoMock package. +package mock_resource + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockEBSDiscovery is a mock of EBSDiscovery interface. +type MockEBSDiscovery struct { + ctrl *gomock.Controller + recorder *MockEBSDiscoveryMockRecorder +} + +// MockEBSDiscoveryMockRecorder is the mock recorder for MockEBSDiscovery. +type MockEBSDiscoveryMockRecorder struct { + mock *MockEBSDiscovery +} + +// NewMockEBSDiscovery creates a new mock instance. +func NewMockEBSDiscovery(ctrl *gomock.Controller) *MockEBSDiscovery { + mock := &MockEBSDiscovery{ctrl: ctrl} + mock.recorder = &MockEBSDiscoveryMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockEBSDiscovery) EXPECT() *MockEBSDiscoveryMockRecorder { + return m.recorder +} + +// ConfirmEBSVolumeIsAttached mocks base method. +func (m *MockEBSDiscovery) ConfirmEBSVolumeIsAttached(arg0, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConfirmEBSVolumeIsAttached", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// ConfirmEBSVolumeIsAttached indicates an expected call of ConfirmEBSVolumeIsAttached. +func (mr *MockEBSDiscoveryMockRecorder) ConfirmEBSVolumeIsAttached(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfirmEBSVolumeIsAttached", reflect.TypeOf((*MockEBSDiscovery)(nil).ConfirmEBSVolumeIsAttached), arg0, arg1) +} diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/resource_attachment.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/resource_attachment.go index ff81db0af0b..78ae3591a0c 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/resource_attachment.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/resource_attachment.go @@ -14,14 +14,15 @@ package resource import ( + "errors" + "fmt" "sync" "time" "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo" + "github.com/aws/amazon-ecs-agent/ecs-agent/api/status" "github.com/aws/amazon-ecs-agent/ecs-agent/logger" "github.com/aws/amazon-ecs-agent/ecs-agent/utils/ttime" - - "github.com/pkg/errors" ) type ResourceAttachment struct { @@ -38,6 +39,7 @@ type ResourceAttachment struct { ackTimer ttime.Timer // guard protects access to fields of this struct guard sync.RWMutex + err error } // Agent Communication Service (ACS) can send messages of type ConfirmAttachmentMessage. These messages include @@ -104,7 +106,7 @@ func getResourceAttachmentLogFields(ra *ResourceAttachment, duration time.Durati "attachmentType": ra.AttachmentProperties[ResourceTypeName], "attachmentSent": ra.AttachStatusSent, "volumeSizeInGiB": ra.AttachmentProperties[VolumeSizeInGiBName], - "RequestedSizeName": ra.AttachmentProperties[RequestedSizeName], + "requestedSizeName": ra.AttachmentProperties[RequestedSizeName], "volumeId": ra.AttachmentProperties[VolumeIdName], "deviceName": ra.AttachmentProperties[DeviceName], "filesystemType": ra.AttachmentProperties[FileSystemTypeName], @@ -127,7 +129,7 @@ func (ra *ResourceAttachment) StartTimer(timeoutFunc func()) error { now := time.Now() duration := ra.ExpiresAt.Sub(now) if duration <= 0 { - return errors.Errorf("resource attachment: timer expiration is in the past; expiration [%s] < now [%s]", + return fmt.Errorf("resource attachment: timer expiration is in the past; expiration [%s] < now [%s]", ra.ExpiresAt.String(), now.String()) } logger.Info("Starting resource attachment ack timer", getResourceAttachmentLogFields(ra, duration)) @@ -174,6 +176,22 @@ func (ra *ResourceAttachment) SetSentStatus() { ra.AttachStatusSent = true } +// IsAttached checks if the resource attachment has been found attached on the host +func (ra *ResourceAttachment) IsAttached() bool { + ra.guard.RLock() + defer ra.guard.RUnlock() + + return ra.Status == status.AttachmentAttached +} + +// SetAttachedStatus marks the resouce attachment as attached once it's been found on the host +func (ra *ResourceAttachment) SetAttachedStatus() { + ra.guard.Lock() + defer ra.guard.Unlock() + + ra.Status = status.AttachmentAttached +} + // StopAckTimer stops the ack timer set on the resource attachment func (ra *ResourceAttachment) StopAckTimer() { ra.guard.Lock() @@ -190,3 +208,36 @@ func (ra *ResourceAttachment) HasExpired() bool { return time.Now().After(ra.ExpiresAt) } + +// SetError sets the error for a resource attachment if it can't be found. +func (ra *ResourceAttachment) SetError(err error) { + ra.guard.Lock() + defer ra.guard.Unlock() + ra.err = err +} + +// EBSToString returns a string representation of an EBS volume resource attachment. +func (ra *ResourceAttachment) EBSToString() string { + ra.guard.RLock() + defer ra.guard.RUnlock() + + return ra.ebsToStringUnsafe() +} + +func (ra *ResourceAttachment) ebsToStringUnsafe() string { + return fmt.Sprintf( + "Resource Attachment: attachment=%s attachmentType=%s attachmentSent=%t volumeSizeInGiB=%s requestedSizeName=%s volumeId=%s deviceName=%s filesystemType=%s status=%s expiresAt=%s error=%v", + ra.AttachmentARN, ra.AttachmentProperties[ResourceTypeName], ra.AttachStatusSent, ra.AttachmentProperties[VolumeSizeInGiBName], ra.AttachmentProperties[RequestedSizeName], ra.AttachmentProperties[VolumeIdName], + ra.AttachmentProperties[DeviceName], ra.AttachmentProperties[FileSystemTypeName], ra.Status.String(), ra.ExpiresAt.Format(time.RFC3339), ra.err) +} + +// GetAttachmentProperties returns the specific attachment property of the resource attachment object +func (ra *ResourceAttachment) GetAttachmentProperties(key string) string { + ra.guard.RLock() + defer ra.guard.RUnlock() + val, ok := ra.AttachmentProperties[key] + if ok { + return val + } + return "" +} diff --git a/agent/vendor/modules.txt b/agent/vendor/modules.txt index 48cc4221739..e0718e460c6 100644 --- a/agent/vendor/modules.txt +++ b/agent/vendor/modules.txt @@ -19,6 +19,7 @@ github.com/aws/amazon-ecs-agent/ecs-agent/api/eni github.com/aws/amazon-ecs-agent/ecs-agent/api/errors github.com/aws/amazon-ecs-agent/ecs-agent/api/mocks github.com/aws/amazon-ecs-agent/ecs-agent/api/resource +github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/mocks github.com/aws/amazon-ecs-agent/ecs-agent/api/status github.com/aws/amazon-ecs-agent/ecs-agent/credentials github.com/aws/amazon-ecs-agent/ecs-agent/credentials/mocks diff --git a/ecs-agent/api/resource/ebs_discovery.go b/ecs-agent/api/resource/ebs_discovery.go new file mode 100644 index 00000000000..77bf30b9456 --- /dev/null +++ b/ecs-agent/api/resource/ebs_discovery.go @@ -0,0 +1,64 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package resource + +import ( + "context" + "errors" + "fmt" + "strings" + "time" +) + +const ( + ebsnvmeIDTimeoutDuration = 5 * time.Second + ebsResourceKeyPrefix = "ebs-volume:" + ScanPeriod = 500 * time.Millisecond +) + +var ( + ErrInvalidVolumeID = errors.New("EBS volume IDs do not match") +) + +// EBSDiscoveryClient is used to scan and validate if an EBS volume is attached on the host instance. +type EBSDiscoveryClient struct { + ctx context.Context +} + +// NewDiscoveryClient creates a new EBSDiscoveryClient object +func NewDiscoveryClient(ctx context.Context) EBSDiscovery { + return &EBSDiscoveryClient{ + ctx: ctx, + } +} + +// ScanEBSVolumes will iterate through the entire list of pending EBS volume attachments within the agent state and checks if it's attached on the host. +func ScanEBSVolumes[T GenericEBSAttachmentObject](pendingAttachments map[string]T, dc EBSDiscovery) []string { + var err error + var foundVolumes []string + for key, ebs := range pendingAttachments { + volumeId := strings.TrimPrefix(key, ebsResourceKeyPrefix) + deviceName := ebs.GetAttachmentProperties(DeviceName) + err = dc.ConfirmEBSVolumeIsAttached(deviceName, volumeId) + if err != nil { + if !errors.Is(err, ErrInvalidVolumeID) { + err = fmt.Errorf("%w; failed to confirm if EBS volume is attached to the host", err) + } + ebs.SetError(err) + continue + } + foundVolumes = append(foundVolumes, key) + } + return foundVolumes +} diff --git a/ecs-agent/api/resource/ebs_discovery_linux.go b/ecs-agent/api/resource/ebs_discovery_linux.go new file mode 100644 index 00000000000..c8da7cd4d9b --- /dev/null +++ b/ecs-agent/api/resource/ebs_discovery_linux.go @@ -0,0 +1,92 @@ +//go:build linux +// +build linux + +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package resource + +import ( + "context" + "encoding/json" + "fmt" + "os/exec" + "strings" +) + +// LsblkOutput is used to manage and track the output of `lsblk` +type LsblkOutput struct { + BlockDevies []BD `json:"blockdevices"` +} +type BD struct { + Name string `json:"name"` + Serial string `json:"serial"` + Children []BDChild `json:"children"` +} +type BDChild struct { + Name string `json:"name"` + Serial string `json:"serial"` +} + +// ConfirmEBSVolumeIsAttached is used to scan for an EBS volume that's on the host with a specific device name and/or volume ID. +// There are two cases: +// 1. On nitro-based instance we check both device name and volume ID. +// 2. On xen-based instance we only check by the device name. +func (api *EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error { + var lsblkOut LsblkOutput + ctxWithTimeout, cancel := context.WithTimeout(api.ctx, ebsnvmeIDTimeoutDuration) + defer cancel() + + // The lsblk command will output the name and volume ID of all block devices on the host in JSON format + output, err := exec.CommandContext(ctxWithTimeout, "lsblk", "-o", "NAME,SERIAL", "-J").CombinedOutput() + if err != nil { + err = fmt.Errorf("%w; failed to run lsblk %v", err, string(output)) + return err + } + err = json.Unmarshal(output, &lsblkOut) + if err != nil { + err = fmt.Errorf("%w; failed to unmarshal string: %v", err, string(output)) + return err + } + + actualVolumeId, err := parseLsblkOutput(&lsblkOut, deviceName) + if err != nil { + return err + } + expectedVolumeId := strings.ReplaceAll(volumeID, "-", "") + + // On Xen-based instances, the volume ID can't be obtained and so we don't need to check by volume ID. + if actualVolumeId != "" && expectedVolumeId != actualVolumeId { + err = fmt.Errorf("%w; expected EBS volume %v but found %v", ErrInvalidVolumeID, volumeID, actualVolumeId) + return err + } + + return nil +} + +// parseLsblkOutput will parse the `lsblk` output and search for a EBS volume with a specific device name. +// Once found we return the volume ID, otherwise we return an empty string along with an error +// The output of the "lsblk -o +SERIAL" command looks like the following: +// NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT SERIAL +// nvme0n1 259:0 0 30G 0 disk vol123 +// ├─nvme0n1p1 259:1 0 30G 0 part / +// └─nvme0n1p128 259:2 0 1M 0 part +func parseLsblkOutput(output *LsblkOutput, deviceName string) (string, error) { + actualDeviceName := deviceName[strings.LastIndex(deviceName, "/")+1:] + for _, block := range output.BlockDevies { + if block.Name == actualDeviceName { + return block.Serial, nil + } + } + return "", fmt.Errorf("cannot find EBS volume with device name: %v", actualDeviceName) +} diff --git a/ecs-agent/api/resource/ebs_discovery_linux_test.go b/ecs-agent/api/resource/ebs_discovery_linux_test.go new file mode 100644 index 00000000000..9c53b81fe0a --- /dev/null +++ b/ecs-agent/api/resource/ebs_discovery_linux_test.go @@ -0,0 +1,64 @@ +//go:build linux && unit +// +build linux,unit + +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package resource + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testVolumeID = "vol-1234" + testDeviceName = "nvme0n1" +) + +func TestParseLsblkOutput(t *testing.T) { + blockDevice := BD{ + Name: testDeviceName, + Serial: testVolumeID, + Children: make([]BDChild, 0), + } + + lsblkOutput := &LsblkOutput{ + BlockDevies: []BD{ + blockDevice, + }, + } + + actualVolumeId, err := parseLsblkOutput(lsblkOutput, "/dev/"+testDeviceName) + require.NoError(t, err) + assert.Equal(t, testVolumeID, actualVolumeId) +} + +func TestParseLsblkOutputError(t *testing.T) { + blockDevice := BD{ + Name: "nvme1n1", + Serial: testVolumeID, + Children: make([]BDChild, 0), + } + + lsblkOutput := &LsblkOutput{ + BlockDevies: []BD{ + blockDevice, + }, + } + actualVolumeId, err := parseLsblkOutput(lsblkOutput, testDeviceName) + require.Error(t, err, "cannot find the device name: %v", "/dev/"+testDeviceName) + assert.Equal(t, "", actualVolumeId) +} diff --git a/ecs-agent/api/resource/ebs_discovery_windows.go b/ecs-agent/api/resource/ebs_discovery_windows.go new file mode 100644 index 00000000000..425844610c1 --- /dev/null +++ b/ecs-agent/api/resource/ebs_discovery_windows.go @@ -0,0 +1,21 @@ +//go:build windows +// +build windows + +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package resource + +func (api EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error { + return nil +} diff --git a/ecs-agent/api/resource/generate_mocks.go b/ecs-agent/api/resource/generate_mocks.go new file mode 100644 index 00000000000..a66369fc121 --- /dev/null +++ b/ecs-agent/api/resource/generate_mocks.go @@ -0,0 +1,16 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +//go:generate mockgen -destination=mocks/ebs_mocks.go -copyright_file=../../../scripts/copyright_file github.com/aws/amazon-ecs-agent/ecs-agent/api/resource EBSDiscovery + +package resource diff --git a/ecs-agent/api/resource/interfaces.go b/ecs-agent/api/resource/interfaces.go new file mode 100644 index 00000000000..9be152f1b3f --- /dev/null +++ b/ecs-agent/api/resource/interfaces.go @@ -0,0 +1,27 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package resource + +// EBSDiscovery is an interface used to find EBS volumes that are attached onto the host instance. It is implemented by +// EBSDiscoveryClient +type EBSDiscovery interface { + ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error +} + +// GenericEBSAttachmentObject is an interface used to implement the Resource attachment objects that's saved within the agent state +type GenericEBSAttachmentObject interface { + GetAttachmentProperties(key string) string + EBSToString() string + SetError(err error) +} diff --git a/ecs-agent/api/resource/mocks/ebs_mocks.go b/ecs-agent/api/resource/mocks/ebs_mocks.go new file mode 100644 index 00000000000..7e8eb5918ce --- /dev/null +++ b/ecs-agent/api/resource/mocks/ebs_mocks.go @@ -0,0 +1,62 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. +// + +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/aws/amazon-ecs-agent/ecs-agent/api/resource (interfaces: EBSDiscovery) + +// Package mock_resource is a generated GoMock package. +package mock_resource + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockEBSDiscovery is a mock of EBSDiscovery interface. +type MockEBSDiscovery struct { + ctrl *gomock.Controller + recorder *MockEBSDiscoveryMockRecorder +} + +// MockEBSDiscoveryMockRecorder is the mock recorder for MockEBSDiscovery. +type MockEBSDiscoveryMockRecorder struct { + mock *MockEBSDiscovery +} + +// NewMockEBSDiscovery creates a new mock instance. +func NewMockEBSDiscovery(ctrl *gomock.Controller) *MockEBSDiscovery { + mock := &MockEBSDiscovery{ctrl: ctrl} + mock.recorder = &MockEBSDiscoveryMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockEBSDiscovery) EXPECT() *MockEBSDiscoveryMockRecorder { + return m.recorder +} + +// ConfirmEBSVolumeIsAttached mocks base method. +func (m *MockEBSDiscovery) ConfirmEBSVolumeIsAttached(arg0, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConfirmEBSVolumeIsAttached", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// ConfirmEBSVolumeIsAttached indicates an expected call of ConfirmEBSVolumeIsAttached. +func (mr *MockEBSDiscoveryMockRecorder) ConfirmEBSVolumeIsAttached(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfirmEBSVolumeIsAttached", reflect.TypeOf((*MockEBSDiscovery)(nil).ConfirmEBSVolumeIsAttached), arg0, arg1) +} diff --git a/ecs-agent/api/resource/resource_attachment.go b/ecs-agent/api/resource/resource_attachment.go index ff81db0af0b..78ae3591a0c 100644 --- a/ecs-agent/api/resource/resource_attachment.go +++ b/ecs-agent/api/resource/resource_attachment.go @@ -14,14 +14,15 @@ package resource import ( + "errors" + "fmt" "sync" "time" "github.com/aws/amazon-ecs-agent/ecs-agent/api/attachmentinfo" + "github.com/aws/amazon-ecs-agent/ecs-agent/api/status" "github.com/aws/amazon-ecs-agent/ecs-agent/logger" "github.com/aws/amazon-ecs-agent/ecs-agent/utils/ttime" - - "github.com/pkg/errors" ) type ResourceAttachment struct { @@ -38,6 +39,7 @@ type ResourceAttachment struct { ackTimer ttime.Timer // guard protects access to fields of this struct guard sync.RWMutex + err error } // Agent Communication Service (ACS) can send messages of type ConfirmAttachmentMessage. These messages include @@ -104,7 +106,7 @@ func getResourceAttachmentLogFields(ra *ResourceAttachment, duration time.Durati "attachmentType": ra.AttachmentProperties[ResourceTypeName], "attachmentSent": ra.AttachStatusSent, "volumeSizeInGiB": ra.AttachmentProperties[VolumeSizeInGiBName], - "RequestedSizeName": ra.AttachmentProperties[RequestedSizeName], + "requestedSizeName": ra.AttachmentProperties[RequestedSizeName], "volumeId": ra.AttachmentProperties[VolumeIdName], "deviceName": ra.AttachmentProperties[DeviceName], "filesystemType": ra.AttachmentProperties[FileSystemTypeName], @@ -127,7 +129,7 @@ func (ra *ResourceAttachment) StartTimer(timeoutFunc func()) error { now := time.Now() duration := ra.ExpiresAt.Sub(now) if duration <= 0 { - return errors.Errorf("resource attachment: timer expiration is in the past; expiration [%s] < now [%s]", + return fmt.Errorf("resource attachment: timer expiration is in the past; expiration [%s] < now [%s]", ra.ExpiresAt.String(), now.String()) } logger.Info("Starting resource attachment ack timer", getResourceAttachmentLogFields(ra, duration)) @@ -174,6 +176,22 @@ func (ra *ResourceAttachment) SetSentStatus() { ra.AttachStatusSent = true } +// IsAttached checks if the resource attachment has been found attached on the host +func (ra *ResourceAttachment) IsAttached() bool { + ra.guard.RLock() + defer ra.guard.RUnlock() + + return ra.Status == status.AttachmentAttached +} + +// SetAttachedStatus marks the resouce attachment as attached once it's been found on the host +func (ra *ResourceAttachment) SetAttachedStatus() { + ra.guard.Lock() + defer ra.guard.Unlock() + + ra.Status = status.AttachmentAttached +} + // StopAckTimer stops the ack timer set on the resource attachment func (ra *ResourceAttachment) StopAckTimer() { ra.guard.Lock() @@ -190,3 +208,36 @@ func (ra *ResourceAttachment) HasExpired() bool { return time.Now().After(ra.ExpiresAt) } + +// SetError sets the error for a resource attachment if it can't be found. +func (ra *ResourceAttachment) SetError(err error) { + ra.guard.Lock() + defer ra.guard.Unlock() + ra.err = err +} + +// EBSToString returns a string representation of an EBS volume resource attachment. +func (ra *ResourceAttachment) EBSToString() string { + ra.guard.RLock() + defer ra.guard.RUnlock() + + return ra.ebsToStringUnsafe() +} + +func (ra *ResourceAttachment) ebsToStringUnsafe() string { + return fmt.Sprintf( + "Resource Attachment: attachment=%s attachmentType=%s attachmentSent=%t volumeSizeInGiB=%s requestedSizeName=%s volumeId=%s deviceName=%s filesystemType=%s status=%s expiresAt=%s error=%v", + ra.AttachmentARN, ra.AttachmentProperties[ResourceTypeName], ra.AttachStatusSent, ra.AttachmentProperties[VolumeSizeInGiBName], ra.AttachmentProperties[RequestedSizeName], ra.AttachmentProperties[VolumeIdName], + ra.AttachmentProperties[DeviceName], ra.AttachmentProperties[FileSystemTypeName], ra.Status.String(), ra.ExpiresAt.Format(time.RFC3339), ra.err) +} + +// GetAttachmentProperties returns the specific attachment property of the resource attachment object +func (ra *ResourceAttachment) GetAttachmentProperties(key string) string { + ra.guard.RLock() + defer ra.guard.RUnlock() + val, ok := ra.AttachmentProperties[key] + if ok { + return val + } + return "" +} From c0995503769f010622dce3389a1d67a118f57141 Mon Sep 17 00:00:00 2001 From: saurabhc123 <12913032+saurabhc123@users.noreply.github.com> Date: Tue, 5 Sep 2023 16:38:05 -0400 Subject: [PATCH 2/3] Added the implementation for EBS volume discovery on Windows (#49) * Added the implementation for EBS volume discovery on Windows * incorporated the review comments. Also fixed the name of the timeout variable. * incorporated the review comments. Also fixed the name of the timeout variable. --- .../ecs-agent/api/resource/ebs_discovery.go | 34 +++--- .../api/resource/ebs_discovery_linux.go | 2 +- .../api/resource/ebs_discovery_windows.go | 102 +++++++++++++++++- ecs-agent/api/resource/ebs_discovery.go | 34 +++--- ecs-agent/api/resource/ebs_discovery_linux.go | 2 +- .../api/resource/ebs_discovery_windows.go | 102 +++++++++++++++++- .../resource/ebs_discovery_windows_test.go | 87 +++++++++++++++ 7 files changed, 333 insertions(+), 30 deletions(-) create mode 100644 ecs-agent/api/resource/ebs_discovery_windows_test.go diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go index 77bf30b9456..54185d09769 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go @@ -15,47 +15,55 @@ package resource import ( "context" - "errors" - "fmt" "strings" "time" + + "github.com/aws/amazon-ecs-agent/ecs-agent/logger" + + "github.com/pkg/errors" ) const ( - ebsnvmeIDTimeoutDuration = 5 * time.Second - ebsResourceKeyPrefix = "ebs-volume:" - ScanPeriod = 500 * time.Millisecond + ebsVolumeDiscoveryTimeout = 5 * time.Second + ebsResourceKeyPrefix = "ebs-volume:" + ScanPeriod = 500 * time.Millisecond ) var ( ErrInvalidVolumeID = errors.New("EBS volume IDs do not match") ) -// EBSDiscoveryClient is used to scan and validate if an EBS volume is attached on the host instance. type EBSDiscoveryClient struct { ctx context.Context } -// NewDiscoveryClient creates a new EBSDiscoveryClient object -func NewDiscoveryClient(ctx context.Context) EBSDiscovery { +func NewDiscoveryClient(ctx context.Context) *EBSDiscoveryClient { return &EBSDiscoveryClient{ ctx: ctx, } } // ScanEBSVolumes will iterate through the entire list of pending EBS volume attachments within the agent state and checks if it's attached on the host. -func ScanEBSVolumes[T GenericEBSAttachmentObject](pendingAttachments map[string]T, dc EBSDiscovery) []string { +func ScanEBSVolumes[T GenericEBSAttachmentObject](t map[string]T, dc EBSDiscovery) []string { var err error var foundVolumes []string - for key, ebs := range pendingAttachments { + for key, ebs := range t { volumeId := strings.TrimPrefix(key, ebsResourceKeyPrefix) deviceName := ebs.GetAttachmentProperties(DeviceName) err = dc.ConfirmEBSVolumeIsAttached(deviceName, volumeId) if err != nil { - if !errors.Is(err, ErrInvalidVolumeID) { - err = fmt.Errorf("%w; failed to confirm if EBS volume is attached to the host", err) + if err == ErrInvalidVolumeID || errors.Cause(err) == ErrInvalidVolumeID { + logger.Warn("Found a different EBS volume attached to the host. Expected EBS volume:", logger.Fields{ + "volumeId": volumeId, + "deviceName": deviceName, + }) + } else { + logger.Warn("Failed to confirm if EBS volume is attached to the host. ", logger.Fields{ + "volumeId": volumeId, + "deviceName": deviceName, + "error": err, + }) } - ebs.SetError(err) continue } foundVolumes = append(foundVolumes, key) diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go index c8da7cd4d9b..8c3c63cdb30 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go @@ -44,7 +44,7 @@ type BDChild struct { // 2. On xen-based instance we only check by the device name. func (api *EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error { var lsblkOut LsblkOutput - ctxWithTimeout, cancel := context.WithTimeout(api.ctx, ebsnvmeIDTimeoutDuration) + ctxWithTimeout, cancel := context.WithTimeout(api.ctx, ebsVolumeDiscoveryTimeout) defer cancel() // The lsblk command will output the name and volume ID of all block devices on the host in JSON format diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_windows.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_windows.go index 425844610c1..581cda967d7 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_windows.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_windows.go @@ -16,6 +16,106 @@ package resource -func (api EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error { +import ( + "context" + "fmt" + "os/exec" + "strings" + + log "github.com/cihub/seelog" + "github.com/pkg/errors" +) + +const ( + diskNumberOffset = 0 + volumeIdOffset = 1 + deviceNameOffset = 2 + volumeInfoLength = 3 +) + +func (api *EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error { + ctxWithTimeout, cancel := context.WithTimeout(api.ctx, ebsVolumeDiscoveryTimeout) + defer cancel() + output, err := exec.CommandContext(ctxWithTimeout, + "C:\\PROGRAMDATA\\Amazon\\Tools\\ebsnvme-id.exe").CombinedOutput() + if err != nil { + return errors.Wrapf(err, "failed to run ebsnvme-id.exe: %s", string(output)) + } + + _, err = parseExecutableOutput(output, volumeID, deviceName) + if err != nil { + return errors.Wrapf(err, "failed to parse ebsnvme-id.exe output for volumeID: %s and deviceName: %s", + volumeID, deviceName) + } + + log.Info(fmt.Sprintf("found volume with volumeID: %s and deviceName: %s", volumeID, deviceName)) + return nil } + +// parseExecutableOutput parses the output of `ebsnvme-id.exe` and returns the volumeId. +func parseExecutableOutput(output []byte, candidateVolumeId string, candidateDeviceName string) (string, error) { + /* The output of the ebsnvme-id.exe is emitted like the following: + Disk Number: 0 + Volume ID: vol-0a1234f340444abcd + Device Name: sda1 + + Disk Number: 1 + Volume ID: vol-abcdef1234567890a + Device Name: /dev/sdf */ + + out := string(output) + // Replace double line with a single line and split based on single line + volumeInfo := strings.Split(strings.Replace(string(out), "\r\n\r\n", "\r\n", -1), "\r\n") + + if len(volumeInfo) < volumeInfoLength { + return "", errors.New("cannot find the volume ID. Encountered error message: " + out) + } + + //Read every 3 lines of disk information + for volumeIndex := 0; volumeIndex <= len(volumeInfo)-volumeInfoLength; volumeIndex = volumeIndex + volumeInfoLength { + _, volumeId, deviceName, err := parseSet(volumeInfo[volumeIndex : volumeIndex+volumeInfoLength]) + if err != nil { + return "", errors.Wrapf(err, "failed to parse the output for volumeID: %s and deviceName: %s. "+ + "Output:%s", candidateVolumeId, candidateDeviceName, out) + } + + if volumeId == candidateVolumeId && deviceName == candidateDeviceName { + return volumeId, nil + } + + } + + return "", errors.New("cannot find the volume ID:" + candidateVolumeId) +} + +// parseSet parses the single volume information that is 3 lines long +func parseSet(lines []string) (string, string, string, error) { + if len(lines) != 3 { + return "", "", "", errors.New("the number of entries in the volume information is insufficient to parse. Expected 3 lines") + } + + diskNumber, err := parseValue(lines[diskNumberOffset], "Disk Number:") + if err != nil { + return "", "", "", err + } + volumeId, err := parseValue(lines[volumeIdOffset], "Volume ID:") + if err != nil { + return "", "", "", err + } + deviceName, err := parseValue(lines[deviceNameOffset], "Device Name:") + if err != nil { + return "", "", "", err + } + return diskNumber, volumeId, deviceName, nil +} + +// parseValue looks for the volume information identifier and replaces it to return just the value +func parseValue(inputBuffer string, stringToTrim string) (string, error) { + // if the input buffer doesn't have the identifier for the information, return an error + if !strings.Contains(inputBuffer, stringToTrim) { + return "", errors.New("output buffer was missing the string:" + stringToTrim) + } + + return strings.TrimSpace(strings.Replace(inputBuffer, stringToTrim, "", -1)), nil +} diff --git a/ecs-agent/api/resource/ebs_discovery.go b/ecs-agent/api/resource/ebs_discovery.go index 77bf30b9456..54185d09769 100644 --- a/ecs-agent/api/resource/ebs_discovery.go +++ b/ecs-agent/api/resource/ebs_discovery.go @@ -15,47 +15,55 @@ package resource import ( "context" - "errors" - "fmt" "strings" "time" + + "github.com/aws/amazon-ecs-agent/ecs-agent/logger" + + "github.com/pkg/errors" ) const ( - ebsnvmeIDTimeoutDuration = 5 * time.Second - ebsResourceKeyPrefix = "ebs-volume:" - ScanPeriod = 500 * time.Millisecond + ebsVolumeDiscoveryTimeout = 5 * time.Second + ebsResourceKeyPrefix = "ebs-volume:" + ScanPeriod = 500 * time.Millisecond ) var ( ErrInvalidVolumeID = errors.New("EBS volume IDs do not match") ) -// EBSDiscoveryClient is used to scan and validate if an EBS volume is attached on the host instance. type EBSDiscoveryClient struct { ctx context.Context } -// NewDiscoveryClient creates a new EBSDiscoveryClient object -func NewDiscoveryClient(ctx context.Context) EBSDiscovery { +func NewDiscoveryClient(ctx context.Context) *EBSDiscoveryClient { return &EBSDiscoveryClient{ ctx: ctx, } } // ScanEBSVolumes will iterate through the entire list of pending EBS volume attachments within the agent state and checks if it's attached on the host. -func ScanEBSVolumes[T GenericEBSAttachmentObject](pendingAttachments map[string]T, dc EBSDiscovery) []string { +func ScanEBSVolumes[T GenericEBSAttachmentObject](t map[string]T, dc EBSDiscovery) []string { var err error var foundVolumes []string - for key, ebs := range pendingAttachments { + for key, ebs := range t { volumeId := strings.TrimPrefix(key, ebsResourceKeyPrefix) deviceName := ebs.GetAttachmentProperties(DeviceName) err = dc.ConfirmEBSVolumeIsAttached(deviceName, volumeId) if err != nil { - if !errors.Is(err, ErrInvalidVolumeID) { - err = fmt.Errorf("%w; failed to confirm if EBS volume is attached to the host", err) + if err == ErrInvalidVolumeID || errors.Cause(err) == ErrInvalidVolumeID { + logger.Warn("Found a different EBS volume attached to the host. Expected EBS volume:", logger.Fields{ + "volumeId": volumeId, + "deviceName": deviceName, + }) + } else { + logger.Warn("Failed to confirm if EBS volume is attached to the host. ", logger.Fields{ + "volumeId": volumeId, + "deviceName": deviceName, + "error": err, + }) } - ebs.SetError(err) continue } foundVolumes = append(foundVolumes, key) diff --git a/ecs-agent/api/resource/ebs_discovery_linux.go b/ecs-agent/api/resource/ebs_discovery_linux.go index c8da7cd4d9b..8c3c63cdb30 100644 --- a/ecs-agent/api/resource/ebs_discovery_linux.go +++ b/ecs-agent/api/resource/ebs_discovery_linux.go @@ -44,7 +44,7 @@ type BDChild struct { // 2. On xen-based instance we only check by the device name. func (api *EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error { var lsblkOut LsblkOutput - ctxWithTimeout, cancel := context.WithTimeout(api.ctx, ebsnvmeIDTimeoutDuration) + ctxWithTimeout, cancel := context.WithTimeout(api.ctx, ebsVolumeDiscoveryTimeout) defer cancel() // The lsblk command will output the name and volume ID of all block devices on the host in JSON format diff --git a/ecs-agent/api/resource/ebs_discovery_windows.go b/ecs-agent/api/resource/ebs_discovery_windows.go index 425844610c1..581cda967d7 100644 --- a/ecs-agent/api/resource/ebs_discovery_windows.go +++ b/ecs-agent/api/resource/ebs_discovery_windows.go @@ -16,6 +16,106 @@ package resource -func (api EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error { +import ( + "context" + "fmt" + "os/exec" + "strings" + + log "github.com/cihub/seelog" + "github.com/pkg/errors" +) + +const ( + diskNumberOffset = 0 + volumeIdOffset = 1 + deviceNameOffset = 2 + volumeInfoLength = 3 +) + +func (api *EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID string) error { + ctxWithTimeout, cancel := context.WithTimeout(api.ctx, ebsVolumeDiscoveryTimeout) + defer cancel() + output, err := exec.CommandContext(ctxWithTimeout, + "C:\\PROGRAMDATA\\Amazon\\Tools\\ebsnvme-id.exe").CombinedOutput() + if err != nil { + return errors.Wrapf(err, "failed to run ebsnvme-id.exe: %s", string(output)) + } + + _, err = parseExecutableOutput(output, volumeID, deviceName) + if err != nil { + return errors.Wrapf(err, "failed to parse ebsnvme-id.exe output for volumeID: %s and deviceName: %s", + volumeID, deviceName) + } + + log.Info(fmt.Sprintf("found volume with volumeID: %s and deviceName: %s", volumeID, deviceName)) + return nil } + +// parseExecutableOutput parses the output of `ebsnvme-id.exe` and returns the volumeId. +func parseExecutableOutput(output []byte, candidateVolumeId string, candidateDeviceName string) (string, error) { + /* The output of the ebsnvme-id.exe is emitted like the following: + Disk Number: 0 + Volume ID: vol-0a1234f340444abcd + Device Name: sda1 + + Disk Number: 1 + Volume ID: vol-abcdef1234567890a + Device Name: /dev/sdf */ + + out := string(output) + // Replace double line with a single line and split based on single line + volumeInfo := strings.Split(strings.Replace(string(out), "\r\n\r\n", "\r\n", -1), "\r\n") + + if len(volumeInfo) < volumeInfoLength { + return "", errors.New("cannot find the volume ID. Encountered error message: " + out) + } + + //Read every 3 lines of disk information + for volumeIndex := 0; volumeIndex <= len(volumeInfo)-volumeInfoLength; volumeIndex = volumeIndex + volumeInfoLength { + _, volumeId, deviceName, err := parseSet(volumeInfo[volumeIndex : volumeIndex+volumeInfoLength]) + if err != nil { + return "", errors.Wrapf(err, "failed to parse the output for volumeID: %s and deviceName: %s. "+ + "Output:%s", candidateVolumeId, candidateDeviceName, out) + } + + if volumeId == candidateVolumeId && deviceName == candidateDeviceName { + return volumeId, nil + } + + } + + return "", errors.New("cannot find the volume ID:" + candidateVolumeId) +} + +// parseSet parses the single volume information that is 3 lines long +func parseSet(lines []string) (string, string, string, error) { + if len(lines) != 3 { + return "", "", "", errors.New("the number of entries in the volume information is insufficient to parse. Expected 3 lines") + } + + diskNumber, err := parseValue(lines[diskNumberOffset], "Disk Number:") + if err != nil { + return "", "", "", err + } + volumeId, err := parseValue(lines[volumeIdOffset], "Volume ID:") + if err != nil { + return "", "", "", err + } + deviceName, err := parseValue(lines[deviceNameOffset], "Device Name:") + if err != nil { + return "", "", "", err + } + return diskNumber, volumeId, deviceName, nil +} + +// parseValue looks for the volume information identifier and replaces it to return just the value +func parseValue(inputBuffer string, stringToTrim string) (string, error) { + // if the input buffer doesn't have the identifier for the information, return an error + if !strings.Contains(inputBuffer, stringToTrim) { + return "", errors.New("output buffer was missing the string:" + stringToTrim) + } + + return strings.TrimSpace(strings.Replace(inputBuffer, stringToTrim, "", -1)), nil +} diff --git a/ecs-agent/api/resource/ebs_discovery_windows_test.go b/ecs-agent/api/resource/ebs_discovery_windows_test.go new file mode 100644 index 00000000000..27fb2036aed --- /dev/null +++ b/ecs-agent/api/resource/ebs_discovery_windows_test.go @@ -0,0 +1,87 @@ +//go:build windows && unit +// +build windows,unit + +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package resource + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testVolumeID = "vol-0a1234f340444abcd" + deviceName = "/dev/sdf" +) + +func TestParseExecutableOutputWithHappyPath(t *testing.T) { + output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nVolume ID: %s\r\nDevice Name: %s\r\n\r\n", testVolumeID, deviceName) + parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, deviceName) + require.NoError(t, err) + assert.True(t, strings.Contains(parsedOutput, testVolumeID)) +} + +func TestParseExecutableOutputWithMissingDiskNumber(t *testing.T) { + output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nVolume ID: %s\r\nDevice Name: %s\r\n\r\n", testVolumeID, deviceName) + parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, deviceName) + require.Error(t, err) + assert.Equal(t, "", parsedOutput) +} + +func TestParseExecutableOutputWithMissingVolumeInformation(t *testing.T) { + output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nDevice Name: %s\r\n\r\n", deviceName) + parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, deviceName) + require.Error(t, err) + assert.Equal(t, "", parsedOutput) +} + +func TestParseExecutableOutputWithMissingDeviceName(t *testing.T) { + output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nVolume ID: %s\r\n\r\n", testVolumeID) + parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, deviceName) + require.Error(t, err) + assert.Equal(t, "", parsedOutput) +} + +func TestParseExecutableOutputWithVolumeNameMismatch(t *testing.T) { + output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nVolume ID: %s\r\nDevice Name: %s\r\n\r\n", testVolumeID, deviceName) + parsedOutput, err := parseExecutableOutput([]byte(output), "MismatchedVolumeName", deviceName) + require.Error(t, err) + assert.Equal(t, "", parsedOutput) +} + +func TestParseExecutableOutputWithDeviceNameMismatch(t *testing.T) { + output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nVolume ID: %s\r\nDevice Name: %s\r\n\r\n", testVolumeID, deviceName) + parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, "MismatchedDeviceName") + require.Error(t, err) + assert.Equal(t, "", parsedOutput) +} + +func TestParseExecutableOutputWithTruncatedOutputBuffer(t *testing.T) { + output := "Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nVolume ID: TruncatedBuffer..." + parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, deviceName) + require.Error(t, err) + assert.Equal(t, "", parsedOutput) +} + +func TestParseExecutableOutputWithUnexpectedOutput(t *testing.T) { + output := "No EBS NVMe disks found." + parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, deviceName) + require.Error(t, err, "cannot find the volume ID: %s", output) + assert.Equal(t, "", parsedOutput) +} From e47cb40979326c8400f981196fdb7eabbb695771 Mon Sep 17 00:00:00 2001 From: mye956 Date: Wed, 6 Sep 2023 03:17:30 +0000 Subject: [PATCH 3/3] Fix shared EBS discovery implementation --- agent/ebs/watcher.go | 8 +-- agent/ebs/watcher_test.go | 54 +++++++++++++++++++ .../dockerstate/docker_task_engine_state.go | 4 +- .../ecs-agent/api/resource/ebs_discovery.go | 28 ++++------ .../api/resource/ebs_discovery_linux.go | 33 ++++++------ .../api/resource/resource_attachment.go | 8 +++ ecs-agent/api/resource/ebs_discovery.go | 28 ++++------ ecs-agent/api/resource/ebs_discovery_linux.go | 33 ++++++------ .../api/resource/ebs_discovery_linux_test.go | 12 ++--- .../resource/ebs_discovery_windows_test.go | 45 +++++++++++++--- ecs-agent/api/resource/resource_attachment.go | 8 +++ 11 files changed, 175 insertions(+), 86 deletions(-) diff --git a/agent/ebs/watcher.go b/agent/ebs/watcher.go index 48be77e41f7..10238a5488f 100644 --- a/agent/ebs/watcher.go +++ b/agent/ebs/watcher.go @@ -90,10 +90,12 @@ func (w *EBSWatcher) HandleResourceAttachment(ebs *apiebs.ResourceAttachment) er } volumeId := ebs.GetAttachmentProperties(apiebs.VolumeIdName) - _, ok := w.agentState.GetEBSByVolumeId(volumeId) + ebsAttachment, ok := w.agentState.GetEBSByVolumeId(volumeId) if ok { log.Infof("EBS Volume attachment already exists. Skip handling EBS attachment %v.", ebs.EBSToString()) - return nil + return ebsAttachment.StartTimer(func() { + w.handleEBSAckTimeout(volumeId) + }) } if err := w.addEBSAttachmentToState(ebs); err != nil { @@ -116,7 +118,7 @@ func (w *EBSWatcher) notifyFoundEBS(volumeId string) { // TODO: Add the EBS volume to data client ebs, ok := w.agentState.GetEBSByVolumeId(volumeId) if !ok { - log.Warnf("Unable to find EBS volume with volume ID: %v.", volumeId) + log.Warnf("Unable to find EBS volume with volume ID: %v within agent state.", volumeId) return } diff --git a/agent/ebs/watcher_test.go b/agent/ebs/watcher_test.go index 7ee370e97e1..657931d1b1f 100644 --- a/agent/ebs/watcher_test.go +++ b/agent/ebs/watcher_test.go @@ -18,6 +18,7 @@ package ebs import ( "context" + "fmt" "sync" "testing" "time" @@ -313,3 +314,56 @@ func TestHandleEBSAckTimeout(t *testing.T) { ebsAttachment, ok := taskEngineState.(*dockerstate.DockerTaskEngineState).GetEBSByVolumeId(volumeID) assert.False(t, ok) } + +// TestHandleMismatchEBSAttachment tests handling an EBS attachment but found a different volume attached +// onto the host during the scanning process. +func TestHandleMismatchEBSAttachment(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + ctx := context.Background() + taskEngineState := dockerstate.NewTaskEngineState() + eventChannel := make(chan statechange.Event) + mockDiscoveryClient := mock_ebs_discovery.NewMockEBSDiscovery(mockCtrl) + + watcher := newTestEBSWatcher(ctx, taskEngineState, eventChannel, mockDiscoveryClient) + + testAttachmentProperties := map[string]string{ + apiebs.ResourceTypeName: apiebs.ElasticBlockStorage, + apiebs.DeviceName: deviceName, + apiebs.VolumeIdName: volumeID, + } + + expiresAt := time.Now().Add(time.Millisecond * testconst.WaitTimeoutMillis) + ebsAttachment := &apiebs.ResourceAttachment{ + AttachmentInfo: attachmentinfo.AttachmentInfo{ + TaskARN: taskARN, + TaskClusterARN: taskClusterARN, + ContainerInstanceARN: containerInstanceARN, + ExpiresAt: expiresAt, + Status: status.AttachmentNone, + AttachmentARN: resourceAttachmentARN, + }, + AttachmentProperties: testAttachmentProperties, + } + + var wg sync.WaitGroup + wg.Add(1) + mockDiscoveryClient.EXPECT().ConfirmEBSVolumeIsAttached(deviceName, volumeID). + Do(func(deviceName, volumeID string) { + wg.Done() + }). + Return(fmt.Errorf("%w; expected EBS volume %s but found %s", apiebs.ErrInvalidVolumeID, volumeID, "vol-321")). + MinTimes(1) + + err := watcher.HandleResourceAttachment(ebsAttachment) + assert.NoError(t, err) + + pendingEBS := watcher.agentState.GetAllPendingEBSAttachmentsWithKey() + foundVolumes := apiebs.ScanEBSVolumes(pendingEBS, watcher.discoveryClient) + + assert.Empty(t, foundVolumes) + ebsAttachment, ok := taskEngineState.(*dockerstate.DockerTaskEngineState).GetEBSByVolumeId(volumeID) + assert.True(t, ok) + assert.ErrorIs(t, ebsAttachment.GetError(), apiebs.ErrInvalidVolumeID) +} diff --git a/agent/engine/dockerstate/docker_task_engine_state.go b/agent/engine/dockerstate/docker_task_engine_state.go index 2e5803f9f8f..92c7b10f7df 100644 --- a/agent/engine/dockerstate/docker_task_engine_state.go +++ b/agent/engine/dockerstate/docker_task_engine_state.go @@ -300,7 +300,7 @@ func (state *DockerTaskEngineState) GetAllPendingEBSAttachments() []*apiresource func (state *DockerTaskEngineState) allPendingEBSAttachmentsUnsafe() []*apiresource.ResourceAttachment { var pendingEBSAttachments []*apiresource.ResourceAttachment for _, v := range state.ebsAttachments { - if !v.IsAttached() { + if !v.IsAttached() && !v.IsSent() { pendingEBSAttachments = append(pendingEBSAttachments, v) } } @@ -319,7 +319,7 @@ func (state *DockerTaskEngineState) GetAllPendingEBSAttachmentsWithKey() map[str func (state *DockerTaskEngineState) allPendingEBSAttachmentsWithKeyUnsafe() map[string]*apiresource.ResourceAttachment { pendingEBSAttachments := make(map[string]*apiresource.ResourceAttachment) for k, v := range state.ebsAttachments { - if !v.IsAttached() { + if !v.IsAttached() && !v.IsSent() { pendingEBSAttachments[k] = v } } diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go index 54185d09769..687a419436d 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery.go @@ -15,16 +15,14 @@ package resource import ( "context" + "errors" + "fmt" "strings" "time" - - "github.com/aws/amazon-ecs-agent/ecs-agent/logger" - - "github.com/pkg/errors" ) const ( - ebsVolumeDiscoveryTimeout = 5 * time.Second + ebsVolumeDiscoveryTimeout = 300 * time.Second ebsResourceKeyPrefix = "ebs-volume:" ScanPeriod = 500 * time.Millisecond ) @@ -43,27 +41,19 @@ func NewDiscoveryClient(ctx context.Context) *EBSDiscoveryClient { } } -// ScanEBSVolumes will iterate through the entire list of pending EBS volume attachments within the agent state and checks if it's attached on the host. -func ScanEBSVolumes[T GenericEBSAttachmentObject](t map[string]T, dc EBSDiscovery) []string { +// ScanEBSVolumes will iterate through the entire list of provided EBS volume attachments within the agent state and checks if it's attached on the host. +func ScanEBSVolumes[T GenericEBSAttachmentObject](pendingAttachments map[string]T, dc EBSDiscovery) []string { var err error var foundVolumes []string - for key, ebs := range t { + for key, ebs := range pendingAttachments { volumeId := strings.TrimPrefix(key, ebsResourceKeyPrefix) deviceName := ebs.GetAttachmentProperties(DeviceName) err = dc.ConfirmEBSVolumeIsAttached(deviceName, volumeId) if err != nil { - if err == ErrInvalidVolumeID || errors.Cause(err) == ErrInvalidVolumeID { - logger.Warn("Found a different EBS volume attached to the host. Expected EBS volume:", logger.Fields{ - "volumeId": volumeId, - "deviceName": deviceName, - }) - } else { - logger.Warn("Failed to confirm if EBS volume is attached to the host. ", logger.Fields{ - "volumeId": volumeId, - "deviceName": deviceName, - "error": err, - }) + if !errors.Is(err, ErrInvalidVolumeID) { + err = fmt.Errorf("%w; failed to confirm if EBS volume is attached to the host", err) } + ebs.SetError(err) continue } foundVolumes = append(foundVolumes, key) diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go index 8c3c63cdb30..8ca54a23393 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/ebs_discovery_linux.go @@ -26,16 +26,12 @@ import ( // LsblkOutput is used to manage and track the output of `lsblk` type LsblkOutput struct { - BlockDevies []BD `json:"blockdevices"` + BlockDevices []BlockDevice `json:"blockdevices"` } -type BD struct { - Name string `json:"name"` - Serial string `json:"serial"` - Children []BDChild `json:"children"` -} -type BDChild struct { - Name string `json:"name"` - Serial string `json:"serial"` +type BlockDevice struct { + Name string `json:"name"` + Serial string `json:"serial"` + Children []*BlockDevice `json:"children,omitempty"` } // ConfirmEBSVolumeIsAttached is used to scan for an EBS volume that's on the host with a specific device name and/or volume ID. @@ -76,14 +72,21 @@ func (api *EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID s // parseLsblkOutput will parse the `lsblk` output and search for a EBS volume with a specific device name. // Once found we return the volume ID, otherwise we return an empty string along with an error -// The output of the "lsblk -o +SERIAL" command looks like the following: -// NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT SERIAL -// nvme0n1 259:0 0 30G 0 disk vol123 -// ├─nvme0n1p1 259:1 0 30G 0 part / -// └─nvme0n1p128 259:2 0 1M 0 part +// The output of the "lsblk -o NAME,SERIAL -J" command looks like the following: +// +// { +// "blockdevices": [ +// {"name": "nvme0n1", "serial": "vol087768edff8511a23", +// "children": [ +// {"name": "nvme0n1p1", "serial": null}, +// {"name": "nvme0n1p128", "serial": null} +// ] +// } +// ] +// } func parseLsblkOutput(output *LsblkOutput, deviceName string) (string, error) { actualDeviceName := deviceName[strings.LastIndex(deviceName, "/")+1:] - for _, block := range output.BlockDevies { + for _, block := range output.BlockDevices { if block.Name == actualDeviceName { return block.Serial, nil } diff --git a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/resource_attachment.go b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/resource_attachment.go index 78ae3591a0c..da4a5e12d0b 100644 --- a/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/resource_attachment.go +++ b/agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/api/resource/resource_attachment.go @@ -216,6 +216,14 @@ func (ra *ResourceAttachment) SetError(err error) { ra.err = err } +// GetError returns the error field for a resource attachment. +func (ra *ResourceAttachment) GetError() error { + ra.guard.RLock() + defer ra.guard.RUnlock() + + return ra.err +} + // EBSToString returns a string representation of an EBS volume resource attachment. func (ra *ResourceAttachment) EBSToString() string { ra.guard.RLock() diff --git a/ecs-agent/api/resource/ebs_discovery.go b/ecs-agent/api/resource/ebs_discovery.go index 54185d09769..687a419436d 100644 --- a/ecs-agent/api/resource/ebs_discovery.go +++ b/ecs-agent/api/resource/ebs_discovery.go @@ -15,16 +15,14 @@ package resource import ( "context" + "errors" + "fmt" "strings" "time" - - "github.com/aws/amazon-ecs-agent/ecs-agent/logger" - - "github.com/pkg/errors" ) const ( - ebsVolumeDiscoveryTimeout = 5 * time.Second + ebsVolumeDiscoveryTimeout = 300 * time.Second ebsResourceKeyPrefix = "ebs-volume:" ScanPeriod = 500 * time.Millisecond ) @@ -43,27 +41,19 @@ func NewDiscoveryClient(ctx context.Context) *EBSDiscoveryClient { } } -// ScanEBSVolumes will iterate through the entire list of pending EBS volume attachments within the agent state and checks if it's attached on the host. -func ScanEBSVolumes[T GenericEBSAttachmentObject](t map[string]T, dc EBSDiscovery) []string { +// ScanEBSVolumes will iterate through the entire list of provided EBS volume attachments within the agent state and checks if it's attached on the host. +func ScanEBSVolumes[T GenericEBSAttachmentObject](pendingAttachments map[string]T, dc EBSDiscovery) []string { var err error var foundVolumes []string - for key, ebs := range t { + for key, ebs := range pendingAttachments { volumeId := strings.TrimPrefix(key, ebsResourceKeyPrefix) deviceName := ebs.GetAttachmentProperties(DeviceName) err = dc.ConfirmEBSVolumeIsAttached(deviceName, volumeId) if err != nil { - if err == ErrInvalidVolumeID || errors.Cause(err) == ErrInvalidVolumeID { - logger.Warn("Found a different EBS volume attached to the host. Expected EBS volume:", logger.Fields{ - "volumeId": volumeId, - "deviceName": deviceName, - }) - } else { - logger.Warn("Failed to confirm if EBS volume is attached to the host. ", logger.Fields{ - "volumeId": volumeId, - "deviceName": deviceName, - "error": err, - }) + if !errors.Is(err, ErrInvalidVolumeID) { + err = fmt.Errorf("%w; failed to confirm if EBS volume is attached to the host", err) } + ebs.SetError(err) continue } foundVolumes = append(foundVolumes, key) diff --git a/ecs-agent/api/resource/ebs_discovery_linux.go b/ecs-agent/api/resource/ebs_discovery_linux.go index 8c3c63cdb30..8ca54a23393 100644 --- a/ecs-agent/api/resource/ebs_discovery_linux.go +++ b/ecs-agent/api/resource/ebs_discovery_linux.go @@ -26,16 +26,12 @@ import ( // LsblkOutput is used to manage and track the output of `lsblk` type LsblkOutput struct { - BlockDevies []BD `json:"blockdevices"` + BlockDevices []BlockDevice `json:"blockdevices"` } -type BD struct { - Name string `json:"name"` - Serial string `json:"serial"` - Children []BDChild `json:"children"` -} -type BDChild struct { - Name string `json:"name"` - Serial string `json:"serial"` +type BlockDevice struct { + Name string `json:"name"` + Serial string `json:"serial"` + Children []*BlockDevice `json:"children,omitempty"` } // ConfirmEBSVolumeIsAttached is used to scan for an EBS volume that's on the host with a specific device name and/or volume ID. @@ -76,14 +72,21 @@ func (api *EBSDiscoveryClient) ConfirmEBSVolumeIsAttached(deviceName, volumeID s // parseLsblkOutput will parse the `lsblk` output and search for a EBS volume with a specific device name. // Once found we return the volume ID, otherwise we return an empty string along with an error -// The output of the "lsblk -o +SERIAL" command looks like the following: -// NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT SERIAL -// nvme0n1 259:0 0 30G 0 disk vol123 -// ├─nvme0n1p1 259:1 0 30G 0 part / -// └─nvme0n1p128 259:2 0 1M 0 part +// The output of the "lsblk -o NAME,SERIAL -J" command looks like the following: +// +// { +// "blockdevices": [ +// {"name": "nvme0n1", "serial": "vol087768edff8511a23", +// "children": [ +// {"name": "nvme0n1p1", "serial": null}, +// {"name": "nvme0n1p128", "serial": null} +// ] +// } +// ] +// } func parseLsblkOutput(output *LsblkOutput, deviceName string) (string, error) { actualDeviceName := deviceName[strings.LastIndex(deviceName, "/")+1:] - for _, block := range output.BlockDevies { + for _, block := range output.BlockDevices { if block.Name == actualDeviceName { return block.Serial, nil } diff --git a/ecs-agent/api/resource/ebs_discovery_linux_test.go b/ecs-agent/api/resource/ebs_discovery_linux_test.go index 9c53b81fe0a..09e51ab6d46 100644 --- a/ecs-agent/api/resource/ebs_discovery_linux_test.go +++ b/ecs-agent/api/resource/ebs_discovery_linux_test.go @@ -29,14 +29,14 @@ const ( ) func TestParseLsblkOutput(t *testing.T) { - blockDevice := BD{ + blockDevice := BlockDevice{ Name: testDeviceName, Serial: testVolumeID, - Children: make([]BDChild, 0), + Children: make([]*BlockDevice, 0), } lsblkOutput := &LsblkOutput{ - BlockDevies: []BD{ + BlockDevices: []BlockDevice{ blockDevice, }, } @@ -47,14 +47,14 @@ func TestParseLsblkOutput(t *testing.T) { } func TestParseLsblkOutputError(t *testing.T) { - blockDevice := BD{ + blockDevice := BlockDevice{ Name: "nvme1n1", Serial: testVolumeID, - Children: make([]BDChild, 0), + Children: make([]*BlockDevice, 0), } lsblkOutput := &LsblkOutput{ - BlockDevies: []BD{ + BlockDevices: []BlockDevice{ blockDevice, }, } diff --git a/ecs-agent/api/resource/ebs_discovery_windows_test.go b/ecs-agent/api/resource/ebs_discovery_windows_test.go index 27fb2036aed..61c1d762b0b 100644 --- a/ecs-agent/api/resource/ebs_discovery_windows_test.go +++ b/ecs-agent/api/resource/ebs_discovery_windows_test.go @@ -31,49 +31,80 @@ const ( ) func TestParseExecutableOutputWithHappyPath(t *testing.T) { - output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nVolume ID: %s\r\nDevice Name: %s\r\n\r\n", testVolumeID, deviceName) + output := fmt.Sprintf("Disk Number: 0\r\n"+ + "Volume ID: vol-abcdef1234567890a\r\n"+ + "Device Name: sda1\r\n\r\n"+ + "Disk Number: 1\r\n"+ + "Volume ID: %s\r\n"+ + "Device Name: %s\r\n\r\n", testVolumeID, deviceName) parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, deviceName) require.NoError(t, err) assert.True(t, strings.Contains(parsedOutput, testVolumeID)) } func TestParseExecutableOutputWithMissingDiskNumber(t *testing.T) { - output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nVolume ID: %s\r\nDevice Name: %s\r\n\r\n", testVolumeID, deviceName) + output := fmt.Sprintf("Disk Number: 0\r\n"+ + "Volume ID: vol-abcdef1234567890a\r\n"+ + "Device Name: sda1\r\n\r\n"+ + "Volume ID: %s\r\n"+ + "Device Name: %s\r\n\r\n", testVolumeID, deviceName) parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, deviceName) require.Error(t, err) assert.Equal(t, "", parsedOutput) } func TestParseExecutableOutputWithMissingVolumeInformation(t *testing.T) { - output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nDevice Name: %s\r\n\r\n", deviceName) + output := fmt.Sprintf("Disk Number: 0\r\n"+ + "Volume ID: vol-abcdef1234567890a\r\n"+ + "Device Name: sda1\r\n\r\n"+ + "Disk Number: 1\r\n"+ + "Device Name: %s\r\n\r\n", deviceName) parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, deviceName) require.Error(t, err) assert.Equal(t, "", parsedOutput) } func TestParseExecutableOutputWithMissingDeviceName(t *testing.T) { - output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nVolume ID: %s\r\n\r\n", testVolumeID) + output := fmt.Sprintf("Disk Number: 0\r\n"+ + "Volume ID: vol-abcdef1234567890a\r\n"+ + "Device Name: sda1\r\n\r\n"+ + "Disk Number: 1\r\n"+ + "Volume ID: %s\r\n\r\n", testVolumeID) parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, deviceName) require.Error(t, err) assert.Equal(t, "", parsedOutput) } func TestParseExecutableOutputWithVolumeNameMismatch(t *testing.T) { - output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nVolume ID: %s\r\nDevice Name: %s\r\n\r\n", testVolumeID, deviceName) + output := fmt.Sprintf("Disk Number: 0\r\n"+ + "Volume ID: vol-abcdef1234567890a\r\n"+ + "Device Name: sda1\r\n\r\n"+ + "Disk Number: 1\r\n"+ + "Volume ID: %s\r\n"+ + "Device Name: %s\r\n\r\n", testVolumeID, deviceName) parsedOutput, err := parseExecutableOutput([]byte(output), "MismatchedVolumeName", deviceName) require.Error(t, err) assert.Equal(t, "", parsedOutput) } func TestParseExecutableOutputWithDeviceNameMismatch(t *testing.T) { - output := fmt.Sprintf("Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nVolume ID: %s\r\nDevice Name: %s\r\n\r\n", testVolumeID, deviceName) + output := fmt.Sprintf("Disk Number: 0\r\n"+ + "Volume ID: vol-abcdef1234567890a\r\n"+ + "Device Name: sda1\r\n\r\n"+ + "Disk Number: 1\r\n"+ + "Volume ID: %s\r\n"+ + "Device Name: %s\r\n\r\n", testVolumeID, deviceName) parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, "MismatchedDeviceName") require.Error(t, err) assert.Equal(t, "", parsedOutput) } func TestParseExecutableOutputWithTruncatedOutputBuffer(t *testing.T) { - output := "Disk Number: 0\r\nVolume ID: vol-abcdef1234567890a\r\nDevice Name: sda1\r\n\r\nDisk Number: 1\r\nVolume ID: TruncatedBuffer..." + output := "Disk Number: 0\r\n" + + "Volume ID: vol-abcdef1234567890a\r\n" + + "Device Name: sda1\r\n\r\n" + + "Disk Number: 1\r\n" + + "Volume ID: TruncatedBuffer..." parsedOutput, err := parseExecutableOutput([]byte(output), testVolumeID, deviceName) require.Error(t, err) assert.Equal(t, "", parsedOutput) diff --git a/ecs-agent/api/resource/resource_attachment.go b/ecs-agent/api/resource/resource_attachment.go index 78ae3591a0c..da4a5e12d0b 100644 --- a/ecs-agent/api/resource/resource_attachment.go +++ b/ecs-agent/api/resource/resource_attachment.go @@ -216,6 +216,14 @@ func (ra *ResourceAttachment) SetError(err error) { ra.err = err } +// GetError returns the error field for a resource attachment. +func (ra *ResourceAttachment) GetError() error { + ra.guard.RLock() + defer ra.guard.RUnlock() + + return ra.err +} + // EBSToString returns a string representation of an EBS volume resource attachment. func (ra *ResourceAttachment) EBSToString() string { ra.guard.RLock()