From a0280d91bae800de5c39d8c862313a5b9e226e3d Mon Sep 17 00:00:00 2001
From: Fabio Bertinatto <fbertina@redhat.com>
Date: Mon, 16 Sep 2019 13:08:31 +0200
Subject: [PATCH] Return success if instance or volume are not found

---
 pkg/cloud/cloud.go            | 29 +++++++++++++++++++++++++++++
 pkg/driver/controller.go      |  3 +++
 pkg/driver/controller_test.go | 28 ++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+)

diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go
index fc80f2a209..66de2e1b14 100644
--- a/pkg/cloud/cloud.go
+++ b/pkg/cloud/cloud.go
@@ -401,6 +401,11 @@ func (c *cloud) DetachDisk(ctx context.Context, volumeID, nodeID string) error {
 
 	_, err = c.ec2.DetachVolumeWithContext(ctx, request)
 	if err != nil {
+		if isAWSErrorIncorrectState(err) ||
+			isAWSErrorInvalidAttachmentNotFound(err) ||
+			isAWSErrorVolumeNotFound(err) {
+			return ErrNotFound
+		}
 		return fmt.Errorf("could not detach volume %q from node %q: %v", volumeID, nodeID, err)
 	}
 
@@ -685,6 +690,9 @@ func (c *cloud) getInstance(ctx context.Context, nodeID string) (*ec2.Instance,
 	for {
 		response, err := c.ec2.DescribeInstancesWithContext(ctx, request)
 		if err != nil {
+			if isAWSErrorInstanceNotFound(err) {
+				return nil, ErrNotFound
+			}
 			return nil, fmt.Errorf("error listing AWS instances: %q", err)
 		}
 
@@ -804,6 +812,13 @@ func isAWSErrorIncorrectModification(err error) bool {
 	return isAWSError(err, "IncorrectModificationState")
 }
 
+// isAWSErrorInstanceNotFound returns a boolean indicating whether the
+// given error is an AWS InvalidInstanceID.NotFound error. This error is
+// reported when the specified instance doesn't exist.
+func isAWSErrorInstanceNotFound(err error) bool {
+	return isAWSError(err, "InvalidInstanceID.NotFound")
+}
+
 // isAWSErrorVolumeNotFound returns a boolean indicating whether the
 // given error is an AWS InvalidVolume.NotFound error. This error is
 // reported when the specified volume doesn't exist.
@@ -811,6 +826,20 @@ func isAWSErrorVolumeNotFound(err error) bool {
 	return isAWSError(err, "InvalidVolume.NotFound")
 }
 
+// isAWSErrorIncorrectState returns a boolean indicating whether the
+// given error is an AWS IncorrectState error. This error is
+// reported when the resource is not in a correct state for the request.
+func isAWSErrorIncorrectState(err error) bool {
+	return isAWSError(err, "IncorrectState")
+}
+
+// isAWSErrorInvalidAttachmentNotFound returns a boolean indicating whether the
+// given error is an AWS InvalidAttachment.NotFound error. This error is reported
+// when attempting to detach a volume from an instance to which it is not attached.
+func isAWSErrorInvalidAttachmentNotFound(err error) bool {
+	return isAWSError(err, "InvalidAttachment.NotFound")
+}
+
 // isAWSErrorSnapshotNotFound returns a boolean indicating whether the
 // given error is an AWS InvalidSnapshot.NotFound error. This error is
 // reported when the specified snapshot doesn't exist.
diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go
index 47d9f2873a..3de13be2c7 100644
--- a/pkg/driver/controller.go
+++ b/pkg/driver/controller.go
@@ -265,6 +265,9 @@ func (d *controllerService) ControllerUnpublishVolume(ctx context.Context, req *
 	}
 
 	if err := d.cloud.DetachDisk(ctx, volumeID, nodeID); err != nil {
+		if err == cloud.ErrNotFound {
+			return &csi.ControllerUnpublishVolumeResponse{}, nil
+		}
 		return nil, status.Errorf(codes.Internal, "Could not detach volume %q from node %q: %v", volumeID, nodeID, err)
 	}
 	klog.V(5).Infof("ControllerUnpublishVolume: volume %s detached from node %s", volumeID, nodeID)
diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go
index 290fdfa8b0..b9a038f984 100644
--- a/pkg/driver/controller_test.go
+++ b/pkg/driver/controller_test.go
@@ -1496,6 +1496,34 @@ func TestControllerPublishVolume(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "success when resource is not found",
+			testFunc: func(t *testing.T) {
+				req := &csi.ControllerUnpublishVolumeRequest{
+					NodeId:   expInstanceID,
+					VolumeId: "vol-test",
+				}
+				expResp := &csi.ControllerUnpublishVolumeResponse{}
+
+				ctx := context.Background()
+
+				mockCtl := gomock.NewController(t)
+				defer mockCtl.Finish()
+
+				mockCloud := mocks.NewMockCloud(mockCtl)
+				mockCloud.EXPECT().DetachDisk(gomock.Eq(ctx), req.VolumeId, req.NodeId).Return(cloud.ErrNotFound)
+
+				awsDriver := controllerService{cloud: mockCloud}
+				resp, err := awsDriver.ControllerUnpublishVolume(ctx, req)
+				if err != nil {
+					t.Fatalf("Unexpected error: %v", err)
+				}
+
+				if !reflect.DeepEqual(resp, expResp) {
+					t.Fatalf("Expected resp to be %+v, got: %+v", expResp, resp)
+				}
+			},
+		},
 		{
 			name: "fail no VolumeId",
 			testFunc: func(t *testing.T) {