diff --git a/.gitignore b/.gitignore index 4b6d3bc3..1767944d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ #ignore files specific to csi-test -bin/mock-driver +bin/* cmd/csi-sanity/csi-sanity diff --git a/Makefile b/Makefile index f9d0a18f..5395ed9c 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ build-sanity: $(MAKE) -C cmd/csi-sanity all -TEST_HOSTPATH_VERSION=v1.7.3 +TEST_HOSTPATH_VERSION=v1.14.1 TEST_HOSTPATH_SOURCE=bin/hostpath-source TEST_HOSTPATH_REPO=https://github.com/kubernetes-csi/csi-driver-host-path.git bin/hostpathplugin: diff --git a/cmd/csi-sanity/main.go b/cmd/csi-sanity/main.go index d1523143..637af49b 100644 --- a/cmd/csi-sanity/main.go +++ b/cmd/csi-sanity/main.go @@ -88,6 +88,7 @@ func main() { int64Var(&config.TestVolumeSize, "testvolumesize", "Base volume size used for provisioned volumes") int64Var(&config.TestVolumeExpandSize, "testvolumeexpandsize", "Target size for expanded volumes") stringVar(&config.TestVolumeParametersFile, "testvolumeparameters", "YAML file of volume parameters for provisioned volumes") + stringVar(&config.TestVolumeMutableParametersFile, "testvolumemutableparameters", "YAML file of mutable parameters for modifying volumes") stringVar(&config.TestSnapshotParametersFile, "testsnapshotparameters", "YAML file of snapshot parameters for provisioned snapshots") boolVar(&config.TestNodeVolumeAttachLimit, "testnodevolumeattachlimit", "Test node volume attach limit") flag.Var(flag.Lookup("ginkgo.junit-report").Value, prefix+"junitfile", "JUnit XML output file where test results will be written (deprecated: use ginkgo.junit-report instead)") diff --git a/hack/e2e.sh b/hack/e2e.sh index 6092bc6d..741d0f89 100755 --- a/hack/e2e.sh +++ b/hack/e2e.sh @@ -22,6 +22,7 @@ TCP_SERVER="tcp://localhost:7654" # ... and slightly differently for gRPC. TCP_CLIENT="dns:///localhost:7654" CSI_ENDPOINTS="$CSI_ENDPOINTS ${UDS}" +MUTABLE_PARAMETER_KEY="realKey" # cleanup mock_driver_pid files... cleanup () { @@ -40,11 +41,16 @@ cleanup () { runTest() ( tmp=$(mktemp -d) - ./bin/hostpathplugin -statedir "$tmp" -endpoint "$1" -nodeid fake-node-id & + ./bin/hostpathplugin -statedir "$tmp" -endpoint "$1" -nodeid fake-node-id -enable-controller-modify-volume -accepted-mutable-parameter-names "$MUTABLE_PARAMETER_KEY" & local pid=$! - trap 'cleanup $pid $1 $tmp' EXIT + trap 'cleanup $pid $1 $tmp $mutableparametersfile' EXIT + + echo "$MUTABLE_PARAMETER_KEY: bar +" > testmutableparameters.yaml + + local mutableparametersfile="$PWD/testmutableparameters.yaml" - ./cmd/csi-sanity/csi-sanity $TESTARGS --csi.endpoint=$2 --csi.testnodevolumeattachlimit + ./cmd/csi-sanity/csi-sanity $TESTARGS --csi.endpoint=$2 --csi.testnodevolumeattachlimit --csi.testvolumemutableparameters=$mutableparametersfile ) runTestAPI() @@ -106,7 +112,7 @@ fi tmp=$(mktemp -d) ./bin/hostpathplugin -statedir "$tmp" -endpoint "$1" -nodeid fake-node-id & local pid=$! - trap 'cleanup $pid $1 $tmp $creationscriptpath $removalscriptpath' EXIT + trap 'cleanup $pid $1 $tmp $creationscriptpath $removalscriptpath $checkscriptpath' EXIT ./cmd/csi-sanity/csi-sanity $TESTARGS \ --csi.endpoint=$2 \ diff --git a/pkg/sanity/controller.go b/pkg/sanity/controller.go index c848fc99..6a6f94ec 100644 --- a/pkg/sanity/controller.go +++ b/pkg/sanity/controller.go @@ -145,6 +145,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo case csi.ControllerServiceCapability_RPC_PUBLISH_READONLY: case csi.ControllerServiceCapability_RPC_CLONE_VOLUME: case csi.ControllerServiceCapability_RPC_EXPAND_VOLUME: + case csi.ControllerServiceCapability_RPC_MODIFY_VOLUME: case csi.ControllerServiceCapability_RPC_LIST_VOLUMES_PUBLISHED_NODES: case csi.ControllerServiceCapability_RPC_GET_VOLUME: case csi.ControllerServiceCapability_RPC_VOLUME_CONDITION: @@ -199,12 +200,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo StartingToken: "invalid-token", }, ) - Expect(err).To(HaveOccurred()) - Expect(vols).To(BeNil()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.Aborted), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(vols, err, codes.Aborted) }) It("check the presence of new volumes and absence of deleted ones in the volume list", func() { @@ -375,23 +371,19 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo }) It("should fail when no name is provided", func() { - _, err := r.CreateVolume( + rsp, err := r.CreateVolume( context.Background(), &csi.CreateVolumeRequest{ Secrets: sc.Secrets.CreateVolumeSecret, Parameters: sc.Config.TestVolumeParameters, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no volume capabilities are provided", func() { name := UniqueString("sanity-controller-create-no-volume-capabilities") - _, err := r.CreateVolume( + rsp, err := r.CreateVolume( context.Background(), &csi.CreateVolumeRequest{ Name: name, @@ -399,11 +391,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo Parameters: sc.Config.TestVolumeParameters, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) // TODO: whether CreateVolume request with no capacity should fail or not depends on driver implementation @@ -522,7 +510,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo ) size2 := 2 * TestVolumeSize(sc) - _, err := r.CreateVolume( + rsp, err := r.CreateVolume( context.Background(), &csi.CreateVolumeRequest{ Name: name, @@ -537,10 +525,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo Parameters: sc.Config.TestVolumeParameters, }, ) - Expect(err).To(HaveOccurred()) - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.AlreadyExists), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.AlreadyExists) }) It("should not fail when creating volume with maximum-length name", func() { @@ -614,11 +599,8 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo }, }, } - _, err := r.CreateVolume(context.Background(), volReq) - Expect(err).To(HaveOccurred()) - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) + rsp, err := r.CreateVolume(context.Background(), volReq) + ExpectErrorCode(rsp, err, codes.NotFound) }) It("should create volume from an existing source volume", func() { @@ -660,11 +642,36 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo }, }, } + rsp, err := r.CreateVolume(context.Background(), volReq) + ExpectErrorCode(rsp, err, codes.NotFound) + }) + + It("should create volume with a volume attribute class", func() { + if !isControllerCapabilitySupported(r, csi.ControllerServiceCapability_RPC_MODIFY_VOLUME) { + Skip("Modify volume not supported") + } + + By("creating a volume") + volName := UniqueString("sanity-controller-vol-with-mutable-parameters") + volReq := MakeCreateVolumeReq(sc, volName) + volReq.MutableParameters = sc.Config.TestVolumeMutableParameters _, err := r.CreateVolume(context.Background(), volReq) - Expect(err).To(HaveOccurred()) - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should not create volume with an invalid volume attribute class", func() { + if !isControllerCapabilitySupported(r, csi.ControllerServiceCapability_RPC_MODIFY_VOLUME) { + Skip("Modify Volume not supported") + } + + By("failing to create a volume") + volName := UniqueString("sanity-controller-vol-with-mutable-parameters") + volReq := MakeCreateVolumeReq(sc, volName) + volReq.MutableParameters = map[string]string{ + "XXX_FakeKey": "XXX_FakeValue", + } + rsp, err := r.CreateVolume(context.Background(), volReq) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) }) @@ -677,17 +684,13 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo It("should fail when no volume id is provided", func() { - _, err := r.DeleteVolume( + rsp, err := r.DeleteVolume( context.Background(), &csi.DeleteVolumeRequest{ Secrets: sc.Secrets.DeleteVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should succeed when an invalid volume id is used", func() { @@ -740,16 +743,12 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo Describe("ValidateVolumeCapabilities", func() { It("should fail when no volume id is provided", func() { - _, err := r.ValidateVolumeCapabilities( + rsp, err := r.ValidateVolumeCapabilities( context.Background(), &csi.ValidateVolumeCapabilitiesRequest{ Secrets: sc.Secrets.ControllerValidateVolumeCapabilitiesSecret, }) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no volume capabilities are provided", func() { @@ -773,18 +772,14 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo }, ) - _, err := r.ValidateVolumeCapabilities( + rsp, err := r.ValidateVolumeCapabilities( context.Background(), &csi.ValidateVolumeCapabilitiesRequest{ VolumeId: vol.GetVolume().GetVolumeId(), VolumeCapabilities: nil, Secrets: sc.Secrets.ControllerValidateVolumeCapabilitiesSecret, }) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should return appropriate values (no optional values added)", func() { @@ -831,7 +826,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo It("should fail when the requested volume does not exist", func() { - _, err := r.ValidateVolumeCapabilities( + rsp, err := r.ValidateVolumeCapabilities( context.Background(), &csi.ValidateVolumeCapabilitiesRequest{ VolumeId: sc.Config.IDGen.GenerateUniqueValidVolumeID(), @@ -841,11 +836,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo Secrets: sc.Secrets.ControllerValidateVolumeCapabilitiesSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.NotFound) }) }) @@ -858,38 +849,30 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo It("should fail when no volume id is provided", func() { - _, err := r.ControllerPublishVolume( + rsp, err := r.ControllerPublishVolume( context.Background(), &csi.ControllerPublishVolumeRequest{ Secrets: sc.Secrets.ControllerPublishVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no node id is provided", func() { - _, err := r.ControllerPublishVolume( + rsp, err := r.ControllerPublishVolume( context.Background(), &csi.ControllerPublishVolumeRequest{ VolumeId: sc.Config.IDGen.GenerateUniqueValidVolumeID(), Secrets: sc.Secrets.ControllerPublishVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no volume capability is provided", func() { - _, err := r.ControllerPublishVolume( + rsp, err := r.ControllerPublishVolume( context.Background(), &csi.ControllerPublishVolumeRequest{ VolumeId: sc.Config.IDGen.GenerateUniqueValidVolumeID(), @@ -897,11 +880,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo Secrets: sc.Secrets.ControllerPublishVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when publishing more volumes than the node max attach limit", func() { @@ -958,12 +937,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo Secrets: sc.Secrets.ControllerPublishVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - Expect(conpubvol).To(BeNil()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(conpubvol, err, codes.NotFound) }) It("should fail when the node does not exist", func() { @@ -1051,12 +1025,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo pubReq.Readonly = true conpubvol, err = r.ControllerPublishVolume(context.Background(), pubReq) - Expect(err).To(HaveOccurred()) - Expect(conpubvol).To(BeNil()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.AlreadyExists), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(conpubvol, err, codes.AlreadyExists) }) }) @@ -1085,17 +1054,13 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo It("should fail when no volume id is provided", func() { - _, err := r.ControllerUnpublishVolume( + rsp, err := r.ControllerUnpublishVolume( context.Background(), &csi.ControllerUnpublishVolumeRequest{ Secrets: sc.Secrets.ControllerUnpublishVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) }) }) @@ -1378,12 +1343,8 @@ var _ = DescribeSanity("DeleteSnapshot [Controller Server]", func(sc *TestContex req.Secrets = sc.Secrets.DeleteSnapshotSecret } - _, err := r.DeleteSnapshot(context.Background(), req) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + rsp, err := r.DeleteSnapshot(context.Background(), req) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should succeed when an invalid snapshot id is used", func() { @@ -1436,11 +1397,8 @@ var _ = DescribeSanity("CreateSnapshot [Controller Server]", func(sc *TestContex req.Secrets = sc.Secrets.CreateSnapshotSecret } - _, err := r.CreateSnapshot(context.Background(), req) - Expect(err).To(HaveOccurred()) - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + rsp, err := r.CreateSnapshot(context.Background(), req) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no source volume id is provided", func() { @@ -1453,11 +1411,8 @@ var _ = DescribeSanity("CreateSnapshot [Controller Server]", func(sc *TestContex req.Secrets = sc.Secrets.CreateSnapshotSecret } - _, err := r.CreateSnapshot(context.Background(), req) - Expect(err).To(HaveOccurred()) - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + rsp, err := r.CreateSnapshot(context.Background(), req) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should succeed when requesting to create a snapshot with already existing name and same source volume ID", func() { @@ -1487,11 +1442,8 @@ var _ = DescribeSanity("CreateSnapshot [Controller Server]", func(sc *TestContex By("creating a snapshot with the same name but different source volume ID") req := MakeCreateSnapshotReq(sc, snapshotName, volume2.GetVolume().GetVolumeId()) - _, err := r.CreateSnapshot(context.Background(), req) - Expect(err).To(HaveOccurred()) - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.AlreadyExists), "unexpected error: %s", serverError.Message()) + rsp, err := r.CreateSnapshot(context.Background(), req) + ExpectErrorCode(rsp, err, codes.AlreadyExists) }) It("should succeed when creating snapshot with maximum-length name", func() { @@ -1530,20 +1482,9 @@ var _ = DescribeSanity("ExpandVolume [Controller Server]", func(sc *TestContext) }) It("should fail if no volume id is given", func() { - expReq := &csi.ControllerExpandVolumeRequest{ - VolumeId: "", - CapacityRange: &csi.CapacityRange{ - RequiredBytes: TestVolumeExpandSize(sc), - }, - Secrets: sc.Secrets.ControllerExpandVolumeSecret, - } + expReq := MakeExpandVolumeReq(sc, "") rsp, err := r.ControllerExpandVolume(context.Background(), expReq) - Expect(err).To(HaveOccurred()) - Expect(rsp).To(BeNil()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail if no capacity range is given", func() { @@ -1552,12 +1493,7 @@ var _ = DescribeSanity("ExpandVolume [Controller Server]", func(sc *TestContext) Secrets: sc.Secrets.ControllerExpandVolumeSecret, } rsp, err := r.ControllerExpandVolume(context.Background(), expReq) - Expect(err).To(HaveOccurred()) - Expect(rsp).To(BeNil()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should work", func() { @@ -1580,18 +1516,116 @@ var _ = DescribeSanity("ExpandVolume [Controller Server]", func(sc *TestContext) vol := r.MustCreateVolume(context.Background(), req) By("expanding the volume") - expReq := &csi.ControllerExpandVolumeRequest{ + expReq := MakeExpandVolumeReq(sc, vol.GetVolume().GetVolumeId()) + rsp, err := r.ControllerExpandVolume(context.Background(), expReq) + Expect(err).NotTo(HaveOccurred()) + Expect(rsp).NotTo(BeNil()) + Expect(rsp.GetCapacityBytes()).To(Equal(TestVolumeExpandSize(sc))) + }) +}) + +var _ = DescribeSanity("ModifyVolume [Controller Server]", func(sc *TestContext) { + var r *Resources + + BeforeEach(func() { + r = &Resources{ + ControllerClient: csi.NewControllerClient(sc.ControllerConn), + Context: sc, + } + + if !isControllerCapabilitySupported(r, csi.ControllerServiceCapability_RPC_MODIFY_VOLUME) { + Skip("ControllerModifyVolume not supported") + } + }) + + AfterEach(func() { + r.Cleanup() + }) + + It("should fail if no volume id is given", func() { + modifyReq := MakeModifyVolumeReq(sc, "") + rsp, err := r.ControllerModifyVolume(context.Background(), modifyReq) + ExpectErrorCode(rsp, err, codes.InvalidArgument) + }) + + It("should fail if volume does not exist", func() { + modifyReq := MakeModifyVolumeReq(sc, "non-existing-volume-id") + rsp, err := r.ControllerModifyVolume(context.Background(), modifyReq) + ExpectErrorCode(rsp, err, codes.NotFound) + }) + + It("should fail if specified mutable parameters are not supported by the volume", func() { + + By("creating a new volume") + + volReq := MakeCreateVolumeReq(sc, UniqueString("sanity-modify-volume")) + vol := r.MustCreateVolume(context.Background(), volReq) + + By("failing to modify the volume") + + modifyReq := &csi.ControllerModifyVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - CapacityRange: &csi.CapacityRange{ - RequiredBytes: TestVolumeExpandSize(sc), + MutableParameters: map[string]string{ + "XXX_FakeKey": "XXX_FakeValue", }, - Secrets: sc.Secrets.ControllerExpandVolumeSecret, - VolumeCapability: TestVolumeCapabilityWithAccessType(sc, csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER), + Secrets: sc.Secrets.ControllerModifyVolumeSecret, } - rsp, err := r.ControllerExpandVolume(context.Background(), expReq) + rsp, err := r.ControllerModifyVolume(context.Background(), modifyReq) + ExpectErrorCode(rsp, err, codes.InvalidArgument) + }) + + It("should modify a volume created without a volume attribute class", func() { + + By("creating a new volume") + + volReq := MakeCreateVolumeReq(sc, UniqueString("sanity-modify-volume")) + vol := r.MustCreateVolume(context.Background(), volReq) + + By("modifying the volume") + + modifyReq := MakeModifyVolumeReq(sc, vol.GetVolume().GetVolumeId()) + rsp, err := r.ControllerModifyVolume(context.Background(), modifyReq) + Expect(err).NotTo(HaveOccurred()) Expect(rsp).NotTo(BeNil()) - Expect(rsp.GetCapacityBytes()).To(Equal(TestVolumeExpandSize(sc))) + }) + + It("should modify a volume created with a volume attribute class", func() { + + By("creating a new volume with volume attribute class") + + volReq := MakeCreateVolumeReq(sc, UniqueString("sanity-modify-volume")) + volReq.MutableParameters = sc.Config.TestVolumeMutableParameters + vol := r.MustCreateVolume(context.Background(), volReq) + + By("modifying the volume") + + modifyReq := MakeModifyVolumeReq(sc, vol.GetVolume().GetVolumeId()) + rsp, err := r.ControllerModifyVolume(context.Background(), modifyReq) + + Expect(err).NotTo(HaveOccurred()) + Expect(rsp).NotTo(BeNil()) + }) + + It("should fail to modify a volume created with a volume attribute class if new mutable parameters are not supported by volume", func() { + + By("creating a new volume with volume attribute class") + + volReq := MakeCreateVolumeReq(sc, UniqueString("sanity-modify-volume")) + volReq.MutableParameters = sc.Config.TestVolumeMutableParameters + vol := r.MustCreateVolume(context.Background(), volReq) + + By("failing to modify the volume") + + modifyReq := &csi.ControllerModifyVolumeRequest{ + VolumeId: vol.GetVolume().GetVolumeId(), + MutableParameters: map[string]string{ + "XXX_FakeKey": "XXX_FakeValue", + }, + Secrets: sc.Secrets.ControllerModifyVolumeSecret, + } + rsp, err := r.ControllerModifyVolume(context.Background(), modifyReq) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) }) @@ -1675,6 +1709,27 @@ func MakeControllerUnpublishVolumeReq(sc *TestContext, volID, nodeID string) *cs } } +// MakeExpandVolumeReq creates and returns a ControllerExpandVolumeRequest. +func MakeExpandVolumeReq(sc *TestContext, volID string) *csi.ControllerExpandVolumeRequest { + return &csi.ControllerExpandVolumeRequest{ + VolumeId: volID, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: TestVolumeExpandSize(sc), + }, + Secrets: sc.Secrets.ControllerExpandVolumeSecret, + VolumeCapability: TestVolumeCapabilityWithAccessType(sc, csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER), + } +} + +// MakeModifyVolumeReq creates and returns a ControllerModifyVolumeRequest. +func MakeModifyVolumeReq(sc *TestContext, volID string) *csi.ControllerModifyVolumeRequest { + return &csi.ControllerModifyVolumeRequest{ + VolumeId: volID, + MutableParameters: sc.Config.TestVolumeMutableParameters, + Secrets: sc.Secrets.ControllerModifyVolumeSecret, + } +} + // VolumeLifecycle performs Create-Publish-Unpublish-Delete, with optional repeat count to test idempotency. func VolumeLifecycle(r *Resources, sc *TestContext, count int) { // CSI spec poses no specific requirements for the cluster/storage setups that a SP MUST support. To perform diff --git a/pkg/sanity/groupcontroller.go b/pkg/sanity/groupcontroller.go index 79c87da6..71c275e6 100644 --- a/pkg/sanity/groupcontroller.go +++ b/pkg/sanity/groupcontroller.go @@ -20,10 +20,8 @@ import ( "context" "fmt" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - "github.com/container-storage-interface/spec/lib/go/csi" + "google.golang.org/grpc/codes" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -156,64 +154,48 @@ var _ = DescribeSanity("GroupController Service [GroupController VolumeGroupSnap Describe("CreateVolumeGroupSnapshot", func() { It("should fail when no name is provided", func() { - _, err := r.CreateVolumeGroupSnapshot( + rsp, err := r.CreateVolumeGroupSnapshot( context.Background(), &csi.CreateVolumeGroupSnapshotRequest{ Secrets: sc.Secrets.CreateSnapshotSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) }) Describe("GetVolumeGroupSnapshot", func() { It("should fail when no volume id is provided", func() { - _, err := r.GetVolumeGroupSnapshot( + rsp, err := r.GetVolumeGroupSnapshot( context.Background(), &csi.GetVolumeGroupSnapshotRequest{ Secrets: sc.Secrets.ListSnapshotsSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when an invalid volume id is used", func() { - _, err := r.GetVolumeGroupSnapshot( + rsp, err := r.GetVolumeGroupSnapshot( context.Background(), &csi.GetVolumeGroupSnapshotRequest{ GroupSnapshotId: sc.Config.IDGen.GenerateInvalidVolumeID(), Secrets: sc.Secrets.ListSnapshotsSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.NotFound) }) }) Describe("DeleteVolumeGroupSnapshot", func() { It("should fail when no volume id is provided", func() { - _, err := r.DeleteVolumeGroupSnapshot( + rsp, err := r.DeleteVolumeGroupSnapshot( context.Background(), &csi.DeleteVolumeGroupSnapshotRequest{ Secrets: sc.Secrets.DeleteSnapshotSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should succeed when an invalid volume id is used", func() { diff --git a/pkg/sanity/node.go b/pkg/sanity/node.go index 893368b8..9ab3ad69 100644 --- a/pkg/sanity/node.go +++ b/pkg/sanity/node.go @@ -20,10 +20,8 @@ import ( "context" "fmt" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - "github.com/container-storage-interface/spec/lib/go/csi" + "google.golang.org/grpc/codes" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -377,36 +375,28 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { Describe("NodePublishVolume", func() { It("should fail when no volume id is provided", func() { - _, err := r.NodePublishVolume( + rsp, err := r.NodePublishVolume( context.Background(), &csi.NodePublishVolumeRequest{ Secrets: sc.Secrets.NodePublishVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no target path is provided", func() { - _, err := r.NodePublishVolume( + rsp, err := r.NodePublishVolume( context.Background(), &csi.NodePublishVolumeRequest{ VolumeId: sc.Config.IDGen.GenerateUniqueValidVolumeID(), Secrets: sc.Secrets.NodePublishVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no volume capability is provided", func() { - _, err := r.NodePublishVolume( + rsp, err := r.NodePublishVolume( context.Background(), &csi.NodePublishVolumeRequest{ VolumeId: sc.Config.IDGen.GenerateUniqueValidVolumeID(), @@ -415,39 +405,27 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { Secrets: sc.Secrets.NodePublishVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) }) Describe("NodeUnpublishVolume", func() { It("should fail when no volume id is provided", func() { - _, err := r.NodeUnpublishVolume( + rsp, err := r.NodeUnpublishVolume( context.Background(), &csi.NodeUnpublishVolumeRequest{}) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no target path is provided", func() { - _, err := r.NodeUnpublishVolume( + rsp, err := r.NodeUnpublishVolume( context.Background(), &csi.NodeUnpublishVolumeRequest{ VolumeId: sc.Config.IDGen.GenerateUniqueValidVolumeID(), }) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should remove target path", func() { @@ -523,7 +501,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { }) It("should fail when no volume id is provided", func() { - _, err := r.NodeStageVolume( + rsp, err := r.NodeStageVolume( context.Background(), &csi.NodeStageVolumeRequest{ StagingTargetPath: sc.StagingPath, @@ -534,15 +512,11 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { Secrets: sc.Secrets.NodeStageVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no staging target path is provided", func() { - _, err := r.NodeStageVolume( + rsp, err := r.NodeStageVolume( context.Background(), &csi.NodeStageVolumeRequest{ VolumeId: sc.Config.IDGen.GenerateUniqueValidVolumeID(), @@ -553,11 +527,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { Secrets: sc.Secrets.NodeStageVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no volume capability is provided", func() { @@ -585,7 +555,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { }, ) - _, err := r.NodeStageVolume( + rsp, err := r.NodeStageVolume( context.Background(), &csi.NodeStageVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), @@ -596,11 +566,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { Secrets: sc.Secrets.NodeStageVolumeSecret, }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) }) @@ -613,30 +579,22 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { It("should fail when no volume id is provided", func() { - _, err := r.NodeUnstageVolume( + rsp, err := r.NodeUnstageVolume( context.Background(), &csi.NodeUnstageVolumeRequest{ StagingTargetPath: sc.StagingPath, }) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no staging target path is provided", func() { - _, err := r.NodeUnstageVolume( + rsp, err := r.NodeUnstageVolume( context.Background(), &csi.NodeUnstageVolumeRequest{ VolumeId: sc.Config.IDGen.GenerateUniqueValidVolumeID(), }) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) }) @@ -648,46 +606,34 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { }) It("should fail when no volume id is provided", func() { - _, err := r.NodeGetVolumeStats( + rsp, err := r.NodeGetVolumeStats( context.Background(), &csi.NodeGetVolumeStatsRequest{ VolumePath: "some/path", }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no volume path is provided", func() { - _, err := r.NodeGetVolumeStats( + rsp, err := r.NodeGetVolumeStats( context.Background(), &csi.NodeGetVolumeStatsRequest{ VolumeId: sc.Config.IDGen.GenerateUniqueValidVolumeID(), }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when volume is not found", func() { - _, err := r.NodeGetVolumeStats( + rsp, err := r.NodeGetVolumeStats( context.Background(), &csi.NodeGetVolumeStatsRequest{ VolumeId: sc.Config.IDGen.GenerateUniqueValidVolumeID(), VolumePath: "some/path", }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.NotFound) }) It("should fail when volume does not exist on the specified path", func() { @@ -713,18 +659,14 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { // NodeGetVolumeStats By("Get node volume stats") - _, err = r.NodeGetVolumeStats( + rsp, err := r.NodeGetVolumeStats( context.Background(), &csi.NodeGetVolumeStatsRequest{ VolumeId: vol.GetVolume().GetVolumeId(), VolumePath: "some/path", }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.NotFound) }) }) @@ -738,18 +680,14 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { }) It("should fail when no volume id is provided", func() { - _, err := r.NodeExpandVolume( + rsp, err := r.NodeExpandVolume( context.Background(), &csi.NodeExpandVolumeRequest{ VolumePath: sc.TargetPath, VolumeCapability: TestVolumeCapabilityWithAccessType(sc, csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER), }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when no volume path is provided", func() { @@ -757,33 +695,25 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { vol := createVolume(name) - _, err := r.NodeExpandVolume( + rsp, err := r.NodeExpandVolume( context.Background(), &csi.NodeExpandVolumeRequest{ VolumeId: vol.GetVolume().VolumeId, VolumeCapability: TestVolumeCapabilityWithAccessType(sc, csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER), }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.InvalidArgument) }) It("should fail when volume is not found", func() { - _, err := r.NodeExpandVolume( + rsp, err := r.NodeExpandVolume( context.Background(), &csi.NodeExpandVolumeRequest{ VolumeId: sc.Config.IDGen.GenerateUniqueValidVolumeID(), VolumePath: "some/path", }, ) - Expect(err).To(HaveOccurred()) - - serverError, ok := status.FromError(err) - Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) + ExpectErrorCode(rsp, err, codes.NotFound) }) It("should work if node-expand is called after node-publish", func() { diff --git a/pkg/sanity/sanity.go b/pkg/sanity/sanity.go index 789896e2..53511cb7 100644 --- a/pkg/sanity/sanity.go +++ b/pkg/sanity/sanity.go @@ -47,6 +47,7 @@ type CSISecrets struct { CreateSnapshotSecret map[string]string `yaml:"CreateSnapshotSecret"` DeleteSnapshotSecret map[string]string `yaml:"DeleteSnapshotSecret"` ControllerExpandVolumeSecret map[string]string `yaml:"ControllerExpandVolumeSecret"` + ControllerModifyVolumeSecret map[string]string `yaml:"ControllerModifyVolumeSecret"` ListSnapshotsSecret map[string]string `yaml:"ListSnapshotsSecret"` } @@ -101,6 +102,10 @@ type TestConfig struct { TestSnapshotParametersFile string TestSnapshotParameters map[string]string + // TestVolumeMutableParametersFile for setting ModifyVolumeRequest.MutableParameters. + TestVolumeMutableParametersFile string + TestVolumeMutableParameters map[string]string + // Callback functions to customize the creation of target and staging // directories. Returns the new paths for mount and staging. // If not defined, directories are created in the default way at TargetPath @@ -245,6 +250,8 @@ func (sc *TestContext) Setup() { loadFromFile(sc.Config.TestVolumeParametersFile, &sc.Config.TestVolumeParameters) // Get VolumeSnapshotClass parameters from TestSnapshotParametersFile loadFromFile(sc.Config.TestSnapshotParametersFile, &sc.Config.TestSnapshotParameters) + // Get VolumeAttributeClass parameters from TestVolumeMutableParametersFile + loadFromFile(sc.Config.TestVolumeMutableParametersFile, &sc.Config.TestVolumeMutableParameters) if len(sc.Config.SecretsFile) > 0 { sc.Secrets, err = loadSecrets(sc.Config.SecretsFile) diff --git a/pkg/sanity/util.go b/pkg/sanity/util.go index dab2b100..eb890aaf 100644 --- a/pkg/sanity/util.go +++ b/pkg/sanity/util.go @@ -18,8 +18,11 @@ package sanity import ( "fmt" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/google/uuid" + . "github.com/onsi/gomega" ) // IDGenerator generates valid and invalid Volume and Node IDs to be used in @@ -64,3 +67,13 @@ func (d DefaultIDGenerator) GenerateUniqueValidNodeID() string { func (d DefaultIDGenerator) GenerateInvalidNodeID() string { return "fake-node-id" } + +// ExpectErrorCode confirms that the correct error code was returned +func ExpectErrorCode(response any, err error, code codes.Code) { + Expect(err).To(HaveOccurred()) + Expect(response).To(BeNil()) + + serverError, ok := status.FromError(err) + Expect(ok).To(BeTrue()) + Expect(serverError.Code()).To(Equal(code), "unexpected error: %s", serverError.Message()) +}