Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return success if instance or volume are not found #375

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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