From ce88f8acb2d70c7da60ceb277930b474e0aecd0b Mon Sep 17 00:00:00 2001 From: Andrew Hsu Date: Tue, 19 Sep 2017 13:40:22 -0700 Subject: [PATCH] [17.06] backport fixes for max grpc message size and lint errors (#2378) * Fix linter errors. Signed-off-by: Anshul Pundir (cherry picked from commit aa2c48b53cbae5203b8cf81262eaccf8be353ca6) Signed-off-by: Andrew Hsu * Set grpc max message size to 128MB. (#2375) Signed-off-by: Anshul Pundir (cherry picked from commit b1bcc0560807d8c61b5d2ab8fc4bd4436686d7ef) Signed-off-by: Andrew Hsu * run vndr again because it now deletes unused files Signed-off-by: Andrew Hsu --- ca/keyreadwriter.go | 5 +- manager/allocator/network.go | 6 +- manager/controlapi/network.go | 6 +- manager/controlapi/service.go | 17 +---- manager/dispatcher/dispatcher.go | 5 +- manager/logbroker/broker_test.go | 24 ++----- manager/manager.go | 4 ++ manager/orchestrator/update/updater.go | 5 +- manager/scheduler/scheduler.go | 6 +- manager/state/raft/raft.go | 10 +-- manager/state/raft/storage/storage.go | 5 +- manager/state/raft/transport/transport.go | 5 +- .../hack/integration-cli-on-swarm/README.md | 67 ------------------- .../agent/vendor.conf | 2 - 14 files changed, 23 insertions(+), 144 deletions(-) delete mode 100644 vendor/github.com/docker/docker/hack/integration-cli-on-swarm/README.md delete mode 100644 vendor/github.com/docker/docker/hack/integration-cli-on-swarm/agent/vendor.conf diff --git a/ca/keyreadwriter.go b/ca/keyreadwriter.go index 964cb3ee84..4a7c0a0eee 100644 --- a/ca/keyreadwriter.go +++ b/ca/keyreadwriter.go @@ -187,10 +187,7 @@ func (k *KeyReadWriter) ViewAndRotateKEK(cb func(KEKData, PEMKeyHeaders) (KEKDat return err } - if err := k.writeKey(keyBlock, updatedKEK, updatedHeaderObj); err != nil { - return err - } - return nil + return k.writeKey(keyBlock, updatedKEK, updatedHeaderObj) } // ViewAndUpdateHeaders updates the header manager, and updates any headers on the existing key diff --git a/manager/allocator/network.go b/manager/allocator/network.go index 52572b951f..660d530d2e 100644 --- a/manager/allocator/network.go +++ b/manager/allocator/network.go @@ -168,11 +168,7 @@ func (a *Allocator) doNetworkInit(ctx context.Context) (err error) { if err := a.allocateServices(ctx, false); err != nil { return err } - if err := a.allocateTasks(ctx, false); err != nil { - return err - } - - return nil + return a.allocateTasks(ctx, false) } func (a *Allocator) doNetworkAlloc(ctx context.Context, ev events.Event) { diff --git a/manager/controlapi/network.go b/manager/controlapi/network.go index 708f36b8f6..b150de0e78 100644 --- a/manager/controlapi/network.go +++ b/manager/controlapi/network.go @@ -96,11 +96,7 @@ func validateNetworkSpec(spec *api.NetworkSpec, pg plugingetter.PluginGetter) er return err } - if err := validateIPAM(spec.IPAM, pg); err != nil { - return err - } - - return nil + return validateIPAM(spec.IPAM, pg) } // CreateNetwork creates and returns a Network based on the provided NetworkSpec. diff --git a/manager/controlapi/service.go b/manager/controlapi/service.go index ecf79579c6..4a745bad56 100644 --- a/manager/controlapi/service.go +++ b/manager/controlapi/service.go @@ -52,10 +52,7 @@ func validateResourceRequirements(r *api.ResourceRequirements) error { if err := validateResources(r.Limits); err != nil { return err } - if err := validateResources(r.Reservations); err != nil { - return err - } - return nil + return validateResources(r.Reservations) } func validateRestartPolicy(rp *api.RestartPolicy) error { @@ -151,11 +148,7 @@ func validateContainerSpec(taskSpec api.TaskSpec) error { return err } - if err := validateHealthCheck(container.Healthcheck); err != nil { - return err - } - - return nil + return validateHealthCheck(container.Healthcheck) } // validateImage validates image name in containerSpec @@ -471,11 +464,7 @@ func validateServiceSpec(spec *api.ServiceSpec) error { if err := validateEndpointSpec(spec.Endpoint); err != nil { return err } - if err := validateMode(spec); err != nil { - return err - } - - return nil + return validateMode(spec) } // checkPortConflicts does a best effort to find if the passed in spec has port diff --git a/manager/dispatcher/dispatcher.go b/manager/dispatcher/dispatcher.go index 4eeb4ccb10..c1d2d17ceb 100644 --- a/manager/dispatcher/dispatcher.go +++ b/manager/dispatcher/dispatcher.go @@ -846,10 +846,7 @@ func (d *Dispatcher) Assignments(r *api.AssignmentsRequest, stream api.Dispatche appliesTo = msg.ResultsIn msg.Type = assignmentType - if err := stream.Send(&msg); err != nil { - return err - } - return nil + return stream.Send(&msg) } // TODO(aaronl): Also send node secrets that should be exposed to diff --git a/manager/logbroker/broker_test.go b/manager/logbroker/broker_test.go index ca825525f7..c133f0097c 100644 --- a/manager/logbroker/broker_test.go +++ b/manager/logbroker/broker_test.go @@ -402,18 +402,14 @@ func TestLogBrokerNoFollow(t *testing.T) { return err } - if err := store.CreateTask(tx, &api.Task{ + return store.CreateTask(tx, &api.Task{ ID: "task2", ServiceID: "service", Status: api.TaskStatus{ State: api.TaskStateRunning, }, NodeID: agent2Security.ServerTLSCreds.NodeID(), - }); err != nil { - return err - } - - return nil + }) })) // We need to sleep here to give ListenSubscriptions time to call @@ -524,18 +520,14 @@ func TestLogBrokerNoFollowMissingNode(t *testing.T) { return err } - if err := store.CreateTask(tx, &api.Task{ + return store.CreateTask(tx, &api.Task{ ID: "task2", ServiceID: "service", NodeID: "node-2", Status: api.TaskStatus{ State: api.TaskStateRunning, }, - }); err != nil { - return err - } - - return nil + }) })) // We need to sleep here to give ListenSubscriptions time to call @@ -655,18 +647,14 @@ func TestLogBrokerNoFollowDisconnect(t *testing.T) { return err } - if err := store.CreateTask(tx, &api.Task{ + return store.CreateTask(tx, &api.Task{ ID: "task2", ServiceID: "service", Status: api.TaskStatus{ State: api.TaskStateRunning, }, NodeID: agent2Security.ServerTLSCreds.NodeID(), - }); err != nil { - return err - } - - return nil + }) })) // We need to sleep here to give ListenSubscriptions time to call diff --git a/manager/manager.go b/manager/manager.go index b8d7383f8e..dd8b7faa09 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -52,6 +52,9 @@ import ( const ( // defaultTaskHistoryRetentionLimit is the number of tasks to keep. defaultTaskHistoryRetentionLimit = 5 + + // Default value for grpc max message size. + grpcMaxMessageSize = 128 << 20 ) // RemoteAddrs provides a listening address and an optional advertise address @@ -214,6 +217,7 @@ func New(config *Config) (*Manager, error) { grpc.Creds(config.SecurityConfig.ServerTLSCreds), grpc.StreamInterceptor(grpc_prometheus.StreamServerInterceptor), grpc.UnaryInterceptor(grpc_prometheus.UnaryServerInterceptor), + grpc.MaxMsgSize(grpcMaxMessageSize), } m := &Manager{ diff --git a/manager/orchestrator/update/updater.go b/manager/orchestrator/update/updater.go index 41cccf837a..72075ce465 100644 --- a/manager/orchestrator/update/updater.go +++ b/manager/orchestrator/update/updater.go @@ -384,10 +384,7 @@ func (u *Updater) updateTask(ctx context.Context, slot orchestrator.Slot, update return errors.New("service was deleted") } - if err := store.CreateTask(tx, updated); err != nil { - return err - } - return nil + return store.CreateTask(tx, updated) }) if err != nil { return err diff --git a/manager/scheduler/scheduler.go b/manager/scheduler/scheduler.go index 96df5633a5..2600331216 100644 --- a/manager/scheduler/scheduler.go +++ b/manager/scheduler/scheduler.go @@ -86,11 +86,7 @@ func (s *Scheduler) setupTasksList(tx store.ReadTx) error { tasksByNode[t.NodeID][t.ID] = t } - if err := s.buildNodeSet(tx, tasksByNode); err != nil { - return err - } - - return nil + return s.buildNodeSet(tx, tasksByNode) } // Run is the scheduler event loop. diff --git a/manager/state/raft/raft.go b/manager/state/raft/raft.go index b793374095..c6f7495bdb 100644 --- a/manager/state/raft/raft.go +++ b/manager/state/raft/raft.go @@ -1249,10 +1249,7 @@ func (n *Node) reportNewAddress(ctx context.Context, id uint64) error { return err } newAddr := net.JoinHostPort(newHost, officialPort) - if err := n.transport.UpdatePeerAddr(id, newAddr); err != nil { - return err - } - return nil + return n.transport.UpdatePeerAddr(id, newAddr) } // ProcessRaftMessage calls 'Step' which advances the @@ -1814,10 +1811,7 @@ func (n *Node) applyAddNode(cc raftpb.ConfChange) error { return nil } - if err = n.registerNode(member); err != nil { - return err - } - return nil + return n.registerNode(member) } // applyUpdateNode is called when we receive a ConfChange from a member in the diff --git a/manager/state/raft/storage/storage.go b/manager/state/raft/storage/storage.go index a737aabde3..539e05d5ce 100644 --- a/manager/state/raft/storage/storage.go +++ b/manager/state/raft/storage/storage.go @@ -226,10 +226,7 @@ func (e *EncryptedRaftLogger) SaveSnapshot(snapshot raftpb.Snapshot) error { if err := snapshotter.SaveSnap(snapshot); err != nil { return err } - if err := e.wal.ReleaseLockTo(snapshot.Metadata.Index); err != nil { - return err - } - return nil + return e.wal.ReleaseLockTo(snapshot.Metadata.Index) } // GC garbage collects snapshots and wals older than the provided index and term diff --git a/manager/state/raft/transport/transport.go b/manager/state/raft/transport/transport.go index 69057f6246..b259013d8a 100644 --- a/manager/state/raft/transport/transport.go +++ b/manager/state/raft/transport/transport.go @@ -235,10 +235,7 @@ func (t *Transport) UpdatePeerAddr(id uint64, addr string) error { if !ok { return ErrIsNotFound } - if err := p.updateAddr(addr); err != nil { - return err - } - return nil + return p.updateAddr(addr) } // PeerConn returns raw grpc connection to peer. diff --git a/vendor/github.com/docker/docker/hack/integration-cli-on-swarm/README.md b/vendor/github.com/docker/docker/hack/integration-cli-on-swarm/README.md deleted file mode 100644 index 7366f72336..0000000000 --- a/vendor/github.com/docker/docker/hack/integration-cli-on-swarm/README.md +++ /dev/null @@ -1,67 +0,0 @@ -# Integration Testing on Swarm - -IT on Swarm allows you to execute integration test in parallel across a Docker Swarm cluster - -## Architecture - -### Master service - - - Works as a funker caller - - Calls a worker funker (`-worker-service`) with a chunk of `-check.f` filter strings (passed as a file via `-input` flag, typically `/mnt/input`) - -### Worker service - - - Works as a funker callee - - Executes an equivalent of `TESTFLAGS=-check.f TestFoo|TestBar|TestBaz ... make test-integration-cli` using the bind-mounted API socket (`docker.sock`) - -### Client - - - Controls master and workers via `docker stack` - - No need to have a local daemon - -Typically, the master and workers are supposed to be running on a cloud environment, -while the client is supposed to be running on a laptop, e.g. Docker for Mac/Windows. - -## Requirement - - - Docker daemon 1.13 or later - - Private registry for distributed execution with multiple nodes - -## Usage - -### Step 1: Prepare images - - $ make build-integration-cli-on-swarm - -Following environment variables are known to work in this step: - - - `BUILDFLAGS` - - `DOCKER_INCREMENTAL_BINARY` - -### Step 2: Execute tests - - $ ./hack/integration-cli-on-swarm/integration-cli-on-swarm -replicas 40 -push-worker-image YOUR_REGISTRY.EXAMPLE.COM/integration-cli-worker:latest - -Following environment variables are known to work in this step: - - - `DOCKER_GRAPHDRIVER` - - `DOCKER_EXPERIMENTAL` - -#### Flags - -Basic flags: - - - `-replicas N`: the number of worker service replicas. i.e. degree of parallelism. - - `-chunks N`: the number of chunks. By default, `chunks` == `replicas`. - - `-push-worker-image REGISTRY/IMAGE:TAG`: push the worker image to the registry. Note that if you have only single node and hence you do not need a private registry, you do not need to specify `-push-worker-image`. - -Experimental flags for mitigating makespan nonuniformity: - - - `-shuffle`: Shuffle the test filter strings - -Flags for debugging IT on Swarm itself: - - - `-rand-seed N`: the random seed. This flag is useful for deterministic replaying. By default(0), the timestamp is used. - - `-filters-file FILE`: the file contains `-check.f` strings. By default, the file is automatically generated. - - `-dry-run`: skip the actual workload - - `keep-executor`: do not auto-remove executor containers, which is used for running privileged programs on Swarm diff --git a/vendor/github.com/docker/docker/hack/integration-cli-on-swarm/agent/vendor.conf b/vendor/github.com/docker/docker/hack/integration-cli-on-swarm/agent/vendor.conf deleted file mode 100644 index efd6d6d049..0000000000 --- a/vendor/github.com/docker/docker/hack/integration-cli-on-swarm/agent/vendor.conf +++ /dev/null @@ -1,2 +0,0 @@ -# dependencies specific to worker (i.e. github.com/docker/docker/...) are not vendored here -github.com/bfirsh/funker-go eaa0a2e06f30e72c9a0b7f858951e581e26ef773