Fix shared EBS discovery implementation

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.GetAllPendingEBSAttachmentWithKey()
+	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)
+}

ecs-agent/api/resource/ebs_discovery.go

@@ -15,12 +15,10 @@ package resource
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"strings"
 	"time"
-
-	"github.com/aws/amazon-ecs-agent/ecs-agent/logger"
-
-	"github.com/pkg/errors"
 )
 
 const (
@@ -44,26 +42,18 @@ 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 {
+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)