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 1a76d9c01c..692ac4ccea 100644 --- a/cmd/swarmctl/main.go +++ b/cmd/swarmctl/main.go @@ -13,15 +13,15 @@ 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" ) 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 { + if _, ok := status.FromError(err); !ok { 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..56d861a35c 100644 --- a/manager/controlapi/ca_rotation_test.go +++ b/manager/controlapi/ca_rotation_test.go @@ -3,21 +3,20 @@ 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" "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, grpc.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 3774527442..e794a3be0f 100644 --- a/manager/controlapi/config_test.go +++ b/manager/controlapi/config_test.go @@ -7,9 +7,9 @@ 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" "google.golang.org/grpc/codes" ) @@ -54,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), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } for _, badSpec := range []*api.ConfigSpec{ @@ -63,7 +63,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, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } for _, goodName := range []string{ @@ -98,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), grpc.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 @@ -123,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), grpc.ErrorDesc(err)) + assert.Equal(t, codes.AlreadyExists, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } func TestGetConfig(t *testing.T) { @@ -133,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), grpc.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), grpc.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)) @@ -168,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), grpc.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), grpc.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} @@ -182,7 +182,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, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // updating an existing config's Name returns an error config.Spec.Data = nil @@ -192,7 +192,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, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // updating the config with the original spec succeeds config.Spec.Data = []byte("data") @@ -244,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), grpc.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)) @@ -260,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), grpc.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } @@ -301,8 +301,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, 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 _, err = ts.Client.RemoveConfig(context.Background(), &api.RemoveConfigRequest{ConfigID: resp2.Config.ID}) @@ -311,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), grpc.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 cf66e39144..25dab57856 100644 --- a/manager/controlapi/secret_test.go +++ b/manager/controlapi/secret_test.go @@ -7,9 +7,9 @@ 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" "google.golang.org/grpc/codes" ) @@ -54,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), grpc.ErrorDesc(err)) + assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } for _, badSpec := range []*api.SecretSpec{ @@ -63,7 +63,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, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } for _, goodName := range []string{ @@ -95,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), grpc.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) @@ -108,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), grpc.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 ---- @@ -135,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), grpc.ErrorDesc(err)) + assert.Equal(t, codes.AlreadyExists, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } func TestGetSecret(t *testing.T) { @@ -145,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), grpc.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), grpc.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)) @@ -184,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), grpc.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), grpc.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} @@ -198,7 +198,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, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // updating an existing secret's Name returns an error secret.Spec.Data = nil @@ -208,7 +208,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, testutils.ErrorCode(err), testutils.ErrorDesc(err)) // updating the secret with the original spec succeeds secret.Spec.Data = []byte("data") @@ -260,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), grpc.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)) @@ -276,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), grpc.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } @@ -317,8 +317,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, 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 _, err = ts.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{SecretID: resp2.Secret.ID}) @@ -327,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), grpc.ErrorDesc(err)) + assert.Equal(t, codes.NotFound, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } func TestListSecrets(t *testing.T) { diff --git a/manager/controlapi/service.go b/manager/controlapi/service.go index 3912052bf0..c1f982683f 100644 --- a/manager/controlapi/service.go +++ b/manager/controlapi/service.go @@ -680,13 +680,17 @@ func (s *Server) CreateService(ctx context.Context, request *api.CreateServiceRe return store.CreateService(tx, service) }) - if err != nil { + switch err { + case store.ErrNameConflict: + // 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: return nil, err } - - return &api.CreateServiceResponse{ - Service: service, - }, nil } // GetService returns a Service given a ServiceID. @@ -896,7 +900,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 7ce97cd982..252b1b38bf 100644 --- a/manager/controlapi/service_test.go +++ b/manager/controlapi/service_test.go @@ -8,10 +8,10 @@ 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" - "google.golang.org/grpc" "google.golang.org/grpc/codes" ) @@ -181,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 { @@ -201,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 { @@ -225,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 { @@ -283,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{ @@ -361,7 +361,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, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } good1 := api.TaskSpec{ @@ -431,7 +431,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, testutils.ErrorCode(err), testutils.ErrorDesc(err)) } for _, good := range []*api.ServiceSpec{ @@ -464,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 { @@ -491,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 { @@ -504,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}) @@ -526,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) @@ -590,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) @@ -607,14 +607,26 @@ 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)) + + 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)) + + // 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) { @@ -627,7 +639,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. @@ -635,21 +647,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() @@ -668,7 +680,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 @@ -697,7 +709,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) { @@ -710,7 +722,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. @@ -718,21 +730,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() @@ -772,7 +784,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) { @@ -780,11 +792,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}) @@ -800,16 +812,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) @@ -870,6 +882,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{ @@ -891,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)) spec3.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{ {PublishedPort: uint32(9001), TargetPort: uint32(9000), Protocol: api.PortConfig_Protocol(api.ProtocolTCP)}, }} @@ -913,7 +935,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) { @@ -995,7 +1017,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}) @@ -1090,14 +1112,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) @@ -1148,7 +1170,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 09dcf0d684..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}) @@ -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 @@ -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/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/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 bdd3ec0293..644e295214 100644 --- a/manager/state/raft/transport/peer.go +++ b/manager/state/raft/transport/peer.go @@ -16,6 +16,7 @@ import ( "github.com/docker/swarmkit/log" "github.com/docker/swarmkit/manager/state/raft/membership" "github.com/pkg/errors" + "google.golang.org/grpc/status" ) const ( @@ -238,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 && grpc.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 new file mode 100644 index 0000000000..ddf9c95ccf --- /dev/null +++ b/testutils/grpc.go @@ -0,0 +1,24 @@ +package testutils + +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. +func ErrorDesc(err error) string { + if s, ok := status.FromError(err); ok { + return s.Message() + } + 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 +}