From 5a30e0deee131e962c6052e225590111cf3ef157 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 23 May 2018 20:36:21 +0200 Subject: [PATCH 1/4] Remove use of deprecated grpc.ErrorDesc() Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 2dfbafc1d21847497cc39c17b21363fb5c7332ab) Signed-off-by: Sebastiaan van Stijn --- cmd/swarmctl/main.go | 4 +++- integration/cluster.go | 2 +- manager/controlapi/ca_rotation_test.go | 6 ++--- manager/controlapi/config_test.go | 31 ++++++++++++------------ manager/controlapi/secret_test.go | 33 +++++++++++++------------- manager/controlapi/service_test.go | 5 ++-- manager/dispatcher/dispatcher_test.go | 4 ++-- manager/manager_test.go | 10 ++++---- manager/state/raft/raft_test.go | 2 +- manager/state/raft/transport/peer.go | 3 ++- testutils/grpc.go | 12 ++++++++++ 11 files changed, 65 insertions(+), 47 deletions(-) create mode 100644 testutils/grpc.go diff --git a/cmd/swarmctl/main.go b/cmd/swarmctl/main.go index 1a76d9c01c..250e8faa05 100644 --- a/cmd/swarmctl/main.go +++ b/cmd/swarmctl/main.go @@ -15,11 +15,13 @@ import ( "github.com/spf13/cobra" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func main() { if c, err := mainCmd.ExecuteC(); err != nil { - c.Println("Error:", grpc.ErrorDesc(err)) + s, _ := status.FromError(err) + c.Println("Error:", s.Message()) // if it's not a grpc, we assume it's a user error and we display the usage. if grpc.Code(err) == codes.Unknown { c.Println(c.UsageString()) diff --git a/integration/cluster.go b/integration/cluster.go index e46e01edbe..e199d48f86 100644 --- a/integration/cluster.go +++ b/integration/cluster.go @@ -322,7 +322,7 @@ func (c *testCluster) SetNodeRole(id string, role api.NodeRole) error { }); err != nil { // there possible problems on calling update node because redirecting // node or leader might want to shut down - if grpc.ErrorDesc(err) == "update out of sequence" { + if testutils.ErrorDesc(err) == "update out of sequence" { continue } return err diff --git a/manager/controlapi/ca_rotation_test.go b/manager/controlapi/ca_rotation_test.go index 1fccd14385..0a80fc4584 100644 --- a/manager/controlapi/ca_rotation_test.go +++ b/manager/controlapi/ca_rotation_test.go @@ -3,18 +3,18 @@ package controlapi import ( "context" "crypto/x509" + "encoding/pem" "io/ioutil" "os" "testing" "time" - "encoding/pem" - "github.com/cloudflare/cfssl/helpers" "github.com/cloudflare/cfssl/initca" "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/ca" "github.com/docker/swarmkit/ca/testutils" + grpcutils "github.com/docker/swarmkit/testutils" "github.com/stretchr/testify/require" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -310,7 +310,7 @@ func TestValidateCAConfigInvalidValues(t *testing.T) { _, err := validateCAConfig(context.Background(), secConfig, cluster) require.Error(t, err, invalid.expectErrorString) require.Equal(t, codes.InvalidArgument, grpc.Code(err), invalid.expectErrorString) - require.Contains(t, grpc.ErrorDesc(err), invalid.expectErrorString) + require.Contains(t, grpcutils.ErrorDesc(err), invalid.expectErrorString) } } diff --git a/manager/controlapi/config_test.go b/manager/controlapi/config_test.go index 3774527442..f231f619ee 100644 --- a/manager/controlapi/config_test.go +++ b/manager/controlapi/config_test.go @@ -7,6 +7,7 @@ import ( "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/manager/state/store" + "github.com/docker/swarmkit/testutils" "github.com/stretchr/testify/assert" "golang.org/x/net/context" "google.golang.org/grpc" @@ -54,7 +55,7 @@ func TestValidateConfigSpec(t *testing.T) { } { err := validateConfigSpec(createConfigSpec(badName, []byte("valid config"), nil)) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) } for _, badSpec := range []*api.ConfigSpec{ @@ -63,7 +64,7 @@ func TestValidateConfigSpec(t *testing.T) { } { err := validateConfigSpec(badSpec) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) } for _, goodName := range []string{ @@ -98,7 +99,7 @@ func TestCreateConfig(t *testing.T) { // ---- creating a config with an invalid spec fails, thus checking that CreateConfig validates the spec ---- _, err := ts.Client.CreateConfig(context.Background(), &api.CreateConfigRequest{Spec: createConfigSpec("", nil, nil)}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // ---- creating a config with a valid spec succeeds, and returns a config that reflects the config in the store // exactly @@ -123,7 +124,7 @@ func TestCreateConfig(t *testing.T) { // ---- creating a config with the same name, even if it's the exact same spec, fails due to a name conflict ---- _, err = ts.Client.CreateConfig(context.Background(), &validSpecRequest) assert.Error(t, err) - assert.Equal(t, codes.AlreadyExists, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.AlreadyExists, grpc.Code(err), testutils.ErrorDesc(err)) } func TestGetConfig(t *testing.T) { @@ -133,12 +134,12 @@ func TestGetConfig(t *testing.T) { // ---- getting a config without providing an ID results in an InvalidArgument ---- _, err := ts.Client.GetConfig(context.Background(), &api.GetConfigRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // ---- getting a non-existent config fails with NotFound ---- _, err = ts.Client.GetConfig(context.Background(), &api.GetConfigRequest{ConfigID: "12345"}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) // ---- getting an existing config returns the config ---- config := configFromConfigSpec(createConfigSpec("name", []byte("data"), nil)) @@ -168,12 +169,12 @@ func TestUpdateConfig(t *testing.T) { // updating a config without providing an ID results in an InvalidArgument _, err = ts.Client.UpdateConfig(context.Background(), &api.UpdateConfigRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // getting a non-existent config fails with NotFound _, err = ts.Client.UpdateConfig(context.Background(), &api.UpdateConfigRequest{ConfigID: "1234adsaa", ConfigVersion: &api.Version{Index: 1}}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) // updating an existing config's data returns an error config.Spec.Data = []byte{1} @@ -182,7 +183,7 @@ func TestUpdateConfig(t *testing.T) { Spec: &config.Spec, ConfigVersion: &config.Meta.Version, }) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // updating an existing config's Name returns an error config.Spec.Data = nil @@ -192,7 +193,7 @@ func TestUpdateConfig(t *testing.T) { Spec: &config.Spec, ConfigVersion: &config.Meta.Version, }) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // updating the config with the original spec succeeds config.Spec.Data = []byte("data") @@ -244,7 +245,7 @@ func TestRemoveUnusedConfig(t *testing.T) { // removing a config without providing an ID results in an InvalidArgument _, err := ts.Client.RemoveConfig(context.Background(), &api.RemoveConfigRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // removing a config that exists succeeds config := configFromConfigSpec(createConfigSpec("name", []byte("data"), nil)) @@ -260,7 +261,7 @@ func TestRemoveUnusedConfig(t *testing.T) { // ---- it was really removed because attempting to remove it again fails with a NotFound ---- _, err = ts.Client.RemoveConfig(context.Background(), &api.RemoveConfigRequest{ConfigID: config.ID}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) } @@ -301,8 +302,8 @@ func TestRemoveUsedConfig(t *testing.T) { // removing a config that exists but is in use fails _, err = ts.Client.RemoveConfig(context.Background(), &api.RemoveConfigRequest{ConfigID: resp.Config.ID}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) - assert.Regexp(t, "service[1-2], service[1-2]", grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Regexp(t, "service[1-2], service[1-2]", testutils.ErrorDesc(err)) // removing a config that exists but is not in use succeeds _, err = ts.Client.RemoveConfig(context.Background(), &api.RemoveConfigRequest{ConfigID: resp2.Config.ID}) @@ -311,7 +312,7 @@ func TestRemoveUsedConfig(t *testing.T) { // it was really removed because attempting to remove it again fails with a NotFound _, err = ts.Client.RemoveConfig(context.Background(), &api.RemoveConfigRequest{ConfigID: resp2.Config.ID}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) } func TestListConfigs(t *testing.T) { diff --git a/manager/controlapi/secret_test.go b/manager/controlapi/secret_test.go index cf66e39144..8a7c693ffb 100644 --- a/manager/controlapi/secret_test.go +++ b/manager/controlapi/secret_test.go @@ -7,6 +7,7 @@ import ( "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/manager/state/store" + "github.com/docker/swarmkit/testutils" "github.com/stretchr/testify/assert" "golang.org/x/net/context" "google.golang.org/grpc" @@ -54,7 +55,7 @@ func TestValidateSecretSpec(t *testing.T) { } { err := validateSecretSpec(createSecretSpec(badName, []byte("valid secret"), nil)) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) } for _, badSpec := range []*api.SecretSpec{ @@ -63,7 +64,7 @@ func TestValidateSecretSpec(t *testing.T) { } { err := validateSecretSpec(badSpec) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) } for _, goodName := range []string{ @@ -95,7 +96,7 @@ func TestValidateSecretSpec(t *testing.T) { spec.Driver = &api.Driver{} err := validateSecretSpec(spec) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) spec.Driver.Name = "secret-driver" err = validateSecretSpec(spec) assert.NoError(t, err) @@ -108,7 +109,7 @@ func TestCreateSecret(t *testing.T) { // ---- creating a secret with an invalid spec fails, thus checking that CreateSecret validates the spec ---- _, err := ts.Client.CreateSecret(context.Background(), &api.CreateSecretRequest{Spec: createSecretSpec("", nil, nil)}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // ---- creating a secret with a valid spec succeeds, and returns a secret that reflects the secret in the store // exactly, but without the private data ---- @@ -135,7 +136,7 @@ func TestCreateSecret(t *testing.T) { // ---- creating a secret with the same name, even if it's the exact same spec, fails due to a name conflict ---- _, err = ts.Client.CreateSecret(context.Background(), &validSpecRequest) assert.Error(t, err) - assert.Equal(t, codes.AlreadyExists, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.AlreadyExists, grpc.Code(err), testutils.ErrorDesc(err)) } func TestGetSecret(t *testing.T) { @@ -145,12 +146,12 @@ func TestGetSecret(t *testing.T) { // ---- getting a secret without providing an ID results in an InvalidArgument ---- _, err := ts.Client.GetSecret(context.Background(), &api.GetSecretRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // ---- getting a non-existent secret fails with NotFound ---- _, err = ts.Client.GetSecret(context.Background(), &api.GetSecretRequest{SecretID: "12345"}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) // ---- getting an existing secret returns the secret with all the private data cleaned ---- secret := secretFromSecretSpec(createSecretSpec("name", []byte("data"), nil)) @@ -184,12 +185,12 @@ func TestUpdateSecret(t *testing.T) { // updating a secret without providing an ID results in an InvalidArgument _, err = ts.Client.UpdateSecret(context.Background(), &api.UpdateSecretRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // getting a non-existent secret fails with NotFound _, err = ts.Client.UpdateSecret(context.Background(), &api.UpdateSecretRequest{SecretID: "1234adsaa", SecretVersion: &api.Version{Index: 1}}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) // updating an existing secret's data returns an error secret.Spec.Data = []byte{1} @@ -198,7 +199,7 @@ func TestUpdateSecret(t *testing.T) { Spec: &secret.Spec, SecretVersion: &secret.Meta.Version, }) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // updating an existing secret's Name returns an error secret.Spec.Data = nil @@ -208,7 +209,7 @@ func TestUpdateSecret(t *testing.T) { Spec: &secret.Spec, SecretVersion: &secret.Meta.Version, }) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // updating the secret with the original spec succeeds secret.Spec.Data = []byte("data") @@ -260,7 +261,7 @@ func TestRemoveUnusedSecret(t *testing.T) { // removing a secret without providing an ID results in an InvalidArgument _, err := ts.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) // removing a secret that exists succeeds secret := secretFromSecretSpec(createSecretSpec("name", []byte("data"), nil)) @@ -276,7 +277,7 @@ func TestRemoveUnusedSecret(t *testing.T) { // ---- it was really removed because attempting to remove it again fails with a NotFound ---- _, err = ts.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{SecretID: secret.ID}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) } @@ -317,8 +318,8 @@ func TestRemoveUsedSecret(t *testing.T) { // removing a secret that exists but is in use fails _, err = ts.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{SecretID: resp.Secret.ID}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) - assert.Regexp(t, "service[1-2], service[1-2]", grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Regexp(t, "service[1-2], service[1-2]", testutils.ErrorDesc(err)) // removing a secret that exists but is not in use succeeds _, err = ts.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{SecretID: resp2.Secret.ID}) @@ -327,7 +328,7 @@ func TestRemoveUsedSecret(t *testing.T) { // it was really removed because attempting to remove it again fails with a NotFound _, err = ts.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{SecretID: resp2.Secret.ID}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) } func TestListSecrets(t *testing.T) { diff --git a/manager/controlapi/service_test.go b/manager/controlapi/service_test.go index 7ce97cd982..be943744a4 100644 --- a/manager/controlapi/service_test.go +++ b/manager/controlapi/service_test.go @@ -8,6 +8,7 @@ import ( "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/manager/state/store" + "github.com/docker/swarmkit/testutils" gogotypes "github.com/gogo/protobuf/types" "github.com/stretchr/testify/assert" "golang.org/x/net/context" @@ -361,7 +362,7 @@ func TestValidateContainerSpec(t *testing.T) { } { err := validateContainerSpec(bad.spec) assert.Error(t, err) - assert.Equal(t, bad.c, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, bad.c, grpc.Code(err), testutils.ErrorDesc(err)) } good1 := api.TaskSpec{ @@ -431,7 +432,7 @@ func TestValidateServiceSpec(t *testing.T) { } { err := validateServiceSpec(bad.spec) assert.Error(t, err) - assert.Equal(t, bad.c, grpc.Code(err), grpc.ErrorDesc(err)) + assert.Equal(t, bad.c, grpc.Code(err), testutils.ErrorDesc(err)) } for _, good := range []*api.ServiceSpec{ diff --git a/manager/dispatcher/dispatcher_test.go b/manager/dispatcher/dispatcher_test.go index 09dcf0d684..02b1bd65f6 100644 --- a/manager/dispatcher/dispatcher_test.go +++ b/manager/dispatcher/dispatcher_test.go @@ -384,7 +384,7 @@ func TestHeartbeatTimeout(t *testing.T) { resp, err := gd.Clients[0].Heartbeat(context.Background(), &api.HeartbeatRequest{SessionID: expectedSessionID}) assert.Nil(t, resp) assert.Error(t, err) - assert.Equal(t, grpc.ErrorDesc(err), ErrNodeNotRegistered.Error()) + assert.Equal(t, testutils.ErrorDesc(err), ErrNodeNotRegistered.Error()) } func TestHeartbeatUnregistered(t *testing.T) { @@ -394,7 +394,7 @@ func TestHeartbeatUnregistered(t *testing.T) { resp, err := gd.Clients[0].Heartbeat(context.Background(), &api.HeartbeatRequest{}) assert.Nil(t, resp) assert.Error(t, err) - assert.Equal(t, ErrSessionInvalid.Error(), grpc.ErrorDesc(err)) + assert.Equal(t, ErrSessionInvalid.Error(), testutils.ErrorDesc(err)) } // If the session ID is not sent as part of the Assignments request, an error is returned to the stream diff --git a/manager/manager_test.go b/manager/manager_test.go index f55b7f38a8..cf306cabe0 100644 --- a/manager/manager_test.go +++ b/manager/manager_test.go @@ -88,7 +88,7 @@ func TestManager(t *testing.T) { client := api.NewDispatcherClient(conn) require.NoError(t, testutils.PollFuncWithTimeout(nil, func() error { _, err = client.Heartbeat(tc.Context, &api.HeartbeatRequest{}) - if dispatcher.ErrNodeNotRegistered.Error() != grpc.ErrorDesc(err) { + if dispatcher.ErrNodeNotRegistered.Error() != testutils.ErrorDesc(err) { return err } _, err = client.Session(tc.Context, &api.SessionRequest{}) @@ -109,7 +109,7 @@ func TestManager(t *testing.T) { client = api.NewDispatcherClient(conn2) _, err = client.Heartbeat(context.Background(), &api.HeartbeatRequest{}) - require.Contains(t, grpc.ErrorDesc(err), "Permission denied: unauthorized peer role: rpc error: code = PermissionDenied desc = Permission denied: remote certificate not part of organization") + require.Contains(t, testutils.ErrorDesc(err), "Permission denied: unauthorized peer role: rpc error: code = PermissionDenied desc = Permission denied: remote certificate not part of organization") // Verify that requests to the various GRPC services running on TCP // are rejected if they don't have certs. @@ -210,7 +210,7 @@ func TestManager(t *testing.T) { client = api.NewDispatcherClient(conn) _, err = client.Heartbeat(context.Background(), &api.HeartbeatRequest{}) - require.Contains(t, grpc.ErrorDesc(err), "removed from swarm") + require.Contains(t, testutils.ErrorDesc(err), "removed from swarm") m.Stop(tc.Context, false) @@ -313,7 +313,7 @@ func TestManagerLockUnlock(t *testing.T) { ClusterVersion: &cluster.Meta.Version, Spec: spec, }) - if grpc.ErrorDesc(err) == "update out of sequence" { + if testutils.ErrorDesc(err) == "update out of sequence" { continue } // if there is any other type of error, this should fail @@ -386,7 +386,7 @@ func TestManagerLockUnlock(t *testing.T) { ClusterVersion: &cluster.Meta.Version, Spec: spec, }) - if grpc.ErrorDesc(err) == "update out of sequence" { + if testutils.ErrorDesc(err) == "update out of sequence" { continue } require.NoError(t, err) diff --git a/manager/state/raft/raft_test.go b/manager/state/raft/raft_test.go index 3b3172e3dd..340ff5d0fb 100644 --- a/manager/state/raft/raft_test.go +++ b/manager/state/raft/raft_test.go @@ -827,7 +827,7 @@ func TestRaftJoinWithIncorrectAddress(t *testing.T) { err := n.JoinAndStart(context.Background()) assert.NotNil(t, err) - assert.Contains(t, grpc.ErrorDesc(err), "could not connect to prospective new cluster member using its advertised address") + assert.Contains(t, testutils.ErrorDesc(err), "could not connect to prospective new cluster member using its advertised address") // Check if first node still has only itself registered in the memberlist assert.Len(t, nodes[1].GetMemberlist(), 1) diff --git a/manager/state/raft/transport/peer.go b/manager/state/raft/transport/peer.go index bdd3ec0293..9d72d5991f 100644 --- a/manager/state/raft/transport/peer.go +++ b/manager/state/raft/transport/peer.go @@ -15,6 +15,7 @@ import ( "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/log" "github.com/docker/swarmkit/manager/state/raft/membership" + "github.com/docker/swarmkit/testutils" "github.com/pkg/errors" ) @@ -244,7 +245,7 @@ func (p *peer) sendProcessMessage(ctx context.Context, m raftpb.Message) error { } // Handle errors. - if grpc.Code(err) == codes.NotFound && grpc.ErrorDesc(err) == membership.ErrMemberRemoved.Error() { + if grpc.Code(err) == codes.NotFound && testutils.ErrorDesc(err) == membership.ErrMemberRemoved.Error() { p.tr.config.NodeRemoved() } if m.Type == raftpb.MsgSnap { diff --git a/testutils/grpc.go b/testutils/grpc.go new file mode 100644 index 0000000000..d6d47d83be --- /dev/null +++ b/testutils/grpc.go @@ -0,0 +1,12 @@ +package testutils + +import "google.golang.org/grpc/status" + +// ErrorDesc returns the error description of err if it was produced by the rpc system. +// Otherwise, it returns err.Error() or empty string when err is nil. +func ErrorDesc(err error) string { + if s, ok := status.FromError(err); ok { + return s.Message() + } + return err.Error() +} From 42a2bfeb93080093cb5cd664da7fafd48583c26f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 23 May 2018 21:18:12 +0200 Subject: [PATCH 2/4] Remove use of deprecated grpc.Code(err) Signed-off-by: Sebastiaan van Stijn (cherry picked from commit cb6dcf20536d8ebbd18c8f5c5c11e9c5074c212a) Signed-off-by: Sebastiaan van Stijn --- agent/session.go | 17 +++-- ca/certificates.go | 7 +- ca/certificates_test.go | 4 +- ca/server_test.go | 5 +- cmd/swarmctl/main.go | 4 +- manager/controlapi/ca_rotation_test.go | 8 +-- manager/controlapi/cluster_test.go | 18 ++--- manager/controlapi/common_test.go | 4 +- manager/controlapi/config_test.go | 29 ++++---- manager/controlapi/network_test.go | 18 ++--- manager/controlapi/node_test.go | 19 +++-- manager/controlapi/secret_test.go | 31 ++++---- manager/controlapi/service_test.go | 71 +++++++++---------- manager/controlapi/task_test.go | 6 +- manager/dispatcher/dispatcher_test.go | 14 ++-- .../state/raft/transport/mock_raft_test.go | 2 +- manager/state/raft/transport/peer.go | 8 ++- .../plugin/raftproxy/test/raftproxy_test.go | 3 +- testutils/grpc.go | 14 +++- 19 files changed, 150 insertions(+), 132 deletions(-) diff --git a/agent/session.go b/agent/session.go index 9bb9773a6c..834889b294 100644 --- a/agent/session.go +++ b/agent/session.go @@ -12,6 +12,7 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) var ( @@ -188,7 +189,8 @@ func (s *session) heartbeat(ctx context.Context) error { cancel() if err != nil { log.G(ctx).WithFields(fields).WithError(err).Errorf("heartbeat to manager %v failed", s.conn.Peer()) - if grpc.Code(err) == codes.NotFound { + st, _ := status.FromError(err) + if st.Code() == codes.NotFound { err = errNodeNotRegistered } @@ -245,7 +247,8 @@ func (s *session) logSubscriptions(ctx context.Context) error { for { resp, err := subscriptions.Recv() - if grpc.Code(err) == codes.Unimplemented { + st, _ := status.FromError(err) + if st.Code() == codes.Unimplemented { log.Warning("manager does not support log subscriptions") // Don't return, because returning would bounce the session select { @@ -296,7 +299,8 @@ func (s *session) watch(ctx context.Context) error { // If we get a code = 12 desc = unknown method Assignments, try to use tasks resp, err = assignmentWatch.Recv() if err != nil { - if grpc.Code(err) != codes.Unimplemented { + st, _ := status.FromError(err) + if st.Code() != codes.Unimplemented { return err } tasksFallback = true @@ -355,20 +359,21 @@ func (s *session) watch(ctx context.Context) error { } // sendTaskStatus uses the current session to send the status of a single task. -func (s *session) sendTaskStatus(ctx context.Context, taskID string, status *api.TaskStatus) error { +func (s *session) sendTaskStatus(ctx context.Context, taskID string, taskStatus *api.TaskStatus) error { client := api.NewDispatcherClient(s.conn.ClientConn) if _, err := client.UpdateTaskStatus(ctx, &api.UpdateTaskStatusRequest{ SessionID: s.sessionID, Updates: []*api.UpdateTaskStatusRequest_TaskStatusUpdate{ { TaskID: taskID, - Status: status, + Status: taskStatus, }, }, }); err != nil { // TODO(stevvooe): Dispatcher should not return this error. Status // reports for unknown tasks should be ignored. - if grpc.Code(err) == codes.NotFound { + st, _ := status.FromError(err) + if st.Code() == codes.NotFound { return errTaskUnknown } diff --git a/ca/certificates.go b/ca/certificates.go index f2d3dbac55..ad2be2c571 100644 --- a/ca/certificates.go +++ b/ca/certificates.go @@ -35,6 +35,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" + "google.golang.org/grpc/status" ) const ( @@ -352,7 +353,8 @@ func (rca *RootCA) getKEKUpdate(ctx context.Context, leafCert *x509.Certificate, defer cancel() response, err := client.GetUnlockKey(ctx, &api.GetUnlockKeyRequest{}) if err != nil { - if grpc.Code(err) == codes.Unimplemented { // if the server does not support keks, return as if no encryption key was specified + s, _ := status.FromError(err) + if s.Code() == codes.Unimplemented { // if the server does not support keks, return as if no encryption key was specified conn.Close(true) return &KEKData{}, nil } @@ -838,8 +840,9 @@ func GetRemoteSignedCertificate(ctx context.Context, csr []byte, rootCAPool *x50 stateCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() statusResponse, err := caClient.NodeCertificateStatus(stateCtx, statusRequest) + s, _ := status.FromError(err) switch { - case err != nil && grpc.Code(err) != codes.DeadlineExceeded: + case err != nil && s.Code() != codes.DeadlineExceeded: conn.Close(false) // Because IssueNodeCertificate succeeded, if this call failed likely it is due to an issue with this // particular connection, so we need to get another. We should try a remote connection - the local node diff --git a/ca/certificates_test.go b/ca/certificates_test.go index 2c7895510c..55435889cd 100644 --- a/ca/certificates_test.go +++ b/ca/certificates_test.go @@ -39,6 +39,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/context" + "google.golang.org/grpc/status" ) func init() { @@ -760,7 +761,8 @@ func TestGetRemoteSignedCertificateWithPending(t *testing.T) { // error - it should have returned after 1 second, but add some more for rudge time. select { case err = <-completed: - require.Equal(t, grpc.Code(err), codes.DeadlineExceeded) + s, _ := status.FromError(err) + require.Equal(t, s.Code(), codes.DeadlineExceeded) case <-time.After(3 * time.Second): require.FailNow(t, "GetRemoteSignedCertificate should have been canceled after 1 second, and it has been 3") } diff --git a/ca/server_test.go b/ca/server_test.go index 996051960d..d70b1cd0f3 100644 --- a/ca/server_test.go +++ b/ca/server_test.go @@ -26,7 +26,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/context" - "google.golang.org/grpc" "google.golang.org/grpc/codes" ) @@ -48,7 +47,7 @@ func TestRestartRootCA(t *testing.T) { _, err := tc.NodeCAClients[0].NodeCertificateStatus(tc.Context, &api.NodeCertificateStatusRequest{NodeID: "foo"}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err)) tc.CAServer.Stop() go tc.CAServer.Run(tc.Context) @@ -57,7 +56,7 @@ func TestRestartRootCA(t *testing.T) { _, err = tc.NodeCAClients[0].NodeCertificateStatus(tc.Context, &api.NodeCertificateStatusRequest{NodeID: "foo"}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err)) } func TestIssueNodeCertificate(t *testing.T) { diff --git a/cmd/swarmctl/main.go b/cmd/swarmctl/main.go index 250e8faa05..692ac4ccea 100644 --- a/cmd/swarmctl/main.go +++ b/cmd/swarmctl/main.go @@ -13,8 +13,6 @@ import ( "github.com/docker/swarmkit/cmd/swarmd/defaults" "github.com/docker/swarmkit/version" "github.com/spf13/cobra" - "google.golang.org/grpc" - "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -23,7 +21,7 @@ func main() { s, _ := status.FromError(err) c.Println("Error:", s.Message()) // if it's not a grpc, we assume it's a user error and we display the usage. - if grpc.Code(err) == codes.Unknown { + if _, ok := status.FromError(err); !ok { c.Println(c.UsageString()) } diff --git a/manager/controlapi/ca_rotation_test.go b/manager/controlapi/ca_rotation_test.go index 0a80fc4584..56d861a35c 100644 --- a/manager/controlapi/ca_rotation_test.go +++ b/manager/controlapi/ca_rotation_test.go @@ -14,10 +14,9 @@ import ( "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/ca" "github.com/docker/swarmkit/ca/testutils" - grpcutils "github.com/docker/swarmkit/testutils" "github.com/stretchr/testify/require" - "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) type rootCARotationTestCase struct { @@ -309,8 +308,9 @@ func TestValidateCAConfigInvalidValues(t *testing.T) { secConfig := getSecurityConfig(t, &localRootCA, cluster) _, err := validateCAConfig(context.Background(), secConfig, cluster) require.Error(t, err, invalid.expectErrorString) - require.Equal(t, codes.InvalidArgument, grpc.Code(err), invalid.expectErrorString) - require.Contains(t, grpcutils.ErrorDesc(err), invalid.expectErrorString) + s, _ := status.FromError(err) + require.Equal(t, codes.InvalidArgument, s.Code(), invalid.expectErrorString) + require.Contains(t, s.Message(), invalid.expectErrorString) } } diff --git a/manager/controlapi/cluster_test.go b/manager/controlapi/cluster_test.go index ad05a13596..44ae9cfc2e 100644 --- a/manager/controlapi/cluster_test.go +++ b/manager/controlapi/cluster_test.go @@ -10,11 +10,11 @@ import ( "github.com/docker/swarmkit/ca/testutils" "github.com/docker/swarmkit/manager/state/store" "github.com/docker/swarmkit/protobuf/ptypes" + grpcutils "github.com/docker/swarmkit/testutils" gogotypes "github.com/gogo/protobuf/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/context" - "google.golang.org/grpc" "google.golang.org/grpc/codes" ) @@ -113,7 +113,7 @@ func TestValidateClusterSpec(t *testing.T) { } { err := validateClusterSpec(bad.spec) assert.Error(t, err) - assert.Equal(t, bad.c, grpc.Code(err)) + assert.Equal(t, bad.c, grpcutils.ErrorCode(err)) } for _, good := range []*api.ClusterSpec{ @@ -130,11 +130,11 @@ func TestGetCluster(t *testing.T) { defer ts.Stop() _, err := ts.Client.GetCluster(context.Background(), &api.GetClusterRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, grpcutils.ErrorCode(err)) _, err = ts.Client.GetCluster(context.Background(), &api.GetClusterRequest{ClusterID: "invalid"}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err)) + assert.Equal(t, codes.NotFound, grpcutils.ErrorCode(err)) cluster := createCluster(t, ts, "name", "name", api.AcceptancePolicy{}, ts.Server.securityConfig.RootCA()) r, err := ts.Client.GetCluster(context.Background(), &api.GetClusterRequest{ClusterID: cluster.ID}) @@ -156,11 +156,11 @@ func TestGetClusterWithSecret(t *testing.T) { defer ts.Stop() _, err := ts.Client.GetCluster(context.Background(), &api.GetClusterRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, grpcutils.ErrorCode(err)) _, err = ts.Client.GetCluster(context.Background(), &api.GetClusterRequest{ClusterID: "invalid"}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err)) + assert.Equal(t, codes.NotFound, grpcutils.ErrorCode(err)) policy := api.AcceptancePolicy{Policies: []*api.AcceptancePolicy_RoleAdmissionPolicy{{Secret: &api.AcceptancePolicy_RoleAdmissionPolicy_Secret{Data: []byte("secret")}}}} cluster := createCluster(t, ts, "name", "name", policy, ts.Server.securityConfig.RootCA()) @@ -180,16 +180,16 @@ func TestUpdateCluster(t *testing.T) { _, err := ts.Client.UpdateCluster(context.Background(), &api.UpdateClusterRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, grpcutils.ErrorCode(err)) _, err = ts.Client.UpdateCluster(context.Background(), &api.UpdateClusterRequest{ClusterID: "invalid", Spec: &cluster.Spec, ClusterVersion: &api.Version{}}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err)) + assert.Equal(t, codes.NotFound, grpcutils.ErrorCode(err)) // No update options. _, err = ts.Client.UpdateCluster(context.Background(), &api.UpdateClusterRequest{ClusterID: cluster.ID, Spec: &cluster.Spec}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, grpcutils.ErrorCode(err)) _, err = ts.Client.UpdateCluster(context.Background(), &api.UpdateClusterRequest{ClusterID: cluster.ID, Spec: &cluster.Spec, ClusterVersion: &cluster.Meta.Version}) assert.NoError(t, err) diff --git a/manager/controlapi/common_test.go b/manager/controlapi/common_test.go index 5cbbf17fbe..af36a2fd05 100644 --- a/manager/controlapi/common_test.go +++ b/manager/controlapi/common_test.go @@ -4,15 +4,15 @@ import ( "testing" "github.com/docker/swarmkit/api" + "github.com/docker/swarmkit/testutils" "github.com/stretchr/testify/assert" - "google.golang.org/grpc" "google.golang.org/grpc/codes" ) func TestValidateAnnotations(t *testing.T) { err := validateAnnotations(api.Annotations{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) for _, good := range []api.Annotations{ {Name: "name"}, diff --git a/manager/controlapi/config_test.go b/manager/controlapi/config_test.go index f231f619ee..e794a3be0f 100644 --- a/manager/controlapi/config_test.go +++ b/manager/controlapi/config_test.go @@ -10,7 +10,6 @@ import ( "github.com/docker/swarmkit/testutils" "github.com/stretchr/testify/assert" "golang.org/x/net/context" - "google.golang.org/grpc" "google.golang.org/grpc/codes" ) @@ -55,7 +54,7 @@ func TestValidateConfigSpec(t *testing.T) { } { err := validateConfigSpec(createConfigSpec(badName, []byte("valid config"), nil)) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } for _, badSpec := range []*api.ConfigSpec{ @@ -64,7 +63,7 @@ func TestValidateConfigSpec(t *testing.T) { } { err := validateConfigSpec(badSpec) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } for _, goodName := range []string{ @@ -99,7 +98,7 @@ func TestCreateConfig(t *testing.T) { // ---- creating a config with an invalid spec fails, thus checking that CreateConfig validates the spec ---- _, err := ts.Client.CreateConfig(context.Background(), &api.CreateConfigRequest{Spec: createConfigSpec("", nil, nil)}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // ---- creating a config with a valid spec succeeds, and returns a config that reflects the config in the store // exactly @@ -124,7 +123,7 @@ func TestCreateConfig(t *testing.T) { // ---- creating a config with the same name, even if it's the exact same spec, fails due to a name conflict ---- _, err = ts.Client.CreateConfig(context.Background(), &validSpecRequest) assert.Error(t, err) - assert.Equal(t, codes.AlreadyExists, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.AlreadyExists, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } func TestGetConfig(t *testing.T) { @@ -134,12 +133,12 @@ func TestGetConfig(t *testing.T) { // ---- getting a config without providing an ID results in an InvalidArgument ---- _, err := ts.Client.GetConfig(context.Background(), &api.GetConfigRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // ---- getting a non-existent config fails with NotFound ---- _, err = ts.Client.GetConfig(context.Background(), &api.GetConfigRequest{ConfigID: "12345"}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // ---- getting an existing config returns the config ---- config := configFromConfigSpec(createConfigSpec("name", []byte("data"), nil)) @@ -169,12 +168,12 @@ func TestUpdateConfig(t *testing.T) { // updating a config without providing an ID results in an InvalidArgument _, err = ts.Client.UpdateConfig(context.Background(), &api.UpdateConfigRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // getting a non-existent config fails with NotFound _, err = ts.Client.UpdateConfig(context.Background(), &api.UpdateConfigRequest{ConfigID: "1234adsaa", ConfigVersion: &api.Version{Index: 1}}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // updating an existing config's data returns an error config.Spec.Data = []byte{1} @@ -183,7 +182,7 @@ func TestUpdateConfig(t *testing.T) { Spec: &config.Spec, ConfigVersion: &config.Meta.Version, }) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // updating an existing config's Name returns an error config.Spec.Data = nil @@ -193,7 +192,7 @@ func TestUpdateConfig(t *testing.T) { Spec: &config.Spec, ConfigVersion: &config.Meta.Version, }) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // updating the config with the original spec succeeds config.Spec.Data = []byte("data") @@ -245,7 +244,7 @@ func TestRemoveUnusedConfig(t *testing.T) { // removing a config without providing an ID results in an InvalidArgument _, err := ts.Client.RemoveConfig(context.Background(), &api.RemoveConfigRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // removing a config that exists succeeds config := configFromConfigSpec(createConfigSpec("name", []byte("data"), nil)) @@ -261,7 +260,7 @@ func TestRemoveUnusedConfig(t *testing.T) { // ---- it was really removed because attempting to remove it again fails with a NotFound ---- _, err = ts.Client.RemoveConfig(context.Background(), &api.RemoveConfigRequest{ConfigID: config.ID}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } @@ -302,7 +301,7 @@ func TestRemoveUsedConfig(t *testing.T) { // removing a config that exists but is in use fails _, err = ts.Client.RemoveConfig(context.Background(), &api.RemoveConfigRequest{ConfigID: resp.Config.ID}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) assert.Regexp(t, "service[1-2], service[1-2]", testutils.ErrorDesc(err)) // removing a config that exists but is not in use succeeds @@ -312,7 +311,7 @@ func TestRemoveUsedConfig(t *testing.T) { // it was really removed because attempting to remove it again fails with a NotFound _, err = ts.Client.RemoveConfig(context.Background(), &api.RemoveConfigRequest{ConfigID: resp2.Config.ID}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } func TestListConfigs(t *testing.T) { diff --git a/manager/controlapi/network_test.go b/manager/controlapi/network_test.go index e719018acc..632ca59110 100644 --- a/manager/controlapi/network_test.go +++ b/manager/controlapi/network_test.go @@ -3,9 +3,9 @@ package controlapi import ( "testing" + "github.com/docker/swarmkit/testutils" "golang.org/x/net/context" - "google.golang.org/grpc" "google.golang.org/grpc/codes" "github.com/docker/swarmkit/api" @@ -89,13 +89,13 @@ func TestValidateDriver(t *testing.T) { err := validateDriver(&api.Driver{Name: ""}, nil, "") assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) } func TestValidateIPAMConfiguration(t *testing.T) { err := validateIPAMConfiguration(nil) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) IPAMConf := &api.IPAMConfig{ Subnet: "", @@ -103,12 +103,12 @@ func TestValidateIPAMConfiguration(t *testing.T) { err = validateIPAMConfiguration(IPAMConf) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) IPAMConf.Subnet = "bad" err = validateIPAMConfiguration(IPAMConf) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) IPAMConf.Subnet = "192.168.0.0/16" err = validateIPAMConfiguration(IPAMConf) @@ -117,12 +117,12 @@ func TestValidateIPAMConfiguration(t *testing.T) { IPAMConf.Range = "bad" err = validateIPAMConfiguration(IPAMConf) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) IPAMConf.Range = "192.169.1.0/24" err = validateIPAMConfiguration(IPAMConf) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) IPAMConf.Range = "192.168.1.0/24" err = validateIPAMConfiguration(IPAMConf) @@ -131,12 +131,12 @@ func TestValidateIPAMConfiguration(t *testing.T) { IPAMConf.Gateway = "bad" err = validateIPAMConfiguration(IPAMConf) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) IPAMConf.Gateway = "192.169.1.1" err = validateIPAMConfiguration(IPAMConf) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) IPAMConf.Gateway = "192.168.1.1" err = validateIPAMConfiguration(IPAMConf) diff --git a/manager/controlapi/node_test.go b/manager/controlapi/node_test.go index 603b72b894..b5e9e99164 100644 --- a/manager/controlapi/node_test.go +++ b/manager/controlapi/node_test.go @@ -16,7 +16,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/context" - "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/grpclog" ) @@ -45,11 +44,11 @@ func TestGetNode(t *testing.T) { _, err := ts.Client.GetNode(context.Background(), &api.GetNodeRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) _, err = ts.Client.GetNode(context.Background(), &api.GetNodeRequest{NodeID: "invalid"}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err)) node := createNode(t, ts, "id", api.NodeRoleManager, api.NodeMembershipAccepted, api.NodeStatus_READY) r, err := ts.Client.GetNode(context.Background(), &api.GetNodeRequest{NodeID: node.ID}) @@ -466,7 +465,7 @@ func TestUpdateNode(t *testing.T) { NodeVersion: &api.Version{}, }) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err)) // Create a node object for the manager assert.NoError(t, nodes[1].MemoryStore().Update(func(tx store.Tx) error { @@ -482,11 +481,11 @@ func TestUpdateNode(t *testing.T) { _, err = ts.Client.UpdateNode(context.Background(), &api.UpdateNodeRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) _, err = ts.Client.UpdateNode(context.Background(), &api.UpdateNodeRequest{NodeID: "invalid", Spec: &api.NodeSpec{}, NodeVersion: &api.Version{}}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err)) r, err := ts.Client.GetNode(context.Background(), &api.GetNodeRequest{NodeID: nodeID}) assert.NoError(t, err) @@ -497,7 +496,7 @@ func TestUpdateNode(t *testing.T) { _, err = ts.Client.UpdateNode(context.Background(), &api.UpdateNodeRequest{NodeID: nodeID}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) spec := r.Node.Spec.Copy() spec.Availability = api.NodeAvailabilityDrain @@ -506,7 +505,7 @@ func TestUpdateNode(t *testing.T) { Spec: spec, }) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) _, err = ts.Client.UpdateNode(context.Background(), &api.UpdateNodeRequest{ NodeID: nodeID, @@ -603,7 +602,7 @@ func testUpdateNodeDemote(t *testing.T) { NodeVersion: version, }) assert.Error(t, err) - assert.Equal(t, codes.FailedPrecondition, grpc.Code(err)) + assert.Equal(t, codes.FailedPrecondition, testutils.ErrorCode(err)) // Restart Node 3 nodes[3] = raftutils.RestartNode(t, clockSource, nodes[3], false) @@ -706,7 +705,7 @@ func testUpdateNodeDemote(t *testing.T) { NodeVersion: version, }) assert.Error(t, err) - assert.Equal(t, codes.FailedPrecondition, grpc.Code(err)) + assert.Equal(t, codes.FailedPrecondition, testutils.ErrorCode(err)) // Propose a change in the spec and check if the remaining node can still process updates r, err = ts.Client.GetNode(context.Background(), &api.GetNodeRequest{NodeID: lastNode.SecurityConfig.ClientTLSCreds.NodeID()}) diff --git a/manager/controlapi/secret_test.go b/manager/controlapi/secret_test.go index 8a7c693ffb..25dab57856 100644 --- a/manager/controlapi/secret_test.go +++ b/manager/controlapi/secret_test.go @@ -10,7 +10,6 @@ import ( "github.com/docker/swarmkit/testutils" "github.com/stretchr/testify/assert" "golang.org/x/net/context" - "google.golang.org/grpc" "google.golang.org/grpc/codes" ) @@ -55,7 +54,7 @@ func TestValidateSecretSpec(t *testing.T) { } { err := validateSecretSpec(createSecretSpec(badName, []byte("valid secret"), nil)) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } for _, badSpec := range []*api.SecretSpec{ @@ -64,7 +63,7 @@ func TestValidateSecretSpec(t *testing.T) { } { err := validateSecretSpec(badSpec) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } for _, goodName := range []string{ @@ -96,7 +95,7 @@ func TestValidateSecretSpec(t *testing.T) { spec.Driver = &api.Driver{} err := validateSecretSpec(spec) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) spec.Driver.Name = "secret-driver" err = validateSecretSpec(spec) assert.NoError(t, err) @@ -109,7 +108,7 @@ func TestCreateSecret(t *testing.T) { // ---- creating a secret with an invalid spec fails, thus checking that CreateSecret validates the spec ---- _, err := ts.Client.CreateSecret(context.Background(), &api.CreateSecretRequest{Spec: createSecretSpec("", nil, nil)}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // ---- creating a secret with a valid spec succeeds, and returns a secret that reflects the secret in the store // exactly, but without the private data ---- @@ -136,7 +135,7 @@ func TestCreateSecret(t *testing.T) { // ---- creating a secret with the same name, even if it's the exact same spec, fails due to a name conflict ---- _, err = ts.Client.CreateSecret(context.Background(), &validSpecRequest) assert.Error(t, err) - assert.Equal(t, codes.AlreadyExists, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.AlreadyExists, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } func TestGetSecret(t *testing.T) { @@ -146,12 +145,12 @@ func TestGetSecret(t *testing.T) { // ---- getting a secret without providing an ID results in an InvalidArgument ---- _, err := ts.Client.GetSecret(context.Background(), &api.GetSecretRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // ---- getting a non-existent secret fails with NotFound ---- _, err = ts.Client.GetSecret(context.Background(), &api.GetSecretRequest{SecretID: "12345"}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // ---- getting an existing secret returns the secret with all the private data cleaned ---- secret := secretFromSecretSpec(createSecretSpec("name", []byte("data"), nil)) @@ -185,12 +184,12 @@ func TestUpdateSecret(t *testing.T) { // updating a secret without providing an ID results in an InvalidArgument _, err = ts.Client.UpdateSecret(context.Background(), &api.UpdateSecretRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // getting a non-existent secret fails with NotFound _, err = ts.Client.UpdateSecret(context.Background(), &api.UpdateSecretRequest{SecretID: "1234adsaa", SecretVersion: &api.Version{Index: 1}}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // updating an existing secret's data returns an error secret.Spec.Data = []byte{1} @@ -199,7 +198,7 @@ func TestUpdateSecret(t *testing.T) { Spec: &secret.Spec, SecretVersion: &secret.Meta.Version, }) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // updating an existing secret's Name returns an error secret.Spec.Data = nil @@ -209,7 +208,7 @@ func TestUpdateSecret(t *testing.T) { Spec: &secret.Spec, SecretVersion: &secret.Meta.Version, }) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // updating the secret with the original spec succeeds secret.Spec.Data = []byte("data") @@ -261,7 +260,7 @@ func TestRemoveUnusedSecret(t *testing.T) { // removing a secret without providing an ID results in an InvalidArgument _, err := ts.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // removing a secret that exists succeeds secret := secretFromSecretSpec(createSecretSpec("name", []byte("data"), nil)) @@ -277,7 +276,7 @@ func TestRemoveUnusedSecret(t *testing.T) { // ---- it was really removed because attempting to remove it again fails with a NotFound ---- _, err = ts.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{SecretID: secret.ID}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } @@ -318,7 +317,7 @@ func TestRemoveUsedSecret(t *testing.T) { // removing a secret that exists but is in use fails _, err = ts.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{SecretID: resp.Secret.ID}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) assert.Regexp(t, "service[1-2], service[1-2]", testutils.ErrorDesc(err)) // removing a secret that exists but is not in use succeeds @@ -328,7 +327,7 @@ func TestRemoveUsedSecret(t *testing.T) { // it was really removed because attempting to remove it again fails with a NotFound _, err = ts.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{SecretID: resp2.Secret.ID}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } func TestListSecrets(t *testing.T) { diff --git a/manager/controlapi/service_test.go b/manager/controlapi/service_test.go index be943744a4..1d8687d2e2 100644 --- a/manager/controlapi/service_test.go +++ b/manager/controlapi/service_test.go @@ -12,7 +12,6 @@ import ( gogotypes "github.com/gogo/protobuf/types" "github.com/stretchr/testify/assert" "golang.org/x/net/context" - "google.golang.org/grpc" "google.golang.org/grpc/codes" ) @@ -182,7 +181,7 @@ func TestValidateResources(t *testing.T) { for _, b := range bad { err := validateResources(b) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) } for _, g := range good { @@ -202,7 +201,7 @@ func TestValidateResourceRequirements(t *testing.T) { for _, b := range bad { err := validateResourceRequirements(b) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) } for _, g := range good { @@ -226,7 +225,7 @@ func TestValidateMode(t *testing.T) { for _, b := range bad { err := validateMode(b) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) } for _, g := range good { @@ -284,7 +283,7 @@ func TestValidateTaskSpec(t *testing.T) { } { err := validateTaskSpec(bad.s) assert.Error(t, err) - assert.Equal(t, bad.c, grpc.Code(err)) + assert.Equal(t, bad.c, testutils.ErrorCode(err)) } for _, good := range []api.TaskSpec{ @@ -362,7 +361,7 @@ func TestValidateContainerSpec(t *testing.T) { } { err := validateContainerSpec(bad.spec) assert.Error(t, err) - assert.Equal(t, bad.c, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, bad.c, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } good1 := api.TaskSpec{ @@ -432,7 +431,7 @@ func TestValidateServiceSpec(t *testing.T) { } { err := validateServiceSpec(bad.spec) assert.Error(t, err) - assert.Equal(t, bad.c, grpc.Code(err), testutils.ErrorDesc(err)) + assert.Equal(t, bad.c, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } for _, good := range []*api.ServiceSpec{ @@ -465,7 +464,7 @@ func TestValidateRestartPolicy(t *testing.T) { for _, b := range bad { err := validateRestartPolicy(b) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) } for _, g := range good { @@ -492,7 +491,7 @@ func TestValidateUpdate(t *testing.T) { for _, b := range bad { err := validateUpdate(b) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) } for _, g := range good { @@ -505,7 +504,7 @@ func TestCreateService(t *testing.T) { defer ts.Stop() _, err := ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) spec := createSpec("name", "image", 1) r, err := ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec}) @@ -527,7 +526,7 @@ func TestCreateService(t *testing.T) { }} _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec2}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // test no port conflicts when no publish port is specified spec3 := createSpec("name4", "image", 1) @@ -591,7 +590,7 @@ func TestCreateService(t *testing.T) { }} _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec2}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // ensure port conflict when host ports overlaps with ingress port (ingress port first) spec = createSpec("name12", "image", 1) @@ -608,14 +607,14 @@ func TestCreateService(t *testing.T) { }} _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec2}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // ingress network cannot be attached explicitly spec = createSpec("name14", "image", 1) spec.Task.Networks = []*api.NetworkAttachmentConfig{{Target: getIngressTargetID(t, ts)}} _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) } func TestSecretValidation(t *testing.T) { @@ -628,7 +627,7 @@ func TestSecretValidation(t *testing.T) { secretRef.SecretName = "404" serviceSpec := createServiceSpecWithSecrets("service", secretRef) _, err := ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: serviceSpec}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // test creating service with a secretRef that has an existing secret // but mismatched SecretName fails. @@ -636,21 +635,21 @@ func TestSecretValidation(t *testing.T) { secretRef1.SecretName = "secret2" serviceSpec = createServiceSpecWithSecrets("service1", secretRef1) _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: serviceSpec}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // test secret target conflicts secretRef2 := createSecret(t, ts, "secret2", "secret2.txt") secretRef3 := createSecret(t, ts, "secret3", "secret2.txt") serviceSpec = createServiceSpecWithSecrets("service2", secretRef2, secretRef3) _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: serviceSpec}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // test secret target conflicts with same secret and two references secretRef3.SecretID = secretRef2.SecretID secretRef3.SecretName = secretRef2.SecretName serviceSpec = createServiceSpecWithSecrets("service3", secretRef2, secretRef3) _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: serviceSpec}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // test two different secretReferences with using the same secret secretRef5 := secretRef2.Copy() @@ -669,7 +668,7 @@ func TestSecretValidation(t *testing.T) { serviceSpec = createServiceSpecWithSecrets("invalid-blank", secretRefBlank) _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: serviceSpec}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // Test secret References with valid filenames // Note: "../secretfile.txt", "../../secretfile.txt" will be rejected @@ -698,7 +697,7 @@ func TestSecretValidation(t *testing.T) { Spec: serviceSpec1, ServiceVersion: &rs.Service.Meta.Version, }) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) } func TestConfigValidation(t *testing.T) { @@ -711,7 +710,7 @@ func TestConfigValidation(t *testing.T) { configRef.ConfigName = "404" serviceSpec := createServiceSpecWithConfigs("service", configRef) _, err := ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: serviceSpec}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // test creating service with a configRef that has an existing config // but mismatched ConfigName fails. @@ -719,21 +718,21 @@ func TestConfigValidation(t *testing.T) { configRef1.ConfigName = "config2" serviceSpec = createServiceSpecWithConfigs("service1", configRef1) _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: serviceSpec}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // test config target conflicts configRef2 := createConfig(t, ts, "config2", "config2.txt") configRef3 := createConfig(t, ts, "config3", "config2.txt") serviceSpec = createServiceSpecWithConfigs("service2", configRef2, configRef3) _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: serviceSpec}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // test config target conflicts with same config and two references configRef3.ConfigID = configRef2.ConfigID configRef3.ConfigName = configRef2.ConfigName serviceSpec = createServiceSpecWithConfigs("service3", configRef2, configRef3) _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: serviceSpec}) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) // test two different configReferences with using the same config configRef5 := configRef2.Copy() @@ -773,7 +772,7 @@ func TestConfigValidation(t *testing.T) { Spec: serviceSpec1, ServiceVersion: &rs.Service.Meta.Version, }) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) } func TestGetService(t *testing.T) { @@ -781,11 +780,11 @@ func TestGetService(t *testing.T) { defer ts.Stop() _, err := ts.Client.GetService(context.Background(), &api.GetServiceRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) _, err = ts.Client.GetService(context.Background(), &api.GetServiceRequest{ServiceID: "invalid"}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err)) service := createService(t, ts, "name", "image", 1) r, err := ts.Client.GetService(context.Background(), &api.GetServiceRequest{ServiceID: service.ID}) @@ -801,16 +800,16 @@ func TestUpdateService(t *testing.T) { _, err := ts.Client.UpdateService(context.Background(), &api.UpdateServiceRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) _, err = ts.Client.UpdateService(context.Background(), &api.UpdateServiceRequest{ServiceID: "invalid", Spec: &service.Spec, ServiceVersion: &api.Version{}}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err)) // No update options. _, err = ts.Client.UpdateService(context.Background(), &api.UpdateServiceRequest{ServiceID: service.ID, Spec: &service.Spec}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) _, err = ts.Client.UpdateService(context.Background(), &api.UpdateServiceRequest{ServiceID: service.ID, Spec: &service.Spec, ServiceVersion: &service.Meta.Version}) assert.NoError(t, err) @@ -892,7 +891,7 @@ func TestUpdateService(t *testing.T) { ServiceVersion: &rs.Service.Meta.Version, }) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) spec3.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{ {PublishedPort: uint32(9001), TargetPort: uint32(9000), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)}, }} @@ -914,7 +913,7 @@ func TestUpdateService(t *testing.T) { ServiceVersion: &rs.Service.Meta.Version, }) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) } func TestServiceUpdateRejectNetworkChange(t *testing.T) { @@ -996,7 +995,7 @@ func TestRemoveService(t *testing.T) { defer ts.Stop() _, err := ts.Client.RemoveService(context.Background(), &api.RemoveServiceRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) service := createService(t, ts, "name", "image", 1) r, err := ts.Client.RemoveService(context.Background(), &api.RemoveServiceRequest{ServiceID: service.ID}) @@ -1091,14 +1090,14 @@ func TestValidateEndpointSpec(t *testing.T) { err := validateEndpointSpec(endPointSpec1) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) err = validateEndpointSpec(endPointSpec2) assert.NoError(t, err) err = validateEndpointSpec(endPointSpec3) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) err = validateEndpointSpec(endPointSpec4) assert.NoError(t, err) @@ -1149,7 +1148,7 @@ func TestServiceEndpointSpecUpdate(t *testing.T) { _, err = ts.Client.UpdateService(context.Background(), &api.UpdateServiceRequest{Spec: spec}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) } func TestListServices(t *testing.T) { diff --git a/manager/controlapi/task_test.go b/manager/controlapi/task_test.go index 075428bb8b..faafd547fd 100644 --- a/manager/controlapi/task_test.go +++ b/manager/controlapi/task_test.go @@ -4,8 +4,8 @@ import ( "strings" "testing" + "github.com/docker/swarmkit/testutils" "golang.org/x/net/context" - "google.golang.org/grpc" "google.golang.org/grpc/codes" "github.com/docker/swarmkit/api" @@ -37,11 +37,11 @@ func TestGetTask(t *testing.T) { _, err := ts.Client.GetTask(context.Background(), &api.GetTaskRequest{}) assert.Error(t, err) - assert.Equal(t, codes.InvalidArgument, grpc.Code(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) _, err = ts.Client.GetTask(context.Background(), &api.GetTaskRequest{TaskID: "invalid"}) assert.Error(t, err) - assert.Equal(t, codes.NotFound, grpc.Code(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err)) task := createTask(t, ts, api.TaskStateRunning) r, err := ts.Client.GetTask(context.Background(), &api.GetTaskRequest{TaskID: task.ID}) diff --git a/manager/dispatcher/dispatcher_test.go b/manager/dispatcher/dispatcher_test.go index 02b1bd65f6..cf23464eee 100644 --- a/manager/dispatcher/dispatcher_test.go +++ b/manager/dispatcher/dispatcher_test.go @@ -267,7 +267,7 @@ func TestRegisterExceedRateLimit(t *testing.T) { assert.NoError(t, err) _, err = stream.Recv() assert.Error(t, err) - assert.Equal(t, codes.Unavailable, grpc.Code(err), err.Error()) + assert.Equal(t, codes.Unavailable, testutils.ErrorCode(err), err.Error()) } } @@ -311,7 +311,7 @@ func TestHeartbeat(t *testing.T) { resp, err := gd.Clients[0].Heartbeat(context.Background(), &api.HeartbeatRequest{}) assert.Nil(t, resp) assert.Error(t, err) - assert.Equal(t, grpc.Code(err), codes.InvalidArgument) + assert.Equal(t, testutils.ErrorCode(err), codes.InvalidArgument) } resp, err := gd.Clients[0].Heartbeat(context.Background(), &api.HeartbeatRequest{SessionID: expectedSessionID}) @@ -414,7 +414,7 @@ func TestAssignmentsErrorsIfNoSessionID(t *testing.T) { resp, err := stream.Recv() assert.Nil(t, resp) assert.Error(t, err) - assert.Equal(t, grpc.Code(err), codes.InvalidArgument) + assert.Equal(t, testutils.ErrorCode(err), codes.InvalidArgument) } func TestAssignmentsSecretDriver(t *testing.T) { @@ -1247,7 +1247,7 @@ func TestTaskUpdate(t *testing.T) { resp, err := gd.Clients[0].UpdateTaskStatus(context.Background(), updReq) assert.Nil(t, resp) assert.Error(t, err) - assert.Equal(t, grpc.Code(err), codes.InvalidArgument) + assert.Equal(t, testutils.ErrorCode(err), codes.InvalidArgument) } updReq.SessionID = expectedSessionID @@ -1266,7 +1266,7 @@ func TestTaskUpdate(t *testing.T) { resp, err := gd.Clients[0].UpdateTaskStatus(context.Background(), updReq) assert.Nil(t, resp) assert.Error(t, err) - assert.Equal(t, grpc.Code(err), codes.PermissionDenied) + assert.Equal(t, testutils.ErrorCode(err), codes.PermissionDenied) } gd.dispatcherServer.processUpdates(context.Background()) @@ -1754,7 +1754,7 @@ func TestOldTasks(t *testing.T) { resp, err := stream.Recv() assert.Nil(t, resp) assert.Error(t, err) - assert.Equal(t, grpc.Code(err), codes.InvalidArgument) + assert.Equal(t, testutils.ErrorCode(err), codes.InvalidArgument) } stream, err := gd.Clients[0].Tasks(context.Background(), &api.TasksRequest{SessionID: expectedSessionID}) @@ -1852,7 +1852,7 @@ func TestOldTasksStatusChange(t *testing.T) { resp, err := stream.Recv() assert.Nil(t, resp) assert.Error(t, err) - assert.Equal(t, grpc.Code(err), codes.InvalidArgument) + assert.Equal(t, testutils.ErrorCode(err), codes.InvalidArgument) } stream, err := gd.Clients[0].Tasks(context.Background(), &api.TasksRequest{SessionID: expectedSessionID}) diff --git a/manager/state/raft/transport/mock_raft_test.go b/manager/state/raft/transport/mock_raft_test.go index f6aa625acf..bf4cc10ef2 100644 --- a/manager/state/raft/transport/mock_raft_test.go +++ b/manager/state/raft/transport/mock_raft_test.go @@ -100,7 +100,7 @@ func (r *mockRaft) ProcessRaftMessage(ctx context.Context, req *api.ProcessRaftM // StreamRaftMessage is the mock server endpoint for streaming messages of type StreamRaftMessageRequest. func (r *mockRaft) StreamRaftMessage(stream api.Raft_StreamRaftMessageServer) error { if r.forceErrorStream { - return grpc.Errorf(codes.Unimplemented, "streaming not supported") + return status.Errorf(codes.Unimplemented, "streaming not supported") } var recvdMsg, assembledMessage *api.StreamRaftMessageRequest var err error diff --git a/manager/state/raft/transport/peer.go b/manager/state/raft/transport/peer.go index 9d72d5991f..644e295214 100644 --- a/manager/state/raft/transport/peer.go +++ b/manager/state/raft/transport/peer.go @@ -15,8 +15,8 @@ import ( "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/log" "github.com/docker/swarmkit/manager/state/raft/membership" - "github.com/docker/swarmkit/testutils" "github.com/pkg/errors" + "google.golang.org/grpc/status" ) const ( @@ -239,13 +239,15 @@ func (p *peer) sendProcessMessage(ctx context.Context, m raftpb.Message) error { } // Try doing a regular rpc if the receiver doesn't support streaming. - if grpc.Code(err) == codes.Unimplemented { + s, _ := status.FromError(err) + if s.Code() == codes.Unimplemented { log.G(ctx).Info("sending message to raft peer using ProcessRaftMessage()") _, err = api.NewRaftClient(p.conn()).ProcessRaftMessage(ctx, &api.ProcessRaftMessageRequest{Message: &m}) } // Handle errors. - if grpc.Code(err) == codes.NotFound && testutils.ErrorDesc(err) == membership.ErrMemberRemoved.Error() { + s, _ = status.FromError(err) + if s.Code() == codes.NotFound && s.Message() == membership.ErrMemberRemoved.Error() { p.tr.config.NodeRemoved() } if m.Type == raftpb.MsgSnap { diff --git a/protobuf/plugin/raftproxy/test/raftproxy_test.go b/protobuf/plugin/raftproxy/test/raftproxy_test.go index 3dd8990661..bdefb65311 100644 --- a/protobuf/plugin/raftproxy/test/raftproxy_test.go +++ b/protobuf/plugin/raftproxy/test/raftproxy_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/docker/swarmkit/testutils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -60,5 +61,5 @@ func TestSimpleRedirect(t *testing.T) { client := NewRouteGuideClient(conn) _, err = client.GetFeature(context.Background(), &Point{}) assert.NotNil(t, err) - assert.Equal(t, codes.ResourceExhausted, grpc.Code(err)) + assert.Equal(t, codes.ResourceExhausted, testutils.ErrorCode(err)) } diff --git a/testutils/grpc.go b/testutils/grpc.go index d6d47d83be..ddf9c95ccf 100644 --- a/testutils/grpc.go +++ b/testutils/grpc.go @@ -1,6 +1,9 @@ package testutils -import "google.golang.org/grpc/status" +import ( + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) // ErrorDesc returns the error description of err if it was produced by the rpc system. // Otherwise, it returns err.Error() or empty string when err is nil. @@ -10,3 +13,12 @@ func ErrorDesc(err error) string { } return err.Error() } + +// ErrorCode returns the error code for err if it was produced by the rpc system. +// Otherwise, it returns codes.Unknown. +func ErrorCode(err error) codes.Code { + if s, ok := status.FromError(err); ok { + return s.Code() + } + return codes.Unknown +} From e3b9bd5cdc866b3c95205e3c63f4f597cf27f3e9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 5 Nov 2018 22:13:05 +0100 Subject: [PATCH 3/4] Return correct error-codes on conflicting names Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 2061af766f61f6bc8d3e43d5553fba315f569597) Signed-off-by: Sebastiaan van Stijn --- manager/controlapi/service.go | 18 ++++++++++++------ manager/controlapi/service_test.go | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/manager/controlapi/service.go b/manager/controlapi/service.go index 3912052bf0..fce132b3ad 100644 --- a/manager/controlapi/service.go +++ b/manager/controlapi/service.go @@ -680,13 +680,14 @@ func (s *Server) CreateService(ctx context.Context, request *api.CreateServiceRe return store.CreateService(tx, service) }) - if err != nil { + switch err { + case store.ErrNameConflict: + return nil, status.Errorf(codes.AlreadyExists, "service %s already exists", request.Spec.Annotations.Name) + case nil: + return &api.CreateServiceResponse{Service: service}, nil + default: return nil, err } - - return &api.CreateServiceResponse{ - Service: service, - }, nil } // GetService returns a Service given a ServiceID. @@ -896,7 +897,12 @@ func (s *Server) ListServices(ctx context.Context, request *api.ListServicesRequ } }) if err != nil { - return nil, err + switch err { + case store.ErrInvalidFindBy: + return nil, status.Errorf(codes.InvalidArgument, err.Error()) + default: + return nil, err + } } if request.Filters != nil { diff --git a/manager/controlapi/service_test.go b/manager/controlapi/service_test.go index 1d8687d2e2..6b14172f39 100644 --- a/manager/controlapi/service_test.go +++ b/manager/controlapi/service_test.go @@ -615,6 +615,14 @@ func TestCreateService(t *testing.T) { _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec}) assert.Error(t, err) assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err)) + + spec = createSpec("notunique", "image", 1) + _, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec}) + assert.NoError(t, err) + + r, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec}) + assert.Error(t, err) + assert.Equal(t, codes.AlreadyExists, testutils.ErrorCode(err)) } func TestSecretValidation(t *testing.T) { @@ -870,6 +878,16 @@ func TestUpdateService(t *testing.T) { }) assert.Error(t, err) + // Attempt to update service name; renaming is not implemented + r.Service.Spec.Annotations.Name = "newname" + _, err = ts.Client.UpdateService(context.Background(), &api.UpdateServiceRequest{ + ServiceID: service.ID, + Spec: &r.Service.Spec, + ServiceVersion: version, + }) + assert.Error(t, err) + assert.Equal(t, codes.Unimplemented, testutils.ErrorCode(err)) + // test port conflicts spec2 := createSpec("name2", "image", 1) spec2.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{ From 651e0d8da68b671f84527300373444570625da69 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 28 Dec 2018 22:13:27 +0100 Subject: [PATCH 4/4] Include old error-message for backward compatibility Commit 2061af766f61f6bc8d3e43d5553fba315f569597 fixed the API returning incorrect status-codes, but also changed the error message for conflicting service-names to be in line with other objects (secrets, configs); "service XX already exists". Unfortunately, there are existing consumers of the API that perform string-matching, and changing the error-message resulted in a breaking change. This patch prepends the `ErrNameConflict` error-message to the error-message, so that those consumers still find the original message, but preserves the enhancement made in 2061af7 (inclusion of the conflicting service name). With this patch applied, the error message will look like this; name conflicts with an existing object: service myservice already exists Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 2bbdeec8998b3fd400ed8d892e87ed1d8935e105) Signed-off-by: Sebastiaan van Stijn --- manager/controlapi/service.go | 5 ++++- manager/controlapi/service_test.go | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/manager/controlapi/service.go b/manager/controlapi/service.go index fce132b3ad..c1f982683f 100644 --- a/manager/controlapi/service.go +++ b/manager/controlapi/service.go @@ -682,7 +682,10 @@ func (s *Server) CreateService(ctx context.Context, request *api.CreateServiceRe }) switch err { case store.ErrNameConflict: - return nil, status.Errorf(codes.AlreadyExists, "service %s already exists", request.Spec.Annotations.Name) + // Enhance the name-confict error to include the service name. The original + // `ErrNameConflict` error-message is included for backward-compatibility + // with older consumers of the API performing string-matching. + return nil, status.Errorf(codes.AlreadyExists, "%s: service %s already exists", err.Error(), request.Spec.Annotations.Name) case nil: return &api.CreateServiceResponse{Service: service}, nil default: diff --git a/manager/controlapi/service_test.go b/manager/controlapi/service_test.go index 6b14172f39..252b1b38bf 100644 --- a/manager/controlapi/service_test.go +++ b/manager/controlapi/service_test.go @@ -623,6 +623,10 @@ func TestCreateService(t *testing.T) { r, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec}) assert.Error(t, err) assert.Equal(t, codes.AlreadyExists, testutils.ErrorCode(err)) + + // Make sure the error contains "name conflicts with an existing object" for + // backward-compatibility with older clients doing string-matching... + assert.Contains(t, err.Error(), "name conflicts with an existing object") } func TestSecretValidation(t *testing.T) {