Let lookupVolume return NotFound when an invalid volume id is given

This commit is contained in:
Helen Zhang
2024-01-06 16:03:05 -08:00
parent b7e0b32f24
commit fa7f020c50

View File

@@ -6,9 +6,9 @@
package service package service
import ( import (
"github.com/container-storage-interface/spec/lib/go/csi"
"github.com/oracle/zfssa-csi-driver/pkg/utils" "github.com/oracle/zfssa-csi-driver/pkg/utils"
"github.com/oracle/zfssa-csi-driver/pkg/zfssarest" "github.com/oracle/zfssa-csi-driver/pkg/zfssarest"
"github.com/container-storage-interface/spec/lib/go/csi"
"golang.org/x/net/context" "golang.org/x/net/context"
"google.golang.org/grpc/codes" "google.golang.org/grpc/codes"
"google.golang.org/grpc/status" "google.golang.org/grpc/status"
@@ -109,20 +109,18 @@ type zVolumeInterface interface {
isBlock() bool isBlock() bool
} }
// This method must be called when the possibility of the volume not existing yet exists. // This method must be called when the possibility of the volume not existing yet exists.
// The following 3 scenarios are possible: // The following 3 scenarios are possible:
// //
// * The volume doesn't exists yet or it exists in the appliance but is not in the // - The volume doesn't exists yet or it exists in the appliance but is not in the
// volume cache yet. Either way, a structure representing the volume is created // volume cache yet. Either way, a structure representing the volume is created
// in the stateCreating state, stored in the cache and a reference returned to the // in the stateCreating state, stored in the cache and a reference returned to the
// caller. // caller.
// * A structure representing the volume already exists in the cache and is in the // - A structure representing the volume already exists in the cache and is in the
// stateCreated state. A reference is returned to the caller. // stateCreated state. A reference is returned to the caller.
// * A structure representing the volume already exists in the cache and is NOT in // - A structure representing the volume already exists in the cache and is NOT in
// the stateCreated state. This means the CO probably lost state and submitted multiple // the stateCreated state. This means the CO probably lost state and submitted multiple
// simultaneous requests for this volume. In this case an error is returned. // simultaneous requests for this volume. In this case an error is returned.
//
func (zd *ZFSSADriver) newVolume(ctx context.Context, pool, project, name string, func (zd *ZFSSADriver) newVolume(ctx context.Context, pool, project, name string,
block bool) (zVolumeInterface, error) { block bool) (zVolumeInterface, error) {
@@ -140,7 +138,7 @@ func (zd *ZFSSADriver) newVolume(ctx context.Context, pool, project, name string
zvol := zd.vCache.lookup(ctx, name) zvol := zd.vCache.lookup(ctx, name)
if zvol != nil { if zvol != nil {
// Volume already known. // Volume already known.
utils.GetLogCTRL(ctx, 5).Println("zd.newVolume", "request", ) utils.GetLogCTRL(ctx, 5).Println("zd.newVolume", "request")
zvol.hold(ctx) zvol.hold(ctx)
zd.vCache.Unlock(ctx) zd.vCache.Unlock(ctx)
if zvol.lock(ctx) != stateCreated { if zvol.lock(ctx) != stateCreated {
@@ -167,7 +165,8 @@ func (zd *ZFSSADriver) lookupVolume(ctx context.Context, token *zfssarest.Token,
vid, err := utils.VolumeIdFromString(volumeId) vid, err := utils.VolumeIdFromString(volumeId)
if err != nil { if err != nil {
return nil, err utils.GetLogCTRL(ctx, 2).Println("Failed to get volumeId from String", err.Error())
return nil, status.Errorf(codes.NotFound, "Volume (%s) not found", volumeId)
} }
// Check first in the list of volumes if the volume is already known. // Check first in the list of volumes if the volume is already known.
@@ -199,7 +198,7 @@ func (zd *ZFSSADriver) lookupVolume(ctx context.Context, token *zfssarest.Token,
if err != nil { if err != nil {
zd.releaseVolume(ctx, zvol) zd.releaseVolume(ctx, zvol)
if httpStatus == http.StatusNotFound { if httpStatus == http.StatusNotFound {
return nil, status.Error(codes.NotFound, "Volume (%s) not found") return nil, status.Errorf(codes.NotFound, "Volume (%s) not found", volumeId)
} }
return nil, err return nil, err
} }
@@ -208,7 +207,7 @@ func (zd *ZFSSADriver) lookupVolume(ctx context.Context, token *zfssarest.Token,
return zvol, nil return zvol, nil
default: default:
zd.releaseVolume(ctx, zvol) zd.releaseVolume(ctx, zvol)
return nil, status.Error(codes.NotFound, "Volume (%s) not found") return nil, status.Errorf(codes.NotFound, "Volume (%s) not found", volumeId)
} }
} }
@@ -234,9 +233,10 @@ func (zd *ZFSSADriver) releaseVolume(ctx context.Context, zvol zVolumeInterface)
// If a snapshot with the passed in name already exists, it is returned. If it doesn't exist, // If a snapshot with the passed in name already exists, it is returned. If it doesn't exist,
// a new snapshot structure is created and returned. This method could fail or reasons: // a new snapshot structure is created and returned. This method could fail or reasons:
// //
// 1) A snapshot with the passed in name already exists but the volume source // 1. A snapshot with the passed in name already exists but the volume source
// is not the volume source passed in. // is not the volume source passed in.
// 2) A snapshot with the passed in name already exists but is not in the stateCreated //
// 2. A snapshot with the passed in name already exists but is not in the stateCreated
// state (or stable state). As for volumes, This would mean the CO lost state and // state (or stable state). As for volumes, This would mean the CO lost state and
// issued simultaneous requests for the same snapshot. // issued simultaneous requests for the same snapshot.
// //
@@ -289,13 +289,13 @@ func (zd *ZFSSADriver) newSnapshot(ctx context.Context, token *zfssarest.Token,
// exclusive access to the returned snapshot and its volume source. This method could fail // exclusive access to the returned snapshot and its volume source. This method could fail
// for the following reasons: // for the following reasons:
// //
// 1) The source volume cannot be found locally or in the appliance. // 1. The source volume cannot be found locally or in the appliance.
// 2) The snapshot exists but is in an unstable state. This would mean the // 2. The snapshot exists but is in an unstable state. This would mean the
// CO lost state and issued multiple simultaneous requests for the same // CO lost state and issued multiple simultaneous requests for the same
// snapshot. // snapshot.
// 3) There's an inconsistency between what the appliance thinks the volume // 3. There's an inconsistency between what the appliance thinks the volume
// source is and what the existing snapshot says it is (a panic is issued). // source is and what the existing snapshot says it is (a panic is issued).
// 4) The snapshot cannot be found locally or in the appliance. // 4. The snapshot cannot be found locally or in the appliance.
func (zd *ZFSSADriver) lookupSnapshot(ctx context.Context, token *zfssarest.Token, func (zd *ZFSSADriver) lookupSnapshot(ctx context.Context, token *zfssarest.Token,
snapshotId string) (*zSnapshot, error) { snapshotId string) (*zSnapshot, error) {