-
Notifications
You must be signed in to change notification settings - Fork 151
sanity: Add tests for NodeStageVolume/NodeUnstageVolume #66
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
Changes from all commits
0e89bd0
b43de19
7349b5a
0590ad0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,15 +16,86 @@ func (s *service) NodeStageVolume( | |
req *csi.NodeStageVolumeRequest) ( | ||
*csi.NodeStageVolumeResponse, error) { | ||
|
||
return nil, status.Error(codes.Unimplemented, "") | ||
device, ok := req.PublishInfo["device"] | ||
if !ok { | ||
return nil, status.Error( | ||
codes.InvalidArgument, | ||
"stage volume info 'device' key required") | ||
} | ||
|
||
if len(req.GetVolumeId()) == 0 { | ||
return nil, status.Error(codes.InvalidArgument, "Volume ID cannot be empty") | ||
} | ||
|
||
if len(req.GetStagingTargetPath()) == 0 { | ||
return nil, status.Error(codes.InvalidArgument, "Staging Target Path cannot be empty") | ||
} | ||
|
||
if req.GetVolumeCapability() == nil { | ||
return nil, status.Error(codes.InvalidArgument, "Volume Capability cannot be empty") | ||
} | ||
|
||
s.volsRWL.Lock() | ||
defer s.volsRWL.Unlock() | ||
|
||
i, v := s.findVolNoLock("id", req.VolumeId) | ||
if i < 0 { | ||
return nil, status.Error(codes.NotFound, req.VolumeId) | ||
} | ||
|
||
// nodeStgPathKey is the key in the volume's attributes that is set to a | ||
// mock stage path if the volume has been published by the node | ||
nodeStgPathKey := path.Join(s.nodeID, req.StagingTargetPath) | ||
|
||
// Check to see if the volume has already been staged. | ||
if v.Attributes[nodeStgPathKey] != "" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assumes the key is in the map and could panic. It needs to get the key by also checking to see if it is there. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just verified that if a key doesn't exists in a map, it doesn't panic. The value would just be empty. And this is done in the same way for NodePublishVolume as well. So, it should be fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right! That must be new since I started using golang (1.2): https://play.golang.org/p/OMC7Taio5h1 |
||
// TODO: Check for the capabilities to be equal. Return "ALREADY_EXISTS" | ||
// if the capabilities don't match. | ||
return &csi.NodeStageVolumeResponse{}, nil | ||
} | ||
|
||
// Stage the volume. | ||
v.Attributes[nodeStgPathKey] = device | ||
s.vols[i] = v | ||
|
||
return &csi.NodeStageVolumeResponse{}, nil | ||
} | ||
|
||
func (s *service) NodeUnstageVolume( | ||
ctx context.Context, | ||
req *csi.NodeUnstageVolumeRequest) ( | ||
*csi.NodeUnstageVolumeResponse, error) { | ||
|
||
return nil, status.Error(codes.Unimplemented, "") | ||
if len(req.GetVolumeId()) == 0 { | ||
return nil, status.Error(codes.InvalidArgument, "Volume ID cannot be empty") | ||
} | ||
|
||
if len(req.GetStagingTargetPath()) == 0 { | ||
return nil, status.Error(codes.InvalidArgument, "Staging Target Path cannot be empty") | ||
} | ||
|
||
s.volsRWL.Lock() | ||
defer s.volsRWL.Unlock() | ||
|
||
i, v := s.findVolNoLock("id", req.VolumeId) | ||
if i < 0 { | ||
return nil, status.Error(codes.NotFound, req.VolumeId) | ||
} | ||
|
||
// nodeStgPathKey is the key in the volume's attributes that is set to a | ||
// mock stage path if the volume has been published by the node | ||
nodeStgPathKey := path.Join(s.nodeID, req.StagingTargetPath) | ||
|
||
// Check to see if the volume has already been unstaged. | ||
if v.Attributes[nodeStgPathKey] == "" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assumes that the key is there. It could panic. There needs to be a check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified that if a key doesn't exists in a map, it doesn't panic. The value would just be empty. |
||
return &csi.NodeUnstageVolumeResponse{}, nil | ||
} | ||
|
||
// Unpublish the volume. | ||
delete(v.Attributes, nodeStgPathKey) | ||
s.vols[i] = v | ||
|
||
return &csi.NodeUnstageVolumeResponse{}, nil | ||
} | ||
|
||
func (s *service) NodePublishVolume( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to modify NodePublishVolume to do the right thing when a StagingTargetPath is provided. |
||
|
@@ -39,6 +110,18 @@ func (s *service) NodePublishVolume( | |
"publish volume info 'device' key required") | ||
} | ||
|
||
if len(req.GetVolumeId()) == 0 { | ||
return nil, status.Error(codes.InvalidArgument, "Volume ID cannot be empty") | ||
} | ||
|
||
if len(req.GetTargetPath()) == 0 { | ||
return nil, status.Error(codes.InvalidArgument, "Target Path cannot be empty") | ||
} | ||
|
||
if req.GetVolumeCapability() == nil { | ||
return nil, status.Error(codes.InvalidArgument, "Volume Capability cannot be empty") | ||
} | ||
|
||
s.volsRWL.Lock() | ||
defer s.volsRWL.Unlock() | ||
|
||
|
@@ -64,7 +147,11 @@ func (s *service) NodePublishVolume( | |
} | ||
|
||
// Publish the volume. | ||
v.Attributes[nodeMntPathKey] = device | ||
if req.GetStagingTargetPath() != "" { | ||
v.Attributes[nodeMntPathKey] = req.GetStagingTargetPath() | ||
} else { | ||
v.Attributes[nodeMntPathKey] = device | ||
} | ||
s.vols[i] = v | ||
|
||
return &csi.NodePublishVolumeResponse{}, nil | ||
|
@@ -130,6 +217,13 @@ func (s *service) NodeGetCapabilities( | |
}, | ||
}, | ||
}, | ||
{ | ||
Type: &csi.NodeServiceCapability_Rpc{ | ||
Rpc: &csi.NodeServiceCapability_RPC{ | ||
Type: csi.NodeServiceCapability_RPC_STAGE_UNSTAGE_VOLUME, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -364,3 +364,157 @@ func testFullWorkflowSuccess(s csi.ControllerClient, c csi.NodeClient, controlle | |
}) | ||
Expect(err).NotTo(HaveOccurred()) | ||
} | ||
|
||
var _ = Describe("NodeStageVolume [Node Server]", func() { | ||
var ( | ||
s csi.ControllerClient | ||
c csi.NodeClient | ||
controllerPublishSupported bool | ||
nodeStageSupported bool | ||
device string | ||
) | ||
|
||
BeforeEach(func() { | ||
s = csi.NewControllerClient(conn) | ||
c = csi.NewNodeClient(conn) | ||
device = "/dev/mock" | ||
controllerPublishSupported = isControllerCapabilitySupported( | ||
s, | ||
csi.ControllerServiceCapability_RPC_PUBLISH_UNPUBLISH_VOLUME) | ||
nodeStageSupported = isNodeCapabilitySupported(c, csi.NodeServiceCapability_RPC_STAGE_UNSTAGE_VOLUME) | ||
if nodeStageSupported { | ||
err := createMountTargetLocation(config.StagingPath) | ||
Expect(err).NotTo(HaveOccurred()) | ||
} else { | ||
Skip("NodeStageVolume not supported") | ||
} | ||
}) | ||
|
||
It("should fail when no volume id is provided", func() { | ||
|
||
_, err := c.NodeStageVolume( | ||
context.Background(), | ||
&csi.NodeStageVolumeRequest{ | ||
StagingTargetPath: config.StagingPath, | ||
VolumeCapability: &csi.VolumeCapability{ | ||
AccessType: &csi.VolumeCapability_Mount{ | ||
Mount: &csi.VolumeCapability_MountVolume{}, | ||
}, | ||
AccessMode: &csi.VolumeCapability_AccessMode{ | ||
Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, | ||
}, | ||
}, | ||
PublishInfo: map[string]string{ | ||
"device": device, | ||
}, | ||
}) | ||
Expect(err).To(HaveOccurred()) | ||
|
||
serverError, ok := status.FromError(err) | ||
Expect(ok).To(BeTrue()) | ||
Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) | ||
}) | ||
|
||
It("should fail when no staging target path is provided", func() { | ||
|
||
_, err := c.NodeStageVolume( | ||
context.Background(), | ||
&csi.NodeStageVolumeRequest{ | ||
VolumeId: "id", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. provide the rest of the required fields. |
||
VolumeCapability: &csi.VolumeCapability{ | ||
AccessType: &csi.VolumeCapability_Mount{ | ||
Mount: &csi.VolumeCapability_MountVolume{}, | ||
}, | ||
AccessMode: &csi.VolumeCapability_AccessMode{ | ||
Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, | ||
}, | ||
}, | ||
PublishInfo: map[string]string{ | ||
"device": device, | ||
}, | ||
}) | ||
Expect(err).To(HaveOccurred()) | ||
|
||
serverError, ok := status.FromError(err) | ||
Expect(ok).To(BeTrue()) | ||
Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) | ||
}) | ||
|
||
It("should fail when no volume capability is provided", func() { | ||
|
||
_, err := c.NodeStageVolume( | ||
context.Background(), | ||
&csi.NodeStageVolumeRequest{ | ||
VolumeId: "id", | ||
StagingTargetPath: config.StagingPath, | ||
PublishInfo: map[string]string{ | ||
"device": device, | ||
}, | ||
}) | ||
Expect(err).To(HaveOccurred()) | ||
|
||
serverError, ok := status.FromError(err) | ||
Expect(ok).To(BeTrue()) | ||
Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) | ||
}) | ||
|
||
It("should return appropriate values (no optional values added)", func() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this similar enough to testFullWorkflowSuccess that we could reuse that - or make some minimal tweaks to that function and reuse it here? https://github.com/kubernetes-csi/csi-test/blob/master/pkg/sanity/node.go#L224 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's completely same. I failed to see that :) |
||
testFullWorkflowSuccess(s, c, controllerPublishSupported, nodeStageSupported) | ||
}) | ||
}) | ||
|
||
var _ = Describe("NodeUnstageVolume [Node Server]", func() { | ||
var ( | ||
s csi.ControllerClient | ||
c csi.NodeClient | ||
controllerPublishSupported bool | ||
nodeStageSupported bool | ||
) | ||
|
||
BeforeEach(func() { | ||
s = csi.NewControllerClient(conn) | ||
c = csi.NewNodeClient(conn) | ||
controllerPublishSupported = isControllerCapabilitySupported( | ||
s, | ||
csi.ControllerServiceCapability_RPC_PUBLISH_UNPUBLISH_VOLUME) | ||
nodeStageSupported = isNodeCapabilitySupported(c, csi.NodeServiceCapability_RPC_STAGE_UNSTAGE_VOLUME) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. skip test if nodeStage not supported |
||
if nodeStageSupported { | ||
err := createMountTargetLocation(config.StagingPath) | ||
Expect(err).NotTo(HaveOccurred()) | ||
} else { | ||
Skip("NodeUnstageVolume not supported") | ||
} | ||
}) | ||
|
||
It("should fail when no volume id is provided", func() { | ||
|
||
_, err := c.NodeUnstageVolume( | ||
context.Background(), | ||
&csi.NodeUnstageVolumeRequest{ | ||
StagingTargetPath: config.StagingPath, | ||
}) | ||
Expect(err).To(HaveOccurred()) | ||
|
||
serverError, ok := status.FromError(err) | ||
Expect(ok).To(BeTrue()) | ||
Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) | ||
}) | ||
|
||
It("should fail when no staging target path is provided", func() { | ||
|
||
_, err := c.NodeUnstageVolume( | ||
context.Background(), | ||
&csi.NodeUnstageVolumeRequest{ | ||
VolumeId: "id", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto: provide rest of the required args |
||
}) | ||
Expect(err).To(HaveOccurred()) | ||
|
||
serverError, ok := status.FromError(err) | ||
Expect(ok).To(BeTrue()) | ||
Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) | ||
}) | ||
|
||
It("should return appropriate values (no optional values added)", func() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this similar enough to |
||
testFullWorkflowSuccess(s, c, controllerPublishSupported, nodeStageSupported) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To support this,
STAGE_UNSTAGE_VOLUME
needs to be supported in https://github.com/kubernetes-csi/csi-test/blob/master/mock/service/node.go#L119 and the test should skip if not supported.