diff --git a/internal/nvmeof/controller/controllerserver.go b/internal/nvmeof/controller/controllerserver.go index 6edf00a5f11..d3742ac392e 100644 --- a/internal/nvmeof/controller/controllerserver.go +++ b/internal/nvmeof/controller/controllerserver.go @@ -26,6 +26,7 @@ import ( "strconv" "github.com/container-storage-interface/spec/lib/go/csi" + "github.com/ghodss/yaml" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -167,7 +168,7 @@ func (cs *Server) CreateVolume( } }() - nvmeofData, err = cs.createNVMeoFResources(ctx, req, rbdPoolName, rbdRadosNameSpace, rbdImageName) + nvmeofData, err = cs.createNVMeoFResources(ctx, req, rbdPoolName, rbdRadosNameSpace, rbdImageName, volumeID) if err != nil { log.ErrorLog(ctx, "NVMe-oF resource setup failed for volumeID %s: %v", volumeID, err) @@ -353,8 +354,21 @@ func (cs *Server) ControllerModifyVolume( return nil, status.Errorf(codes.InvalidArgument, "failed to parse QoS parameters: %v", err) } + hostsList, err := parseHostsParameters(params) + if err != nil { + log.ErrorLog(ctx, "failed to parse NVMe-oF hosts parameters: %v", err) + + return nil, status.Errorf(codes.InvalidArgument, "failed to parse hosts parameters: %v", err) + } if nvmeofQoS != nil { - return cs.modifyNVMeoFQoS(ctx, req, nvmeofQoS) + if err := cs.modifyNVMeoFQoS(ctx, req, nvmeofQoS); err != nil { + return nil, err + } + } + if hostsList != nil { + if err := cs.modifyNVMeoFHosts(ctx, req, hostsList); err != nil { + return nil, err + } } return &csi.ControllerModifyVolumeResponse{}, nil @@ -423,14 +437,10 @@ func validateDHCHAPParameter(dhchapMode string) error { func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error { // Validate required parameters params := req.GetParameters() - requiredParams := []string{ - "subsystemNQN", "nvmeofGatewayAddress", - } - for _, param := range requiredParams { - if params[param] == "" { - return fmt.Errorf("missing required parameter: %s", param) - } + if params["nvmeofGatewayAddress"] == "" { + return errors.New("missing required parameter nvmeofGatewayAddress") } + // Validate listeners JSON if provided countOfListeners, err := validateListenersParameter(params["listeners"]) if err != nil { @@ -465,6 +475,11 @@ func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error { if err != nil { return fmt.Errorf("invalid NVMe-oF QoS parameters: %w", err) } + + _, err = parseHostsParameters(mutableParams) + if err != nil { + return fmt.Errorf("invalid NVMe-oF hosts parameters (for external clients): %w", err) + } err = validateDHCHAPParameter(params["dhchapMode"]) if err != nil { return err @@ -531,72 +546,52 @@ func validateNetworkMask(networkMask string) error { return nil } -// parseQoSParameters extracts and parses QoS parameters from the given map. -func parseQoSParameters(params map[string]string) (*nvmeof.NVMeoFQosVolume, error) { - qos := &nvmeof.NVMeoFQosVolume{} - hasAnyQoS := false - - parseParam := func(key, name string, dest **uint64) error { - if val, exists := params[key]; exists && val != "" { - parsed, err := strconv.ParseUint(val, 10, 64) - if err != nil { - return fmt.Errorf("invalid %s: %w", name, err) - } - *dest = &parsed - hasAnyQoS = true - } - - return nil - } - - if err := parseParam(nvmeof.RwIosPerSecond, nvmeof.RwIosPerSecond, &qos.RwIosPerSecond); err != nil { - return nil, err +// parseHostsParameters parses the hosts yaml list parameter and validates its contents. +// It returns a slice of hostNQNs or an error if the YAML is invalid. +// Returns nil if the key is absent (caller should not modify hosts). +// Returns empty slice if the key is present but empty (caller should remove all hosts). +func parseHostsParameters(params map[string]string) ([]string, error) { + allowHostNQNs, exists := params[AllowHostNQNs] + if !exists { + return nil, nil // Key absent: don't modify existing hosts } - if err := parseParam(nvmeof.RwMbytesPerSecond, nvmeof.RwMbytesPerSecond, &qos.RwMbytesPerSecond); err != nil { - return nil, err + if allowHostNQNs == "" { + return []string{}, nil // Key present but empty: remove all hosts } - if err := parseParam(nvmeof.RMbytesPerSecond, nvmeof.RMbytesPerSecond, &qos.RMbytesPerSecond); err != nil { - return nil, err - } - if err := parseParam(nvmeof.WMbytesPerSecond, nvmeof.WMbytesPerSecond, &qos.WMbytesPerSecond); err != nil { - return nil, err - } - - if !hasAnyQoS { - return nil, nil + var allowHostsList []string + if err := yaml.Unmarshal([]byte(allowHostNQNs), &allowHostsList); err != nil { + return nil, fmt.Errorf("invalid %s: must be a YAML list of strings: %w", AllowHostNQNs, err) } - return qos, nil + return allowHostsList, nil } -// modifyNVMeoFQoS handles NVMe-oF gateway QoS modification. -func (cs *Server) modifyNVMeoFQoS( +// withGatewayConnection is a helper that manages the common pattern of: +// 1. Getting secrets (with fallback to k8s secret) +// 2. Getting NVMe-oF metadata +// 3. Connecting to gateway with proper cleanup +// 4. Executing the provided operation function. +func (cs *Server) withGatewayConnection( ctx context.Context, req *csi.ControllerModifyVolumeRequest, - qos *nvmeof.NVMeoFQosVolume, -) (*csi.ControllerModifyVolumeResponse, error) { - volumeID := req.GetVolumeId() - + volumeID string, + fn func(context.Context, *nvmeof.GatewayRpcClient, *nvmeof.NVMeoFVolumeData) error, +) error { // Step 1: Get secrets - - // Since ControllerModifyVolume doesn't receive volume context and dont have option to take secrets - // because there is no "csi.storage.k8s.io/controller-modify-secret-name" field in the SC !, - // the full solution for it is to use GetControllerExpandSecretRef but there is no such function yet. - // TODO: change the call to GetControllerExpandSecretRef once it is implemented. secrets := req.GetSecrets() if secrets == nil { secretName, secretNamespace, err := util.GetControllerPublishSecretRef(volumeID, util.RBDType) if err != nil { log.ErrorLog(ctx, "Failed to get secret reference: %v", err) - return nil, status.Errorf(codes.Internal, "failed to get secret reference: %v", err) + return status.Errorf(codes.Internal, "failed to get secret reference: %v", err) } secrets, err = k8s.GetSecret(secretName, secretNamespace) if err != nil { log.ErrorLog(ctx, "Failed to get secret from k8s: %v", err) - return nil, status.Errorf(codes.Internal, "failed to get secret: %v", err) + return status.Errorf(codes.Internal, "failed to get secret: %v", err) } } @@ -605,7 +600,7 @@ func (cs *Server) modifyNVMeoFQoS( if err != nil { log.ErrorLog(ctx, "Failed to get NVMe-oF metadata: %v", err) - return nil, nvmeoferrors.ToGRPCError(err) + return nvmeoferrors.ToGRPCError(err) } // Step 3: Connect to gateway @@ -617,7 +612,7 @@ func (cs *Server) modifyNVMeoFQoS( if err != nil { log.ErrorLog(ctx, "Gateway connection failed: %v", err) - return nil, status.Errorf(codes.Unavailable, "gateway connection failed: %v", err) + return status.Errorf(codes.Unavailable, "gateway connection failed: %v", err) } defer func() { if closeErr := gateway.Destroy(); closeErr != nil { @@ -625,27 +620,107 @@ func (cs *Server) modifyNVMeoFQoS( } }() - // Step 4: Apply NVMe-oF QoS via gateway - log.DebugLog(ctx, "Setting QoS for subsystem=%s, nsid=%d", nvmeofData.SubsystemNQN, nvmeofData.NamespaceID) + // Step 4: Execute the operation + return fn(ctx, gateway, nvmeofData) +} - err = gateway.SetQoSLimitsForNamespace(ctx, nvmeofData.SubsystemNQN, nvmeofData.NamespaceID, *qos) - if err != nil { - // Check if error is EEXIST (RBD QoS already set) - if errors.Is(err, nvmeoferrors.ErrRbdQoSExists) { - log.ErrorLog(ctx, "RBD QoS already configured on volume") +// modifyNVMeoFHosts handles adding or removing hosts from the subsystem based on the provided list of host NQNs. +func (cs *Server) modifyNVMeoFHosts(ctx context.Context, req *csi.ControllerModifyVolumeRequest, hosts []string) error { + volumeID := req.GetVolumeId() + + return cs.withGatewayConnection(ctx, req, volumeID, func( + ctx context.Context, + gateway *nvmeof.GatewayRpcClient, + nvmeofData *nvmeof.NVMeoFVolumeData, + ) error { + log.DebugLog(ctx, "Modifying hosts for subsystem=%s, nsid=%d: desired hosts=%v", + nvmeofData.SubsystemNQN, nvmeofData.NamespaceID, hosts) - return nil, status.Error(codes.InvalidArgument, - "RBD QoS already configured on this volume, cannot set NVMe-oF gateway QoS") + err := gateway.UpdateHostsForSubsystem(ctx, nvmeofData.SubsystemNQN, hosts) + if err != nil { + log.ErrorLog(ctx, "Failed to update hosts for subsystem: %v", err) + + return status.Errorf(codes.Internal, "failed to update hosts for subsystem: %v", err) } - log.ErrorLog(ctx, "Failed to set QoS limits: %v", err) + log.DebugLog(ctx, "Successfully modified hosts for volume %s", volumeID) - return nil, status.Errorf(codes.Internal, "failed to set QoS limits: %v", err) + return nil + }) +} + +// parseQoSParameters extracts and parses QoS parameters from the given map. +func parseQoSParameters(params map[string]string) (*nvmeof.NVMeoFQosVolume, error) { + qos := &nvmeof.NVMeoFQosVolume{} + hasAnyQoS := false + + parseParam := func(key, name string, dest **uint64) error { + if val, exists := params[key]; exists && val != "" { + parsed, err := strconv.ParseUint(val, 10, 64) + if err != nil { + return fmt.Errorf("invalid %s: %w", name, err) + } + *dest = &parsed + hasAnyQoS = true + } + + return nil } - log.DebugLog(ctx, "Successfully modified NVMe-oF QoS for volume %s", volumeID) + if err := parseParam(nvmeof.RwIosPerSecond, nvmeof.RwIosPerSecond, &qos.RwIosPerSecond); err != nil { + return nil, err + } + if err := parseParam(nvmeof.RwMbytesPerSecond, nvmeof.RwMbytesPerSecond, &qos.RwMbytesPerSecond); err != nil { + return nil, err + } + if err := parseParam(nvmeof.RMbytesPerSecond, nvmeof.RMbytesPerSecond, &qos.RMbytesPerSecond); err != nil { + return nil, err + } + if err := parseParam(nvmeof.WMbytesPerSecond, nvmeof.WMbytesPerSecond, &qos.WMbytesPerSecond); err != nil { + return nil, err + } - return &csi.ControllerModifyVolumeResponse{}, nil + if !hasAnyQoS { + return nil, nil + } + + return qos, nil +} + +// modifyNVMeoFQoS handles NVMe-oF gateway QoS modification. +func (cs *Server) modifyNVMeoFQoS( + ctx context.Context, + req *csi.ControllerModifyVolumeRequest, + qos *nvmeof.NVMeoFQosVolume, +) error { + volumeID := req.GetVolumeId() + + return cs.withGatewayConnection(ctx, req, volumeID, func( + ctx context.Context, + gateway *nvmeof.GatewayRpcClient, + nvmeofData *nvmeof.NVMeoFVolumeData, + ) error { + log.DebugLog(ctx, "Setting QoS for subsystem=%s, nsid=%d", nvmeofData.SubsystemNQN, nvmeofData.NamespaceID) + + err := gateway.SetQoSLimitsForNamespace(ctx, nvmeofData.SubsystemNQN, nvmeofData.NamespaceID, *qos) + if err != nil { + // Check if error is EEXIST (RBD QoS already set) + if errors.Is(err, nvmeoferrors.ErrRbdQoSExists) { + log.ErrorLog(ctx, "RBD QoS already configured on volume") + + return status.Error(codes.InvalidArgument, + "RBD QoS already configured on this volume, cannot set NVMe-oF gateway QoS") + } + + log.ErrorLog(ctx, "Failed to set QoS limits: %v", err) + + return status.Errorf(codes.Internal, "failed to set QoS limits: %v", err) + } + + log.DebugLog(ctx, "Successfully modified NVMe-oF QoS for volume %s", volumeID) + + return nil + }) } // ensureSubsystem checks if the subsystem exists, and creates it if not. @@ -743,7 +818,8 @@ func (cs *Server) createNVMeoFResources( req *csi.CreateVolumeRequest, rbdPoolName, rbdRadosNameSpace, - rbdImageName string, + rbdImageName, + volumeID string, ) (*nvmeof.NVMeoFVolumeData, error) { // Step 1: Extract parameters (already validated) params := req.GetParameters() @@ -751,7 +827,7 @@ func (cs *Server) createNVMeoFResources( networkMask := params["networkMask"] nvmeofData := &nvmeof.NVMeoFVolumeData{} - if err := nvmeofData.SetFromParameters(params); err != nil { + if err := nvmeofData.SetFromParameters(params, volumeID); err != nil { return nil, fmt.Errorf("failed to set NVMe-oF volume data: %w", err) } @@ -765,7 +841,15 @@ func (cs *Server) createNVMeoFResources( return nil, fmt.Errorf("failed to parse QoS parameters: %w", err) } + // If VAC with hosts list is given (for external client) + // We need to parse the hosts list and pass it to the gateway for creating host entries + // and adding them to the subsystem. + hosts, err := parseHostsParameters(mutableParams) + if err != nil { + log.ErrorLog(ctx, "failed to parse NVMe-oF hosts parameters: %v", err) + return nil, fmt.Errorf("failed to parse hosts parameters: %w", err) + } // Step 2: Connect to gateway config, err := getGatewayConfigFromRequest(params) if err != nil { @@ -813,7 +897,16 @@ func (cs *Server) createNVMeoFResources( return nvmeofData, fmt.Errorf("setting QoS limits failed: %w", err) } } - + if hosts != nil { + log.DebugLog(ctx, "Adding hosts to subsystem: %v", hosts) + for _, host := range hosts { + // TODO - for now we create host with empty DH-CHAP keys, + // in the future we can extend the VAC parameters to allow passing DH-CHAP keys for each host if needed?? + if err := gateway.AddHost(ctx, nvmeofData.SubsystemNQN, host, nvmeof.DHCHAPKeys{}); err != nil { + return nvmeofData, fmt.Errorf("adding host %s to subsystem failed: %w", host, err) + } + } + } // Step 6: If using auto-listeners, query them back for storing in metadata if networkMask != "" { autoListeners, err := gateway.ListListeners(ctx, nvmeofData.SubsystemNQN) @@ -1013,6 +1106,15 @@ func getHostNQNFromNodeID(nodeID string) (string, error) { return prefix + nodeID, nil } +// AllowHostNQNs is the VolumeAttributesClass mutable parameter key for specifying +// a YAML list of host NQNs to allow access to a volume. Use "*" to allow any host. +// Example: +// +// allowHostNQNs: | +// - nqn.2014-08.org.nvmexpress:host1 +// - nqn.2014-08.org.nvmexpress:host2 +const AllowHostNQNs = "allowHostNQNs" + // VolumeContext metadata keys. const ( // NVMe-oF resource info. diff --git a/internal/nvmeof/controller/qos_test.go b/internal/nvmeof/controller/mutable_params_test.go similarity index 58% rename from internal/nvmeof/controller/qos_test.go rename to internal/nvmeof/controller/mutable_params_test.go index 35c8b615f8d..3e3a2938c81 100644 --- a/internal/nvmeof/controller/qos_test.go +++ b/internal/nvmeof/controller/mutable_params_test.go @@ -151,3 +151,109 @@ func TestParseQoSParameters(t *testing.T) { }) } } + +func TestParseHostParameters(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + params map[string]string + expected []string + expectError bool + }{ + { + name: "no allowHostNQNs parameter", + params: map[string]string{}, + expected: nil, + }, + { + name: "empty allowHostNQNs (remove all hosts)", + params: map[string]string{ + AllowHostNQNs: "", + }, + expected: []string{}, // Empty slice, not nil - signals to remove all hosts + }, + { + name: "single host NQN", + params: map[string]string{ + AllowHostNQNs: `- nqn.2014-08.org.nvmexpress:host1`, + }, + expected: []string{"nqn.2014-08.org.nvmexpress:host1"}, + }, + { + name: "multiple host NQNs", + params: map[string]string{ + AllowHostNQNs: `- nqn.2014-08.org.nvmexpress:host1 +- nqn.2014-08.org.nvmexpress:host2 +- nqn.2014-08.org.nvmexpress:host3`, + }, + expected: []string{ + "nqn.2014-08.org.nvmexpress:host1", + "nqn.2014-08.org.nvmexpress:host2", + "nqn.2014-08.org.nvmexpress:host3", + }, + }, + { + name: "wildcard to allow any host", + params: map[string]string{ + AllowHostNQNs: `- "*"`, + }, + expected: []string{"*"}, + }, + { + name: "YAML list in flow style", + params: map[string]string{ + AllowHostNQNs: `["nqn.2014-08.org.nvmexpress:host1", "nqn.2014-08.org.nvmexpress:host2"]`, + }, + expected: []string{ + "nqn.2014-08.org.nvmexpress:host1", + "nqn.2014-08.org.nvmexpress:host2", + }, + }, + { + name: "invalid YAML - not a list (plain string)", + params: map[string]string{ + AllowHostNQNs: `nqn.2014-08.org.nvmexpress:host1`, + }, + expectError: true, + }, + { + name: "invalid YAML - map instead of list", + params: map[string]string{ + AllowHostNQNs: `host1: nqn.2014-08.org.nvmexpress:host1`, + }, + expectError: true, + }, + { + name: "invalid YAML - unclosed quote", + params: map[string]string{ + AllowHostNQNs: `- "nqn.2014-08.org.nvmexpress:host1`, + }, + expectError: true, + }, + { + name: "other parameters present but no allowHostNQNs", + params: map[string]string{ + "someOtherParam": "value", + "nvmeofRWIOsPerSecond": "10000", + }, + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + result, err := parseHostsParameters(tt.params) + + if tt.expectError { + require.Error(t, err) + assert.Nil(t, result) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} diff --git a/internal/nvmeof/nvmeof.go b/internal/nvmeof/nvmeof.go index 3cdec173a40..5c78b51fa36 100644 --- a/internal/nvmeof/nvmeof.go +++ b/internal/nvmeof/nvmeof.go @@ -21,6 +21,7 @@ import ( "crypto/rand" "fmt" "math/big" + "slices" "syscall" pb "github.com/ceph/ceph-nvmeof/lib/go/nvmeof" @@ -605,6 +606,74 @@ func (gw *GatewayRpcClient) RemoveHost(ctx context.Context, subsystemNQN, hostNQ } } +// ListHosts lists all hosts in a subsystem. +// Returns a slice of host NQNs that have access to the subsystem. +func (gw *GatewayRpcClient) ListHosts(ctx context.Context, subsystemNQN string) ([]string, error) { + log.DebugLog(ctx, "Listing hosts in subsystem %s", subsystemNQN) + + req := &pb.ListHostsReq{ + Subsystem: subsystemNQN, + } + + resp, err := gw.client.ListHosts(ctx, req) + if err != nil { + return nil, fmt.Errorf("failed to list hosts in subsystem %s: %w", subsystemNQN, err) + } + if resp.GetStatus() != 0 { + return nil, fmt.Errorf("gateway ListHosts returned error (status=%d): %s", + resp.GetStatus(), resp.GetErrorMessage()) + } + + // Extract host NQNs from response + hosts := make([]string, 0, len(resp.GetHosts())) + for _, host := range resp.GetHosts() { + hosts = append(hosts, host.GetNqn()) + } + + log.DebugLog(ctx, "Listed %d hosts in subsystem %s", len(hosts), subsystemNQN) + + return hosts, nil +} + +// UpdateHostsForSubsystem reconciles the hosts in a subsystem to match the desired list. +// It lists current hosts, then adds/removes hosts to ensure the subsystem has exactly +// the hosts specified in desiredHosts. +func (gw *GatewayRpcClient) UpdateHostsForSubsystem( + ctx context.Context, + subsystemNQN string, + desiredHosts []string, +) error { + currentHosts, err := gw.ListHosts(ctx, subsystemNQN) + if err != nil { + return fmt.Errorf("failed to list current hosts: %w", err) + } + + log.DebugLog(ctx, "Host reconciliation for subsystem %s: current=%v, desired=%v", + subsystemNQN, currentHosts, desiredHosts) + + for _, host := range currentHosts { + if !slices.Contains(desiredHosts, host) { + log.DebugLog(ctx, "Removing host %s from subsystem %s", host, subsystemNQN) + if err := gw.RemoveHost(ctx, subsystemNQN, host); err != nil { + return fmt.Errorf("failed to remove host %s: %w", host, err) + } + } + } + + for _, host := range desiredHosts { + if !slices.Contains(currentHosts, host) { + log.DebugLog(ctx, "Adding host %s to subsystem %s", host, subsystemNQN) + // Note: AddHost requires DHCHAPKeys, but for now we pass empty keys + // TODO: Support DH-CHAP keys if needed for host updates + if err := gw.AddHost(ctx, subsystemNQN, host, DHCHAPKeys{}); err != nil { + return fmt.Errorf("failed to add host %s: %w", host, err) + } + } + } + + return nil +} + // List namespaces in a subsystem. func (gw *GatewayRpcClient) ListNamespaces(ctx context.Context, subsystemNQN string) (*pb.NamespacesInfo, error) { log.DebugLog(ctx, "Listing namespaces in subsystem %s", subsystemNQN) diff --git a/internal/nvmeof/volume.go b/internal/nvmeof/volume.go index 6d4da0cb910..735c0358175 100644 --- a/internal/nvmeof/volume.go +++ b/internal/nvmeof/volume.go @@ -84,10 +84,12 @@ func SetupListeners(listenersJSON string) ([]ListenerDetails, error) { // It extracts the subsystem NQN, gateway management info, security config, and // listener info from the parameters. // It also applies default values to listeners if necessary. -func (v *NVMeoFVolumeData) SetFromParameters(parameters map[string]string) error { +func (v *NVMeoFVolumeData) SetFromParameters(parameters map[string]string, volumeID string) error { // set subsystem NQN v.SubsystemNQN = parameters["subsystemNQN"] - + if v.SubsystemNQN == "" { + v.SubsystemNQN = "nqn.2016-06.io.ceph:subsystem." + volumeID + } // set gw management info if nvmeofGatewayPortStr := parameters["nvmeofGatewayPort"]; nvmeofGatewayPortStr != "" { parsed, err := strconv.ParseUint(nvmeofGatewayPortStr, 10, 32) diff --git a/internal/nvmeof/volume_test.go b/internal/nvmeof/volume_test.go index 7123f50cc2a..0d98fc8612d 100644 --- a/internal/nvmeof/volume_test.go +++ b/internal/nvmeof/volume_test.go @@ -206,10 +206,30 @@ func TestSetFromParameters(t *testing.T) { expected: NVMeoFVolumeData{}, expectError: true, }, + { + name: "missing subsystem NQN (should use default)", + params: map[string]string{ + "nvmeofGatewayAddress": "10.241.1.9", + "nvmeofGatewayPort": "5500", + "listeners": `[{"hostname": "nvmeof-gw-a", "port": 4420, "address": "10.92.3.12"}]`, + }, + expected: NVMeoFVolumeData{ + SubsystemNQN: "nqn.2016-06.io.ceph:subsystem.test-volume-id", + GatewayManagementInfo: GatewayConfig{ + Address: "10.241.1.9", + Port: 5500, + }, + ListenerInfo: []ListenerDetails{ + {Hostname: "nvmeof-gw-a", GatewayAddress: GatewayAddress{Port: 4420, Address: "10.92.3.12"}}, + }, + }, + expectError: false, + }, } for _, test := range tests { vol := &NVMeoFVolumeData{} - err := vol.SetFromParameters(test.params) + volID := "test-volume-id" + err := vol.SetFromParameters(test.params, volID) if test.expectError { require.Error(t, err) } else {