Skip to content

Commit

Permalink
Return success if instance or volume are not found
Browse files Browse the repository at this point in the history
  • Loading branch information
bertinatto committed Nov 25, 2019
1 parent baf9680 commit a0280d9
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 0 deletions.
29 changes: 29 additions & 0 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -804,13 +812,34 @@ 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.
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.
Expand Down
3 changes: 3 additions & 0 deletions pkg/driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 28 additions & 0 deletions pkg/driver/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit a0280d9

Please sign in to comment.