From df74ae01d5ee8d444f283c266061fb95c27d249c Mon Sep 17 00:00:00 2001 From: Mario Valderrama Date: Tue, 13 Apr 2021 17:07:45 +0200 Subject: [PATCH 1/2] Provide more context for status code mismatch errors The default `Expect` error message when comparing gRPC status codes is only somewhat useful on its own. Usually the message contains valuable information to better debug the problem. I only noticed this for a small subset of calls, but I figured it might be useful for the whole file. --- pkg/sanity/node.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/pkg/sanity/node.go b/pkg/sanity/node.go index d8c8b806..d67c175d 100644 --- a/pkg/sanity/node.go +++ b/pkg/sanity/node.go @@ -387,7 +387,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no target path is provided", func() { @@ -402,7 +402,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no volume capability is provided", func() { @@ -419,7 +419,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) }) @@ -433,7 +433,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no target path is provided", func() { @@ -447,7 +447,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) }) @@ -480,7 +480,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no staging target path is provided", func() { @@ -499,7 +499,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no volume capability is provided", func() { @@ -542,7 +542,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) }) @@ -564,7 +564,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no staging target path is provided", func() { @@ -578,7 +578,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) }) @@ -600,7 +600,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no volume path is provided", func() { @@ -614,7 +614,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when volume is not found", func() { @@ -629,7 +629,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound)) + Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) }) It("should fail when volume does not exist on the specified path", func() { @@ -666,7 +666,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound)) + Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) }) }) @@ -691,7 +691,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no volume path is provided", func() { @@ -710,7 +710,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when volume is not found", func() { @@ -725,7 +725,7 @@ var _ = DescribeSanity("Node Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound)) + Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) }) It("should work if node-expand is called after node-publish", func() { From bb5ec319f02ace4da9c1cc41836e19dee7644770 Mon Sep 17 00:00:00 2001 From: Mario Valderrama Date: Wed, 14 Apr 2021 14:58:52 +0200 Subject: [PATCH 2/2] More error context in controller and identity sanity tests --- pkg/sanity/controller.go | 46 ++++++++++++++++++++-------------------- pkg/sanity/identity.go | 2 +- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/pkg/sanity/controller.go b/pkg/sanity/controller.go index 84ed876c..2b2d743c 100644 --- a/pkg/sanity/controller.go +++ b/pkg/sanity/controller.go @@ -202,7 +202,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.Aborted)) + Expect(serverError.Code()).To(Equal(codes.Aborted), "unexpected error: %s", serverError.Message()) }) It("check the presence of new volumes and absence of deleted ones in the volume list", func() { @@ -384,7 +384,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no volume capabilities are provided", func() { @@ -401,7 +401,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) // TODO: whether CreateVolume request with no capacity should fail or not depends on driver implementation @@ -538,7 +538,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo Expect(err).To(HaveOccurred()) serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.AlreadyExists)) + Expect(serverError.Code()).To(Equal(codes.AlreadyExists), "unexpected error: %s", serverError.Message()) }) It("should not fail when creating volume with maximum-length name", func() { @@ -610,7 +610,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo Expect(err).To(HaveOccurred()) serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound)) + Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) }) It("should create volume from an existing source volume", func() { @@ -656,7 +656,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo Expect(err).To(HaveOccurred()) serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound)) + Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) }) }) @@ -679,7 +679,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should succeed when an invalid volume id is used", func() { @@ -741,7 +741,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no volume capabilities are provided", func() { @@ -776,7 +776,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should return appropriate values (no optional values added)", func() { @@ -837,7 +837,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound)) + Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) }) }) @@ -860,7 +860,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no node id is provided", func() { @@ -876,7 +876,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no volume capability is provided", func() { @@ -893,7 +893,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when publishing more volumes than the node max attach limit", func() { @@ -955,7 +955,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound)) + Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) }) It("should fail when the node does not exist", func() { @@ -994,7 +994,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.NotFound)) + Expect(serverError.Code()).To(Equal(codes.NotFound), "unexpected error: %s", serverError.Message()) }) It("should fail when the volume is already published but is incompatible", func() { @@ -1048,7 +1048,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.AlreadyExists)) + Expect(serverError.Code()).To(Equal(codes.AlreadyExists), "unexpected error: %s", serverError.Message()) }) }) @@ -1087,7 +1087,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) }) }) @@ -1337,7 +1337,7 @@ var _ = DescribeSanity("DeleteSnapshot [Controller Server]", func(sc *TestContex serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should succeed when an invalid snapshot id is used", func() { @@ -1394,7 +1394,7 @@ var _ = DescribeSanity("CreateSnapshot [Controller Server]", func(sc *TestContex Expect(err).To(HaveOccurred()) serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail when no source volume id is provided", func() { @@ -1411,7 +1411,7 @@ var _ = DescribeSanity("CreateSnapshot [Controller Server]", func(sc *TestContex Expect(err).To(HaveOccurred()) serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should succeed when requesting to create a snapshot with already existing name and same source volume ID", func() { @@ -1444,7 +1444,7 @@ var _ = DescribeSanity("CreateSnapshot [Controller Server]", func(sc *TestContex Expect(err).To(HaveOccurred()) serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.AlreadyExists)) + Expect(serverError.Code()).To(Equal(codes.AlreadyExists), "unexpected error: %s", serverError.Message()) }) It("should succeed when creating snapshot with maximum-length name", func() { @@ -1500,7 +1500,7 @@ var _ = DescribeSanity("ExpandVolume [Controller Server]", func(sc *TestContext) serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should fail if no capacity range is given", func() { @@ -1514,7 +1514,7 @@ var _ = DescribeSanity("ExpandVolume [Controller Server]", func(sc *TestContext) serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) - Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) + Expect(serverError.Code()).To(Equal(codes.InvalidArgument), "unexpected error: %s", serverError.Message()) }) It("should work", func() { diff --git a/pkg/sanity/identity.go b/pkg/sanity/identity.go index 05c42449..2a93a9ca 100644 --- a/pkg/sanity/identity.go +++ b/pkg/sanity/identity.go @@ -83,7 +83,7 @@ var _ = DescribeSanity("Identity Service", func(sc *TestContext) { serverError, ok := status.FromError(err) Expect(ok).To(BeTrue()) Expect(serverError.Code() == codes.FailedPrecondition || - serverError.Code() == codes.OK).To(BeTrue()) + serverError.Code() == codes.OK).To(BeTrue(), "unexpected error: %s", serverError.Message()) if res.GetReady() != nil { Expect(res.GetReady().GetValue() == true ||