diff --git a/.circleci/config.yml b/.circleci/config.yml index 55a7dcd6e2..4a8c9c620b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -11,7 +11,7 @@ jobs: # Needed to install go OS: linux ARCH: amd64 - GOVERSION: 1.10.3 + GOVERSION: 1.11 # Needed to install protoc PROTOC: https://github.com/google/protobuf/releases/download/v3.5.0/protoc-3.5.0-linux-x86_64.zip diff --git a/.gitignore b/.gitignore index 05d2662884..097fd203b0 100644 --- a/.gitignore +++ b/.gitignore @@ -31,3 +31,6 @@ bin/swarmkitstate # ignore code coverage output *coverage.txt + +# dev sync, if used +/.docker-sync/ diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000000..694ae5c372 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,14 @@ +run: + tests: false +linters: + disable-all: true + enable: + - misspell + - gofmt + - goimports + - golint + - ineffassign + - deadcode + - unconvert + - govet + diff --git a/BUILDING.md b/BUILDING.md index 70e96f5abb..db82902a90 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -113,3 +113,15 @@ NB: As of version 3.0.0-7 the Debian `protobuf-compiler` package lacks a dependency on `libprotobuf-dev` which contains some standard proto definitions, be sure to install both packages. This is [Debian bug #842158](https://bugs.debian.org/842158). + +### Build in a container instead of your local environment + +You can also choose to use a container to build SwarmKit and run tests. Simply +set the `DOCKER_SWARMKIT_USE_CONTAINER` environment variable to any value, +export it, then run `make` targets as you would have done within your local +environment. + +Additionally, if your OS is not Linux, you might want to set and export the +`DOCKER_SWARMKIT_USE_DOCKER_SYNC` environment variable, which will make use of +[docker-sync](https://github.com/EugenMayer/docker-sync) to sync the code to +the container, instead of native mounted volumes. diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000000..924bd00a33 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,30 @@ +# NOTE(dperny): for some reason, alpine was giving me trouble +FROM golang:1.11.0-stretch + +RUN apt-get update && apt-get install -y make git unzip + +# should stay consistent with the version we use in Circle builds +ARG PROTOC_VERSION=3.5.0 +# make a directory to do these operations in +RUN export PROTOC_TMP_DIR=protoc && mkdir -p $PROTOC_TMP_DIR && cd $PROTOC_TMP_DIR \ + # download the pre-built protoc binary + && curl --silent --show-error --location --output protoc.zip \ + https://github.com/google/protobuf/releases/download/v$PROTOC_VERSION/protoc-$PROTOC_VERSION-linux-x86_64.zip \ + # move the binary to /bin. move the well-known types ot /usr/local/include + && unzip protoc.zip && mv bin/protoc /bin/protoc && mv include/* /usr/local/include \ + # remove all of the installation files + && cd .. && rm -rf $PROTOC_TMP_DIR + +WORKDIR /go/src/github.com/docker/swarmkit/ + +# install the dependencies from `make setup` +# we only copy `direct.mk` to avoid busting the cache too easily +COPY direct.mk . +RUN make --file=direct.mk setup + +# now we can copy the rest +COPY . . + +# default to just `make`. If you want to change the default command, change the +# default make command, not this command. +CMD ["make"] diff --git a/Makefile b/Makefile index b9dbdd3d3a..c940d00919 100644 --- a/Makefile +++ b/Makefile @@ -1,159 +1,17 @@ # Root directory of the project (absolute path). ROOTDIR=$(dir $(abspath $(lastword $(MAKEFILE_LIST)))) -# Base path used to install. -DESTDIR=/usr/local - -# Used to populate version variable in main package. -VERSION=$(shell git describe --match 'v[0-9]*' --dirty='.m' --always) - PROJECT_ROOT=github.com/docker/swarmkit -# Race detector is only supported on amd64. -RACE := $(shell test $$(go env GOARCH) != "amd64" || (echo "-race")) - -# Project packages. -PACKAGES=$(shell go list ./... | grep -v /vendor/) -INTEGRATION_PACKAGE=${PROJECT_ROOT}/integration - -# Project binaries. -COMMANDS=swarmd swarmctl swarm-bench swarm-rafttool protoc-gen-gogoswarm -BINARIES=$(addprefix bin/,$(COMMANDS)) - -VNDR=$(shell which vndr || echo '') - -GO_LDFLAGS=-ldflags "-X `go list ./version`.Version=$(VERSION)" - -.PHONY: clean all AUTHORS fmt vet lint build binaries test integration setup generate protos checkprotos coverage ci check help install uninstall dep-validate -.DEFAULT: default - -all: check binaries test integration ## run fmt, vet, lint, build the binaries and run the tests - -check: fmt vet lint ineffassign misspell ## run fmt, vet, lint, ineffassign, misspell - -ci: check binaries checkprotos coverage coverage-integration ## to be used by the CI - -AUTHORS: .mailmap .git/HEAD - git log --format='%aN <%aE>' | sort -fu > $@ - -# This only needs to be generated by hand when cutting full releases. -version/version.go: - ./version/version.sh > $@ - -setup: ## install dependencies - @echo "🐳 $@" - # TODO(stevvooe): Install these from the vendor directory - @go get -u github.com/golang/lint/golint - #@go get -u github.com/kisielk/errcheck - @go get -u github.com/gordonklaus/ineffassign - @go get -u github.com/client9/misspell/cmd/misspell - @go get -u github.com/lk4d4/vndr - @go get -u github.com/stevvooe/protobuild - -generate: protos - @echo "🐳 $@" - @PATH=${ROOTDIR}/bin:${PATH} go generate -x ${PACKAGES} - -protos: bin/protoc-gen-gogoswarm ## generate protobuf - @echo "🐳 $@" - @PATH=${ROOTDIR}/bin:${PATH} protobuild ${PACKAGES} - -checkprotos: generate ## check if protobufs needs to be generated again - @echo "🐳 $@" - @test -z "$$(git status --short | grep ".pb.go" | tee /dev/stderr)" || \ - ((git diff | cat) && \ - (echo "👹 please run 'make generate' when making changes to proto files" && false)) - -# Depends on binaries because vet will silently fail if it can't load compiled -# imports -vet: binaries ## run go vet - @echo "🐳 $@" - @test -z "$$(go vet ${PACKAGES} 2>&1 | grep -v 'constant [0-9]* not a string in call to Errorf' | egrep -v '(timestamp_test.go|duration_test.go|exit status 1)' | tee /dev/stderr)" - -misspell: - @echo "🐳 $@" - @test -z "$$(find . -type f | grep -v vendor/ | grep -v bin/ | grep -v .git/ | grep -v MAINTAINERS | xargs misspell | tee /dev/stderr)" - -fmt: ## run go fmt - @echo "🐳 $@" - @test -z "$$(gofmt -s -l . | grep -v vendor/ | grep -v ".pb.go$$" | tee /dev/stderr)" || \ - (echo "👹 please format Go code with 'gofmt -s -w'" && false) - @test -z "$$(find . -path ./vendor -prune -o ! -name timestamp.proto ! -name duration.proto -name '*.proto' -type f -exec grep -Hn -e "^ " {} \; | tee /dev/stderr)" || \ - (echo "👹 please indent proto files with tabs only" && false) - @test -z "$$(find . -path ./vendor -prune -o -name '*.proto' -type f -exec grep -Hn "Meta meta = " {} \; | grep -v '(gogoproto.nullable) = false' | tee /dev/stderr)" || \ - (echo "👹 meta fields in proto files must have option (gogoproto.nullable) = false" && false) - -lint: ## run go lint - @echo "🐳 $@" - @test -z "$$(golint ./... | grep -v vendor/ | grep -v ".pb.go:" | tee /dev/stderr)" - -ineffassign: ## run ineffassign - @echo "🐳 $@" - @test -z "$$(ineffassign . | grep -v vendor/ | grep -v ".pb.go:" | tee /dev/stderr)" - -#errcheck: ## run go errcheck -# @echo "🐳 $@" -# @test -z "$$(errcheck ./... | grep -v vendor/ | grep -v ".pb.go:" | tee /dev/stderr)" - -build: ## build the go packages - @echo "🐳 $@" - @go build -i -tags "${DOCKER_BUILDTAGS}" -v ${GO_LDFLAGS} ${GO_GCFLAGS} ${PACKAGES} - -test: ## run tests, except integration tests - @echo "🐳 $@" - @go test -parallel 8 ${RACE} -tags "${DOCKER_BUILDTAGS}" $(filter-out ${INTEGRATION_PACKAGE},${PACKAGES}) - -integration: ## run integration tests - @echo "🐳 $@" - @go test -parallel 8 ${RACE} -tags "${DOCKER_BUILDTAGS}" ${INTEGRATION_PACKAGE} - -FORCE: - -# Build a binary from a cmd. -bin/%: cmd/% FORCE - @test $$(go list) = "${PROJECT_ROOT}" || \ - (echo "👹 Please correctly set up your Go build environment. This project must be located at /src/${PROJECT_ROOT}" && false) - @echo "🐳 $@" - @go build -i -tags "${DOCKER_BUILDTAGS}" -o $@ ${GO_LDFLAGS} ${GO_GCFLAGS} ./$< - -binaries: $(BINARIES) ## build binaries - @echo "🐳 $@" - -clean: ## clean up binaries - @echo "🐳 $@" - @rm -f $(BINARIES) - -install: $(BINARIES) ## install binaries - @echo "🐳 $@" - @mkdir -p $(DESTDIR)/bin - @install $(BINARIES) $(DESTDIR)/bin - -uninstall: - @echo "🐳 $@" - @rm -f $(addprefix $(DESTDIR)/bin/,$(notdir $(BINARIES))) - -coverage: ## generate coverprofiles from the unit tests - @echo "🐳 $@" - @( for pkg in $(filter-out ${INTEGRATION_PACKAGE},${PACKAGES}); do \ - go test -i ${RACE} -tags "${DOCKER_BUILDTAGS}" -test.short -coverprofile="../../../$$pkg/coverage.txt" -covermode=atomic $$pkg || exit; \ - go test ${RACE} -tags "${DOCKER_BUILDTAGS}" -test.short -coverprofile="../../../$$pkg/coverage.txt" -covermode=atomic $$pkg || exit; \ - done ) - -coverage-integration: ## generate coverprofiles from the integration tests - @echo "🐳 $@" - go test ${RACE} -tags "${DOCKER_BUILDTAGS}" -test.short -coverprofile="../../../${INTEGRATION_PACKAGE}/coverage.txt" -covermode=atomic ${INTEGRATION_PACKAGE} - -help: ## this help - @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_-]+:.*?## / {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) | sort - -dep-validate: - @echo "+ $@" - $(if $(VNDR), , \ - $(error Please install vndr: go get github.com/lk4d4/vndr)) - @rm -Rf .vendor.bak - @mv vendor .vendor.bak - @$(VNDR) - @test -z "$$(diff -r vendor .vendor.bak 2>&1 | tee /dev/stderr)" || \ - (echo >&2 "+ inconsistent dependencies! what you have in vendor.conf does not match with what you have in vendor" && false) - @rm -Rf vendor - @mv .vendor.bak vendor +SHELL := /bin/bash + +# stop here. do we want to run everything inside of a container, or do we want +# to run it directly on the host? if the user has set ANY non-empty value for +# the variable DOCKER_SWARMKIT_USE_CONTAINER, then we do all of the making +# inside of a container. We will default to using no container, to avoid +# breaking anyone's workflow +ifdef DOCKER_SWARMKIT_USE_CONTAINER +include containerized.mk +else +include direct.mk +endif diff --git a/agent/agent.go b/agent/agent.go index 4a11c69e2f..e63110c3e7 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -573,7 +573,7 @@ func (a *Agent) nodeDescriptionWithHostname(ctx context.Context, tlsInfo *api.No // Override hostname and TLS info if desc != nil { - if a.config.Hostname != "" && desc != nil { + if a.config.Hostname != "" { desc.Hostname = a.config.Hostname } desc.TLSInfo = tlsInfo diff --git a/agent/errors.go b/agent/errors.go index 29f8ff1c9f..f5514d8311 100644 --- a/agent/errors.go +++ b/agent/errors.go @@ -13,10 +13,5 @@ var ( errAgentStarted = errors.New("agent: already started") errAgentNotStarted = errors.New("agent: not started") - errTaskNoController = errors.New("agent: no task controller") - errTaskNotAssigned = errors.New("agent: task not assigned") - errTaskStatusUpdateNoChange = errors.New("agent: no change in task status") - errTaskUnknown = errors.New("agent: task unknown") - - errTaskInvalid = errors.New("task: invalid") + errTaskUnknown = errors.New("agent: task unknown") ) diff --git a/agent/exec/containerd/adapter.go b/agent/exec/containerd/adapter.go index 36365e8887..6f4ba8cc74 100644 --- a/agent/exec/containerd/adapter.go +++ b/agent/exec/containerd/adapter.go @@ -17,7 +17,7 @@ import ( "github.com/docker/swarmkit/api/naming" "github.com/docker/swarmkit/log" gogotypes "github.com/gogo/protobuf/types" - "github.com/opencontainers/runtime-spec/specs-go" + specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/net/context" @@ -310,7 +310,7 @@ func (c *containerAdapter) shutdown(ctx context.Context) error { var ( sig syscall.Signal - timeout = time.Duration(10 * time.Second) + timeout = 10 * time.Second err error ) diff --git a/agent/exec/controller_stub.go b/agent/exec/controller_stub.go index 076955ff80..64c61423b6 100644 --- a/agent/exec/controller_stub.go +++ b/agent/exec/controller_stub.go @@ -1,10 +1,11 @@ package exec import ( - "github.com/docker/swarmkit/api" - "golang.org/x/net/context" "runtime" "strings" + + "github.com/docker/swarmkit/api" + "golang.org/x/net/context" ) // StubController implements the Controller interface, diff --git a/agent/exec/dockerapi/adapter.go b/agent/exec/dockerapi/adapter.go index 45943dd9ea..3fd0e27ed8 100644 --- a/agent/exec/dockerapi/adapter.go +++ b/agent/exec/dockerapi/adapter.go @@ -144,15 +144,13 @@ func (c *containerAdapter) removeNetworks(ctx context.Context) error { } func (c *containerAdapter) create(ctx context.Context) error { - if _, err := c.client.ContainerCreate(ctx, + _, err := c.client.ContainerCreate(ctx, c.container.config(), c.container.hostConfig(), c.container.networkingConfig(), - c.container.name()); err != nil { - return err - } + c.container.name()) - return nil + return err } func (c *containerAdapter) start(ctx context.Context) error { diff --git a/agent/exec/dockerapi/controller.go b/agent/exec/dockerapi/controller.go index 12bac6ec63..e0aa6b9b73 100644 --- a/agent/exec/dockerapi/controller.go +++ b/agent/exec/dockerapi/controller.go @@ -654,7 +654,7 @@ func parsePortMap(portMap nat.PortMap) ([]*api.PortConfig, error) { return nil, err } - protocol := api.ProtocolTCP + var protocol api.PortConfig_Protocol switch strings.ToLower(parts[1]) { case "tcp": protocol = api.ProtocolTCP diff --git a/agent/exec/dockerapi/docker_client_stub.go b/agent/exec/dockerapi/docker_client_stub.go index 653f3df1a8..2b2e6dd741 100644 --- a/agent/exec/dockerapi/docker_client_stub.go +++ b/agent/exec/dockerapi/docker_client_stub.go @@ -1,16 +1,17 @@ package dockerapi import ( + "io" + "runtime" + "strings" + "time" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/events" "github.com/docker/docker/api/types/network" "github.com/docker/docker/client" "golang.org/x/net/context" - "io" - "runtime" - "strings" - "time" ) // StubAPIClient implements the client.APIClient interface, but allows diff --git a/agent/exec/dockerapi/executor.go b/agent/exec/dockerapi/executor.go index 6601ce224b..a87e193ced 100644 --- a/agent/exec/dockerapi/executor.go +++ b/agent/exec/dockerapi/executor.go @@ -3,6 +3,7 @@ package dockerapi import ( "sort" "strings" + "sync" "github.com/docker/docker/api/types/filters" engineapi "github.com/docker/docker/client" @@ -11,7 +12,6 @@ import ( "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/log" "golang.org/x/net/context" - "sync" ) type executor struct { diff --git a/agent/session.go b/agent/session.go index 9bb9773a6c..712173eba9 100644 --- a/agent/session.go +++ b/agent/session.go @@ -16,7 +16,6 @@ import ( var ( dispatcherRPCTimeout = 5 * time.Second - errSessionDisconnect = errors.New("agent: session disconnect") // instructed to disconnect errSessionClosed = errors.New("agent: session closed") ) @@ -129,7 +128,7 @@ func (s *session) start(ctx context.Context, description *api.NodeDescription) e // `ctx` is done and hence fail to propagate the timeout error to the agent. // If the error is not propogated to the agent, the agent will not close // the session or rebuild a new sesssion. - sessionCtx, cancelSession := context.WithCancel(ctx) + sessionCtx, cancelSession := context.WithCancel(ctx) //nolint:govet // Need to run Session in a goroutine since there's no way to set a // timeout for an individual Recv call in a stream. @@ -152,7 +151,7 @@ func (s *session) start(ctx context.Context, description *api.NodeDescription) e select { case err := <-errChan: if err != nil { - return err + return err //nolint:govet } case <-time.After(dispatcherRPCTimeout): cancelSession() diff --git a/agent/testutils/fakes.go b/agent/testutils/fakes.go index b2e5c81150..a67afae8a6 100644 --- a/agent/testutils/fakes.go +++ b/agent/testutils/fakes.go @@ -140,17 +140,13 @@ func (m *MockDispatcher) UpdateTaskStatus(context.Context, *api.UpdateTaskStatus // Tasks keeps an open stream until canceled func (m *MockDispatcher) Tasks(_ *api.TasksRequest, stream api.Dispatcher_TasksServer) error { - select { - case <-stream.Context().Done(): - } + <-stream.Context().Done() return nil } // Assignments keeps an open stream until canceled func (m *MockDispatcher) Assignments(_ *api.AssignmentsRequest, stream api.Dispatcher_AssignmentsServer) error { - select { - case <-stream.Context().Done(): - } + <-stream.Context().Done() return nil } diff --git a/api/api.pb.txt b/api/api.pb.txt index d81acb202b..e8dc3b0ec2 100755 --- a/api/api.pb.txt +++ b/api/api.pb.txt @@ -2490,8 +2490,8 @@ file { label: LABEL_OPTIONAL type: TYPE_UINT32 options { - 65003: "os.FileMode" 65001: 0 + 65003: "os.FileMode" } json_name: "mode" } @@ -2638,8 +2638,8 @@ file { type: TYPE_MESSAGE type_name: ".google.protobuf.Duration" options { - 65011: 1 65001: 0 + 65011: 1 } json_name: "delay" } @@ -3082,8 +3082,8 @@ file { } } options { - 62023: "PublishMode" 62001: 0 + 62023: "PublishMode" } } } @@ -3782,8 +3782,8 @@ file { label: LABEL_OPTIONAL type: TYPE_UINT32 options { - 65003: "os.FileMode" 65001: 0 + 65003: "os.FileMode" } json_name: "mode" } @@ -4199,8 +4199,8 @@ file { } } options { - 62023: "NodeRole" 62001: 0 + 62023: "NodeRole" } } syntax: "proto3" @@ -7980,8 +7980,8 @@ file { type: TYPE_MESSAGE type_name: ".google.protobuf.Duration" options { - 65011: 1 65001: 0 + 65011: 1 } json_name: "period" } @@ -9028,11 +9028,11 @@ file { } } options { - 63017: 1 - 63020: 1 - 63018: 1 63001: 0 63002: 0 + 63017: 1 + 63018: 1 + 63020: 1 } } file { diff --git a/api/dispatcher.pb.go b/api/dispatcher.pb.go index 120df8811f..edd694eb8a 100644 --- a/api/dispatcher.pb.go +++ b/api/dispatcher.pb.go @@ -1685,7 +1685,7 @@ func (p *raftProxyDispatcherServer) Session(r *SessionRequest, stream Dispatcher } streamWrapper := Dispatcher_SessionServerWrapper{ Dispatcher_SessionServer: stream, - ctx: ctx, + ctx: ctx, } return p.local.Session(r, streamWrapper) } @@ -1806,7 +1806,7 @@ func (p *raftProxyDispatcherServer) Tasks(r *TasksRequest, stream Dispatcher_Tas } streamWrapper := Dispatcher_TasksServerWrapper{ Dispatcher_TasksServer: stream, - ctx: ctx, + ctx: ctx, } return p.local.Tasks(r, streamWrapper) } @@ -1857,7 +1857,7 @@ func (p *raftProxyDispatcherServer) Assignments(r *AssignmentsRequest, stream Di } streamWrapper := Dispatcher_AssignmentsServerWrapper{ Dispatcher_AssignmentsServer: stream, - ctx: ctx, + ctx: ctx, } return p.local.Assignments(r, streamWrapper) } diff --git a/api/genericresource/resource_management.go b/api/genericresource/resource_management.go index a89a118d62..506257ab97 100644 --- a/api/genericresource/resource_management.go +++ b/api/genericresource/resource_management.go @@ -2,6 +2,7 @@ package genericresource import ( "fmt" + "github.com/docker/swarmkit/api" ) diff --git a/api/genericresource/validate.go b/api/genericresource/validate.go index eee3706c74..0ad49ff75f 100644 --- a/api/genericresource/validate.go +++ b/api/genericresource/validate.go @@ -2,6 +2,7 @@ package genericresource import ( "fmt" + "github.com/docker/swarmkit/api" ) diff --git a/api/logbroker.pb.go b/api/logbroker.pb.go index 1108088fba..929f35e9c2 100644 --- a/api/logbroker.pb.go +++ b/api/logbroker.pb.go @@ -1356,7 +1356,7 @@ func (p *raftProxyLogsServer) SubscribeLogs(r *SubscribeLogsRequest, stream Logs } streamWrapper := Logs_SubscribeLogsServerWrapper{ Logs_SubscribeLogsServer: stream, - ctx: ctx, + ctx: ctx, } return p.local.SubscribeLogs(r, streamWrapper) } @@ -1479,7 +1479,7 @@ func (p *raftProxyLogBrokerServer) ListenSubscriptions(r *ListenSubscriptionsReq } streamWrapper := LogBroker_ListenSubscriptionsServerWrapper{ LogBroker_ListenSubscriptionsServer: stream, - ctx: ctx, + ctx: ctx, } return p.local.ListenSubscriptions(r, streamWrapper) } @@ -1530,7 +1530,7 @@ func (p *raftProxyLogBrokerServer) PublishLogs(stream LogBroker_PublishLogsServe } streamWrapper := LogBroker_PublishLogsServerWrapper{ LogBroker_PublishLogsServer: stream, - ctx: ctx, + ctx: ctx, } return p.local.PublishLogs(streamWrapper) } diff --git a/api/raft.pb.go b/api/raft.pb.go index ddee3e18a6..2b4e251cf6 100644 --- a/api/raft.pb.go +++ b/api/raft.pb.go @@ -1767,7 +1767,7 @@ func (p *raftProxyRaftServer) StreamRaftMessage(stream Raft_StreamRaftMessageSer } streamWrapper := Raft_StreamRaftMessageServerWrapper{ Raft_StreamRaftMessageServer: stream, - ctx: ctx, + ctx: ctx, } return p.local.StreamRaftMessage(streamWrapper) } diff --git a/api/types.pb.go b/api/types.pb.go index 80ab4a646d..019640aeb5 100644 --- a/api/types.pb.go +++ b/api/types.pb.go @@ -587,7 +587,7 @@ var MaybeEncryptedRecord_Algorithm_name = map[int32]string{ 2: "FERNET_AES_128_CBC", } var MaybeEncryptedRecord_Algorithm_value = map[string]int32{ - "NONE": 0, + "NONE": 0, "SECRETBOX_SALSA20_POLY1305": 1, "FERNET_AES_128_CBC": 2, } diff --git a/ca/certificates_test.go b/ca/certificates_test.go index 2c7895510c..53800ced18 100644 --- a/ca/certificates_test.go +++ b/ca/certificates_test.go @@ -705,12 +705,10 @@ func TestGetRemoteSignedCertificateWithPending(t *testing.T) { var node *api.Node // wait for a new node to show up for node == nil { - select { - case event := <-updates: // we want to skip the first node, which is the test CA - n := event.(api.EventCreateNode).Node.Copy() - if n.Certificate.Status.State == api.IssuanceStatePending { - node = n - } + event := <-updates // we want to skip the first node, which is the test CA + n := event.(api.EventCreateNode).Node.Copy() + if n.Certificate.Status.State == api.IssuanceStatePending { + node = n } } diff --git a/ca/config.go b/ca/config.go index 4a7230ac2f..0df453e196 100644 --- a/ca/config.go +++ b/ca/config.go @@ -32,7 +32,6 @@ const ( rootCAKeyFilename = "swarm-root-ca.key" nodeTLSCertFilename = "swarm-node.crt" nodeTLSKeyFilename = "swarm-node.key" - nodeCSRFilename = "swarm-node.csr" // DefaultRootCN represents the root CN that we should create roots CAs with by default DefaultRootCN = "swarm-ca" @@ -626,10 +625,10 @@ func calculateRandomExpiry(validFrom, validUntil time.Time) time.Duration { if maxValidity-minValidity < 1 { randomExpiry = minValidity } else { - randomExpiry = rand.Intn(maxValidity-minValidity) + int(minValidity) + randomExpiry = rand.Intn(maxValidity-minValidity) + minValidity } - expiry := validFrom.Add(time.Duration(randomExpiry) * time.Minute).Sub(time.Now()) + expiry := time.Until(validFrom.Add(time.Duration(randomExpiry) * time.Minute)) if expiry < 0 { return 0 } diff --git a/ca/config_test.go b/ca/config_test.go index f30d7e3284..69c8c7946c 100644 --- a/ca/config_test.go +++ b/ca/config_test.go @@ -743,24 +743,22 @@ func TestRenewTLSConfigUpdatesRootNonUnknownAuthError(t *testing.T) { go func() { updates, cancel := state.Watch(tc.MemoryStore.WatchQueue(), api.EventCreateNode{}) defer cancel() - select { - case event := <-updates: // we want to skip the first node, which is the test CA - n := event.(api.EventCreateNode).Node - if n.Certificate.Status.State == api.IssuanceStatePending { - signErr <- tc.MemoryStore.Update(func(tx store.Tx) error { - node := store.GetNode(tx, n.ID) - certChain, err := rootCA.ParseValidateAndSignCSR(node.Certificate.CSR, node.Certificate.CN, ca.WorkerRole, tc.Organization) - if err != nil { - return err - } - node.Certificate.Certificate = cautils.ReDateCert(t, certChain, cert, key, time.Now().Add(-5*time.Hour), time.Now().Add(-4*time.Hour)) - node.Certificate.Status = api.IssuanceStatus{ - State: api.IssuanceStateIssued, - } - return store.UpdateNode(tx, node) - }) - return - } + event := <-updates // we want to skip the first node, which is the test CA + n := event.(api.EventCreateNode).Node + if n.Certificate.Status.State == api.IssuanceStatePending { + signErr <- tc.MemoryStore.Update(func(tx store.Tx) error { + node := store.GetNode(tx, n.ID) + certChain, err := rootCA.ParseValidateAndSignCSR(node.Certificate.CSR, node.Certificate.CN, ca.WorkerRole, tc.Organization) + if err != nil { + return err + } + node.Certificate.Certificate = cautils.ReDateCert(t, certChain, cert, key, time.Now().Add(-5*time.Hour), time.Now().Add(-4*time.Hour)) + node.Certificate.Status = api.IssuanceStatus{ + State: api.IssuanceStateIssued, + } + return store.UpdateNode(tx, node) + }) + return } }() diff --git a/ca/external_test.go b/ca/external_test.go index 7018ba1f95..17272dfbd5 100644 --- a/ca/external_test.go +++ b/ca/external_test.go @@ -113,9 +113,7 @@ func TestExternalCASignRequestTimesOut(t *testing.T) { mux := http.NewServeMux() mux.HandleFunc("/", func(http.ResponseWriter, *http.Request) { // hang forever - select { - case <-allDone: - } + <-allDone }) server := httptest.NewServer(mux) diff --git a/ca/transport.go b/ca/transport.go index 6a6309a613..3460e5f7e9 100644 --- a/ca/transport.go +++ b/ca/transport.go @@ -18,12 +18,6 @@ var ( alpnProtoStr = []string{"h2"} ) -type timeoutError struct{} - -func (timeoutError) Error() string { return "mutablecredentials: Dial timed out" } -func (timeoutError) Timeout() bool { return true } -func (timeoutError) Temporary() bool { return true } - // MutableTLSCreds is the credentials required for authenticating a connection using TLS. type MutableTLSCreds struct { // Mutex for the tls config diff --git a/cmd/swarm-bench/collector.go b/cmd/swarm-bench/collector.go index 62d5022868..8b519d0135 100644 --- a/cmd/swarm-bench/collector.go +++ b/cmd/swarm-bench/collector.go @@ -23,10 +23,7 @@ type Collector struct { func (c *Collector) Listen(port int) error { var err error c.ln, err = net.Listen("tcp", ":"+strconv.Itoa(port)) - if err != nil { - return err - } - return nil + return err } // Collect blocks until `count` tasks phoned home. diff --git a/cmd/swarmctl/node/common.go b/cmd/swarmctl/node/common.go index 2d0beb9ad3..4ce9d59a86 100644 --- a/cmd/swarmctl/node/common.go +++ b/cmd/swarmctl/node/common.go @@ -49,49 +49,7 @@ func changeNodeAvailability(cmd *cobra.Command, args []string, availability api. Spec: spec, }) - if err != nil { - return err - } - - return nil -} - -func changeNodeMembership(cmd *cobra.Command, args []string, membership api.NodeSpec_Membership) error { - if len(args) == 0 { - return errors.New("missing node ID") - } - - if len(args) > 1 { - return errors.New("command takes exactly 1 argument") - } - - c, err := common.Dial(cmd) - if err != nil { - return err - } - node, err := getNode(common.Context(cmd), c, args[0]) - if err != nil { - return err - } - spec := &node.Spec - - if spec.Membership == membership { - return errNoChange - } - - spec.Membership = membership - - _, err = c.UpdateNode(common.Context(cmd), &api.UpdateNodeRequest{ - NodeID: node.ID, - NodeVersion: &node.Meta.Version, - Spec: spec, - }) - - if err != nil { - return err - } - - return nil + return err } func changeNodeRole(cmd *cobra.Command, args []string, role api.NodeRole) error { @@ -125,11 +83,7 @@ func changeNodeRole(cmd *cobra.Command, args []string, role api.NodeRole) error Spec: spec, }) - if err != nil { - return err - } - - return nil + return err } func getNode(ctx context.Context, c api.ControlClient, input string) (*api.Node, error) { @@ -208,9 +162,5 @@ func updateNode(cmd *cobra.Command, args []string) error { Spec: spec, }) - if err != nil { - return err - } - - return nil + return err } diff --git a/cmd/swarmctl/service/flagparser/tmpfs.go b/cmd/swarmctl/service/flagparser/tmpfs.go index 0d7a0e276e..aab4509bb2 100644 --- a/cmd/swarmctl/service/flagparser/tmpfs.go +++ b/cmd/swarmctl/service/flagparser/tmpfs.go @@ -64,7 +64,7 @@ func parseTmpfs(flags *pflag.FlagSet, spec *api.ServiceSpec) error { // remove suffix and try again suffix := meat[len(meat)-1] meat = meat[:len(meat)-1] - var multiplier int64 = 1 + var multiplier int64 switch suffix { case 'g': multiplier = 1 << 30 diff --git a/containerized.mk b/containerized.mk new file mode 100644 index 0000000000..063112afa7 --- /dev/null +++ b/containerized.mk @@ -0,0 +1,57 @@ +IMAGE_NAME=docker/swarmkit +GOPATH=/go +DOCKER_IMAGE_DIR=${GOPATH}/src/${PROJECT_ROOT} + +DOCKER_SWARMKIT_DELVE_PORT ?= 2345 + +# don't bother writing every single make target. just pass the call through to +# docker and make +# we prefer `%:` to `.DEFAULT` as the latter doesn't run phony deps +# (see https://www.gnu.org/software/make/manual/html_node/Special-Targets.html) +%:: + @ echo "Running target $@ inside a container" + @ DOCKER_SWARMKIT_DOCKER_RUN_CMD="make $*" $(MAKE) run + +shell: + @ DOCKER_SWARMKIT_DOCKER_RUN_CMD='bash' DOCKER_SWARMKIT_DOCKER_RUN_FLAGS='-i' $(MAKE) run + +.PHONY: image +image: + docker build -t ${IMAGE_NAME} . + +# internal target, only builds the image if it doesn't exist +.PHONY: ensure_image_exists +ensure_image_exists: + @ if [ ! $$(docker images -q ${IMAGE_NAME}) ]; then $(MAKE) image; fi + +# internal target, starts the sync if needed +# uses https://github.com/EugenMayer/docker-sync/blob/47363ee31b71810a60b05822b9c4bd2176951ce8/tasks/sync/sync.thor#L193-L196 +# which is not great, but that's all they expose so far to do this... +# checks if the daemon pid in the .docker-sync directory maps to a running +# process owned by the current user, and otherwise assumes the sync is not +# running, and starts it +.PHONY: ensure_sync_started +ensure_sync_started: + @ kill -0 $$(cat .docker-sync/daemon.pid) 2&> /dev/null || docker-sync start + +# internal target, actually runs a command inside a container +# we don't use the `-i` flag for `docker run` by default as that makes it a pain +# to kill running containers (can't kill with ctrl-c) +.PHONY: run +run: ensure_image_exists + @ [ "$$DOCKER_SWARMKIT_DOCKER_RUN_CMD" ] || exit 1 + @ DOCKER_RUN_COMMAND="docker run -t -v swarmkit-cache:${GOPATH}" \ + && if [ "$$DOCKER_SWARMKIT_USE_DOCKER_SYNC" ]; then \ + $(MAKE) ensure_sync_started && DOCKER_RUN_COMMAND+=" -v swarmkit-sync:${DOCKER_IMAGE_DIR}"; \ + else \ + DOCKER_RUN_COMMAND+=" -v ${ROOTDIR}:${DOCKER_IMAGE_DIR}"; \ + fi \ + && if [ "$$DOCKER_SWARMKIT_USE_DELVE" ]; then \ + DOCKER_RUN_COMMAND="DOCKER_SWARMKIT_DELVE_PORT=${DOCKER_SWARMKIT_DELVE_PORT} $$DOCKER_RUN_COMMAND" ; \ + DOCKER_RUN_COMMAND+=" -p ${DOCKER_SWARMKIT_DELVE_PORT}:${DOCKER_SWARMKIT_DELVE_PORT} -e DOCKER_SWARMKIT_DELVE_PORT"; \ + `# see https://github.com/derekparker/delve/issues/515#issuecomment-214911481'` ; \ + DOCKER_RUN_COMMAND+=" --security-opt=seccomp:unconfined"; \ + fi \ + && DOCKER_RUN_COMMAND+=" $$DOCKER_SWARMKIT_DOCKER_RUN_FLAGS ${IMAGE_NAME} $$DOCKER_SWARMKIT_DOCKER_RUN_CMD" \ + && echo $$DOCKER_RUN_COMMAND \ + && eval $$DOCKER_RUN_COMMAND diff --git a/direct.mk b/direct.mk new file mode 100644 index 0000000000..4e1f4bf216 --- /dev/null +++ b/direct.mk @@ -0,0 +1,150 @@ +# Base path used to install. +DESTDIR=/usr/local + +# Used to populate version variable in main package. +VERSION=$(shell git describe --match 'v[0-9]*' --dirty='.m' --always) + +# Race detector is only supported on amd64. +RACE := $(shell test $$(go env GOARCH) != "amd64" || (echo "-race")) + +# Project packages. +PACKAGES=$(shell go list ./... | grep -v /vendor/) +INTEGRATION_PACKAGE=${PROJECT_ROOT}/integration + +# Project binaries. +COMMANDS=swarmd swarmctl swarm-bench swarm-rafttool protoc-gen-gogoswarm +BINARIES=$(addprefix bin/,$(COMMANDS)) + +VNDR=$(shell which vndr || echo '') + +GO_LDFLAGS=-ldflags "-X `go list ./version`.Version=$(VERSION)" + + +.DEFAULT_GOAL = all +.PHONY: all +all: check binaries test integration-tests ## run check, build the binaries and run the tests + +.PHONY: ci +ci: check binaries checkprotos coverage coverage-integration ## to be used by the CI + +.PHONY: AUTHORS +AUTHORS: .mailmap .git/HEAD + git log --format='%aN <%aE>' | sort -fu > $@ + +# This only needs to be generated by hand when cutting full releases. +version/version.go: + ./version/version.sh > $@ + +.PHONY: setup +setup: ## install dependencies + @echo "🐳 $@" + # TODO(stevvooe): Install these from the vendor directory + # install golangci-lint version 1.17.1 to ./bin/golangci-lint + @curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s v1.17.1 + @go get -u github.com/lk4d4/vndr + @go get -u github.com/stevvooe/protobuild + +.PHONY: generate +generate: protos + @echo "🐳 $@" + @PATH=${ROOTDIR}/bin:${PATH} go generate -x ${PACKAGES} + +.PHONY: protos +protos: bin/protoc-gen-gogoswarm ## generate protobuf + @echo "🐳 $@" + @PATH=${ROOTDIR}/bin:${PATH} protobuild ${PACKAGES} + +.PHONY: checkprotos +checkprotos: generate ## check if protobufs needs to be generated again + @echo "🐳 $@" + @test -z "$$(git status --short | grep ".pb.go" | tee /dev/stderr)" || \ + ((git diff | cat) && \ + (echo "👹 please run 'make generate' when making changes to proto files" && false)) + +.PHONY: check +check: fmt-proto +check: ## Run various source code validation tools + @echo "🐳 $@" + @./bin/golangci-lint run + +.PHONY: fmt-proto +fmt-proto: + @test -z "$$(find . -path ./vendor -prune -o ! -name timestamp.proto ! -name duration.proto -name '*.proto' -type f -exec grep -Hn -e "^ " {} \; | tee /dev/stderr)" || \ + (echo "👹 please indent proto files with tabs only" && false) + @test -z "$$(find . -path ./vendor -prune -o -name '*.proto' -type f -exec grep -Hn "Meta meta = " {} \; | grep -v '(gogoproto.nullable) = false' | tee /dev/stderr)" || \ + (echo "👹 meta fields in proto files must have option (gogoproto.nullable) = false" && false) + +.PHONY: build +build: ## build the go packages + @echo "🐳 $@" + @go build -tags "${DOCKER_BUILDTAGS}" -v ${GO_LDFLAGS} ${GO_GCFLAGS} ${PACKAGES} + +.PHONY: test +test: ## run tests, except integration tests + @echo "🐳 $@" + @go test -parallel 8 ${RACE} -tags "${DOCKER_BUILDTAGS}" $(filter-out ${INTEGRATION_PACKAGE},${PACKAGES}) + +.PHONY: integration-tests +integration-tests: ## run integration tests + @echo "🐳 $@" + @go test -parallel 8 ${RACE} -tags "${DOCKER_BUILDTAGS}" ${INTEGRATION_PACKAGE} + +# Build a binary from a cmd. +bin/%: cmd/% .FORCE + @test $$(go list) = "${PROJECT_ROOT}" || \ + (echo "👹 Please correctly set up your Go build environment. This project must be located at /src/${PROJECT_ROOT}" && false) + @echo "🐳 $@" + @go build -tags "${DOCKER_BUILDTAGS}" -o $@ ${GO_LDFLAGS} ${GO_GCFLAGS} ./$< + +.PHONY: .FORCE +.FORCE: + +.PHONY: binaries +binaries: $(BINARIES) ## build binaries + @echo "🐳 $@" + +.PHONY: clean +clean: ## clean up binaries + @echo "🐳 $@" + @rm -f $(BINARIES) + +.PHONY: install +install: $(BINARIES) ## install binaries + @echo "🐳 $@" + @mkdir -p $(DESTDIR)/bin + @install $(BINARIES) $(DESTDIR)/bin + +.PHONY: uninstall +uninstall: + @echo "🐳 $@" + @rm -f $(addprefix $(DESTDIR)/bin/,$(notdir $(BINARIES))) + +.PHONY: coverage +coverage: ## generate coverprofiles from the unit tests + @echo "🐳 $@" + @( for pkg in $(filter-out ${INTEGRATION_PACKAGE},${PACKAGES}); do \ + go test ${RACE} -tags "${DOCKER_BUILDTAGS}" -test.short -coverprofile="../../../$$pkg/coverage.txt" -covermode=atomic $$pkg || exit; \ + go test ${RACE} -tags "${DOCKER_BUILDTAGS}" -test.short -coverprofile="../../../$$pkg/coverage.txt" -covermode=atomic $$pkg || exit; \ + done ) + +.PHONY: coverage-integration +coverage-integration: ## generate coverprofiles from the integration tests + @echo "🐳 $@" + go test ${RACE} -tags "${DOCKER_BUILDTAGS}" -test.short -coverprofile="../../../${INTEGRATION_PACKAGE}/coverage.txt" -covermode=atomic ${INTEGRATION_PACKAGE} + +.PHONY: help +help: ## this help + @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_-]+:.*?## / {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) | sort + +.PHONY: dep-validate +dep-validate: + @echo "+ $@" + $(if $(VNDR), , \ + $(error Please install vndr: go get github.com/lk4d4/vndr)) + @rm -Rf .vendor.bak + @mv vendor .vendor.bak + @$(VNDR) + @test -z "$$(diff -r vendor .vendor.bak 2>&1 | tee /dev/stderr)" || \ + (echo >&2 "+ inconsistent dependencies! what you have in vendor.conf does not match with what you have in vendor" && false) + @rm -Rf vendor + @mv .vendor.bak vendor diff --git a/docker-sync.yml b/docker-sync.yml new file mode 100644 index 0000000000..67856985da --- /dev/null +++ b/docker-sync.yml @@ -0,0 +1,9 @@ +version: "2" + +options: + verbose: true +syncs: + # should stay the same as the volume name used in `containerized.mk`'s `run` target + swarmkit-sync: + src: '.' + sync_excludes: ['_obj', '_test', 'bin'] diff --git a/hack/debug b/hack/debug new file mode 100755 index 0000000000..34a23fcf4c --- /dev/null +++ b/hack/debug @@ -0,0 +1,71 @@ +#!/bin/bash + +## Installs delve, then runs it as a headless server +## Keeps running until killed +## Also adds some goodies: delve servers ignore interrupts, which is annoying... +## and also once a debugging session is done, the server just hangs there, which +## is equally annoying. +## This script takes care of both these things + +SUBSHELL_PID= +DLV_PID_FILE= +RUNNING=true + +main() { + [ "$1" ] || usage + + ensure_delve_installed || exit $? + + local PORT="$DOCKER_SWARMKIT_DELVE_PORT" + [ "$PORT" ] || PORT=2345 + + local DLV_CMD="dlv test --accept-multiclient --headless --listen=:$PORT --api-version=2 --log $@" + echo $DLV_CMD + + trap handle_interrupt INT + + DLV_PID_FILE=$(mktemp /tmp/dlv.XXXXXX.pid) + local DLV_OUTPUT_FILE=$(mktemp /tmp/dlv.XXXXXX.out) + + # the weird regex is because we keep the output colored + local HALTING_REGEX='^\e\[37mDEBU\e\[0m\[[0-9]+\] halting\s+\e\[37mlayer\e\[0m=debugger' + while $RUNNING; do + # using `script` to keep colored output, and `exec` to get the PID from the + # subshell + script --flush --quiet "$DLV_OUTPUT_FILE" --command 'printf $$ > '"$DLV_PID_FILE && exec $DLV_CMD" & + SUBSHELL_PID=$! + + # wait for either the subshell to die, or for the "halting" line to appear + # in the output + tail --follow --pid="$SUBSHELL_PID" --sleep-interval=0.1 "$DLV_OUTPUT_FILE" 2> /dev/null | grep --perl-regex --line-buffered "$HALTING_REGEX" | ( read && kill_delve ) + + wait "$SUBSHELL_PID" + done + + rm -f "$DLV_PID_FILE" "$DLV_OUTPUT_FILE" +} + +handle_interrupt() { + RUNNING=false + kill_delve +} + +kill_delve() { + if [ -r "$DLV_PID_FILE" ]; then + local DLV_PID=$(cat "$DLV_PID_FILE") + [ "$DLV_PID" ] && kill "$DLV_PID" &> /dev/null + fi + + [ "$SUBSHELL_PID" ] && kill $SUBSHELL_PID &> /dev/null +} + +ensure_delve_installed() { + which dlv &> /dev/null || go get -u github.com/derekparker/delve/cmd/dlv +} + +usage() { + echo "Usage: $0 name/of/go/package [additional dlv test options]" + exit 1 +} + +main "$@" diff --git a/integration/cluster.go b/integration/cluster.go index e46e01edbe..b70af94915 100644 --- a/integration/cluster.go +++ b/integration/cluster.go @@ -39,23 +39,6 @@ type testCluster struct { var testnameKey struct{} -// NewCluster creates new cluster to which nodes can be added. -// AcceptancePolicy is set to most permissive mode on first manager node added. -func newTestCluster(testname string, fips bool) *testCluster { - ctx, cancel := context.WithCancel(context.Background()) - ctx = context.WithValue(ctx, testnameKey, testname) - c := &testCluster{ - ctx: ctx, - cancel: cancel, - nodes: make(map[string]*testNode), - nodesOrder: make(map[string]int), - errs: make(chan error, 1024), - fips: fips, - } - c.api = &dummyAPI{c: c} - return c -} - // Stop makes best effort to stop all nodes and close connections to them. func (c *testCluster) Stop() error { c.cancel() diff --git a/integration/integration_test.go b/integration/integration_test.go index 5fc6ef0efb..53434eaee0 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -63,6 +63,23 @@ func TestMain(m *testing.M) { os.Exit(res) } +// newTestCluster creates new cluster to which nodes can be added. +// AcceptancePolicy is set to most permissive mode on first manager node added. +func newTestCluster(testname string, fips bool) *testCluster { + ctx, cancel := context.WithCancel(context.Background()) + ctx = context.WithValue(ctx, testnameKey, testname) + c := &testCluster{ + ctx: ctx, + cancel: cancel, + nodes: make(map[string]*testNode), + nodesOrder: make(map[string]int), + errs: make(chan error, 1024), + fips: fips, + } + c.api = &dummyAPI{c: c} + return c +} + // pollClusterReady calls control api until all conditions are true: // * all nodes are ready // * all managers has membership == accepted diff --git a/ioutils/ioutils_test.go b/ioutils/ioutils_test.go index 40717a5108..56a69c4ec6 100644 --- a/ioutils/ioutils_test.go +++ b/ioutils/ioutils_test.go @@ -25,7 +25,7 @@ func TestAtomicWriteToFile(t *testing.T) { t.Fatalf("Error reading from file: %v", err) } - if bytes.Compare(actual, expected) != 0 { + if !bytes.Equal(actual, expected) { t.Fatalf("Data mismatch, expected %q, got %q", expected, actual) } } diff --git a/manager/allocator/cnmallocator/networkallocator.go b/manager/allocator/cnmallocator/networkallocator.go index 2d533a47cd..43efc16db9 100644 --- a/manager/allocator/cnmallocator/networkallocator.go +++ b/manager/allocator/cnmallocator/networkallocator.go @@ -805,8 +805,7 @@ func (na *cnmNetworkAllocator) resolveDriver(n *api.Network) (*networkDriver, er d, drvcap := na.drvRegistry.Driver(dName) if d == nil { - var err error - err = na.loadDriver(dName) + err := na.loadDriver(dName) if err != nil { return nil, err } diff --git a/manager/allocator/cnmallocator/portallocator.go b/manager/allocator/cnmallocator/portallocator.go index 113f900242..81447cbdb4 100644 --- a/manager/allocator/cnmallocator/portallocator.go +++ b/manager/allocator/cnmallocator/portallocator.go @@ -407,12 +407,12 @@ func (ps *portSpace) allocate(p *api.PortConfig) (err error) { } defer func() { if err != nil { - ps.dynamicPortSpace.Release(uint64(swarmPort)) + ps.dynamicPortSpace.Release(swarmPort) } }() // Make sure we allocate the same port from the master space. - if err = ps.masterPortSpace.GetSpecificID(uint64(swarmPort)); err != nil { + if err = ps.masterPortSpace.GetSpecificID(swarmPort); err != nil { return } diff --git a/manager/constraint/constraint.go b/manager/constraint/constraint.go index 9f13217ae4..6c49c07728 100644 --- a/manager/constraint/constraint.go +++ b/manager/constraint/constraint.go @@ -56,7 +56,7 @@ func Parse(env []string) ([]Constraint, error) { part0 := strings.TrimSpace(parts[0]) // validate key matched := alphaNumeric.MatchString(part0) - if matched == false { + if !matched { return nil, fmt.Errorf("key '%s' is invalid", part0) } @@ -64,7 +64,7 @@ func Parse(env []string) ([]Constraint, error) { // validate Value matched = valuePattern.MatchString(part1) - if matched == false { + if !matched { return nil, fmt.Errorf("value '%s' is invalid", part1) } // TODO(dongluochen): revisit requirements to see if globing or regex are useful diff --git a/manager/controlapi/service.go b/manager/controlapi/service.go index 3912052bf0..f912bd77f4 100644 --- a/manager/controlapi/service.go +++ b/manager/controlapi/service.go @@ -197,7 +197,7 @@ func validateHealthCheck(hc *api.HealthConfig) error { if err != nil { return err } - if interval != 0 && interval < time.Duration(minimumDuration) { + if interval != 0 && interval < minimumDuration { return status.Errorf(codes.InvalidArgument, "ContainerSpec: Interval in HealthConfig cannot be less than %s", minimumDuration) } } @@ -207,7 +207,7 @@ func validateHealthCheck(hc *api.HealthConfig) error { if err != nil { return err } - if timeout != 0 && timeout < time.Duration(minimumDuration) { + if timeout != 0 && timeout < minimumDuration { return status.Errorf(codes.InvalidArgument, "ContainerSpec: Timeout in HealthConfig cannot be less than %s", minimumDuration) } } @@ -217,7 +217,7 @@ func validateHealthCheck(hc *api.HealthConfig) error { if err != nil { return err } - if sp != 0 && sp < time.Duration(minimumDuration) { + if sp != 0 && sp < minimumDuration { return status.Errorf(codes.InvalidArgument, "ContainerSpec: StartPeriod in HealthConfig cannot be less than %s", minimumDuration) } } diff --git a/manager/dispatcher/dispatcher.go b/manager/dispatcher/dispatcher.go index 991459574a..51140c81ae 100644 --- a/manager/dispatcher/dispatcher.go +++ b/manager/dispatcher/dispatcher.go @@ -230,7 +230,7 @@ func (d *Dispatcher) Run(ctx context.Context) error { if err != nil { return err } - if err == nil && len(clusters) == 1 { + if len(clusters) == 1 { heartbeatPeriod, err := gogotypes.DurationFromProto(clusters[0].Spec.Dispatcher.HeartbeatPeriod) if err == nil && heartbeatPeriod > 0 { d.config.HeartbeatPeriod = heartbeatPeriod @@ -1055,14 +1055,10 @@ func (d *Dispatcher) moveTasksToOrphaned(nodeID string) error { task.Status.State = api.TaskStateOrphaned } - if err := batch.Update(func(tx store.Tx) error { - err := store.UpdateTask(tx, task) - if err != nil { - return err - } - - return nil - }); err != nil { + err := batch.Update(func(tx store.Tx) error { + return store.UpdateTask(tx, task) + }) + if err != nil { return err } diff --git a/manager/dispatcher/dispatcher_test.go b/manager/dispatcher/dispatcher_test.go index 09dcf0d684..7d15e5ac73 100644 --- a/manager/dispatcher/dispatcher_test.go +++ b/manager/dispatcher/dispatcher_test.go @@ -2088,7 +2088,6 @@ func (m *mockPluginGetter) GetAllManagedPluginsByCap(capability string) []plugin return nil } func (m *mockPluginGetter) Handle(capability string, callback func(string, *plugins.Client)) { - return } // MockPlugin mocks a v2 docker plugin diff --git a/manager/dispatcher/nodes.go b/manager/dispatcher/nodes.go index cf35bb869a..fae6dc5f82 100644 --- a/manager/dispatcher/nodes.go +++ b/manager/dispatcher/nodes.go @@ -156,7 +156,7 @@ func (s *nodeStore) Heartbeat(id, sid string) (time.Duration, error) { return 0, err } period := s.periodChooser.Choose() // base period for node - grace := period * time.Duration(s.gracePeriodMultiplierNormal) + grace := period * s.gracePeriodMultiplierNormal rn.mu.Lock() rn.Heartbeat.Update(grace) rn.Heartbeat.Beat() diff --git a/manager/drivers/provider.go b/manager/drivers/provider.go index 0d9be6119d..97c36fe73d 100644 --- a/manager/drivers/provider.go +++ b/manager/drivers/provider.go @@ -22,7 +22,7 @@ func (m *DriverProvider) NewSecretDriver(driver *api.Driver) (*SecretDriver, err if m.pluginGetter == nil { return nil, fmt.Errorf("plugin getter is nil") } - if driver == nil && driver.Name == "" { + if driver == nil || driver.Name == "" { return nil, fmt.Errorf("driver specification is nil") } // Search for the specified plugin diff --git a/manager/metrics/collector.go b/manager/metrics/collector.go index 384743707d..5539a898ca 100644 --- a/manager/metrics/collector.go +++ b/manager/metrics/collector.go @@ -188,7 +188,6 @@ func (c *Collector) handleNodeEvent(event events.Event) { if newNode != nil { nodesMetric.WithValues(strings.ToLower(newNode.Status.State.String())).Inc(1) } - return } func (c *Collector) handleTaskEvent(event events.Event) { @@ -218,8 +217,6 @@ func (c *Collector) handleTaskEvent(event events.Event) { strings.ToLower(newTask.Status.State.String()), ).Inc(1) } - - return } func (c *Collector) handleServiceEvent(event events.Event) { diff --git a/manager/orchestrator/global/global.go b/manager/orchestrator/global/global.go index 2b20813ce2..da5d2a1bc9 100644 --- a/manager/orchestrator/global/global.go +++ b/manager/orchestrator/global/global.go @@ -585,11 +585,3 @@ func (g *Orchestrator) SlotTuple(t *api.Task) orchestrator.SlotTuple { NodeID: t.NodeID, } } - -func isTaskCompleted(t *api.Task, restartPolicy api.RestartPolicy_RestartCondition) bool { - if t == nil || t.DesiredState <= api.TaskStateRunning { - return false - } - return restartPolicy == api.RestartOnNone || - (restartPolicy == api.RestartOnFailure && t.Status.State == api.TaskStateCompleted) -} diff --git a/manager/orchestrator/replicated/update_test.go b/manager/orchestrator/replicated/update_test.go index 1599256fe8..8170b16a0e 100644 --- a/manager/orchestrator/replicated/update_test.go +++ b/manager/orchestrator/replicated/update_test.go @@ -51,38 +51,36 @@ func testUpdaterRollback(t *testing.T, rollbackFailureAction api.UpdateConfig_Fa go func() { failedLast := false for { - select { - case e := <-watchUpdate: - task := e.(api.EventUpdateTask).Task - if task.DesiredState == task.Status.State { - continue - } - if task.DesiredState == api.TaskStateRunning && task.Status.State != api.TaskStateFailed && task.Status.State != api.TaskStateRunning { - err := s.Update(func(tx store.Tx) error { - task = store.GetTask(tx, task.ID) - // Never fail two image2 tasks in a row, so there's a mix of - // failed and successful tasks for the rollback. - if task.Spec.GetContainer().Image == "image1" && atomic.LoadUint32(&failImage1) == 1 { - task.Status.State = api.TaskStateFailed - failedLast = true - } else if task.Spec.GetContainer().Image == "image2" && atomic.LoadUint32(&failImage2) == 1 && !failedLast { - task.Status.State = api.TaskStateFailed - failedLast = true - } else { - task.Status.State = task.DesiredState - failedLast = false - } - return store.UpdateTask(tx, task) - }) - assert.NoError(t, err) - } else if task.DesiredState > api.TaskStateRunning { - err := s.Update(func(tx store.Tx) error { - task = store.GetTask(tx, task.ID) + e := <-watchUpdate + task := e.(api.EventUpdateTask).Task + if task.DesiredState == task.Status.State { + continue + } + if task.DesiredState == api.TaskStateRunning && task.Status.State != api.TaskStateFailed && task.Status.State != api.TaskStateRunning { + err := s.Update(func(tx store.Tx) error { + task = store.GetTask(tx, task.ID) + // Never fail two image2 tasks in a row, so there's a mix of + // failed and successful tasks for the rollback. + if task.Spec.GetContainer().Image == "image1" && atomic.LoadUint32(&failImage1) == 1 { + task.Status.State = api.TaskStateFailed + failedLast = true + } else if task.Spec.GetContainer().Image == "image2" && atomic.LoadUint32(&failImage2) == 1 && !failedLast { + task.Status.State = api.TaskStateFailed + failedLast = true + } else { task.Status.State = task.DesiredState - return store.UpdateTask(tx, task) - }) - assert.NoError(t, err) - } + failedLast = false + } + return store.UpdateTask(tx, task) + }) + assert.NoError(t, err) + } else if task.DesiredState > api.TaskStateRunning { + err := s.Update(func(tx store.Tx) error { + task = store.GetTask(tx, task.ID) + task.Status.State = task.DesiredState + return store.UpdateTask(tx, task) + }) + assert.NoError(t, err) } } }() diff --git a/manager/orchestrator/restart/restart.go b/manager/orchestrator/restart/restart.go index 6af44b734c..3f9c4c8b63 100644 --- a/manager/orchestrator/restart/restart.go +++ b/manager/orchestrator/restart/restart.go @@ -508,20 +508,13 @@ func (r *Supervisor) Cancel(taskID string) { <-delay.doneCh } -// CancelAll aborts all pending restarts and waits for any instances of -// StartNow that have already triggered to complete. +// CancelAll aborts all pending restarts func (r *Supervisor) CancelAll() { - var cancelled []delayedStart - r.mu.Lock() for _, delay := range r.delays { delay.cancel() } r.mu.Unlock() - - for _, delay := range cancelled { - <-delay.doneCh - } } // ClearServiceHistory forgets restart history related to a given service ID. diff --git a/manager/orchestrator/taskinit/init.go b/manager/orchestrator/taskinit/init.go index b893428d51..6316c94a9d 100644 --- a/manager/orchestrator/taskinit/init.go +++ b/manager/orchestrator/taskinit/init.go @@ -80,7 +80,7 @@ func CheckTasks(ctx context.Context, s *store.MemoryStore, readTx store.ReadTx, } if err == nil { restartTime := timestamp.Add(restartDelay) - calculatedRestartDelay := restartTime.Sub(time.Now()) + calculatedRestartDelay := time.Until(restartTime) if calculatedRestartDelay < restartDelay { restartDelay = calculatedRestartDelay } diff --git a/manager/scheduler/constraint_test.go b/manager/scheduler/constraint_test.go index 7bd378139e..04097c8d34 100644 --- a/manager/scheduler/constraint_test.go +++ b/manager/scheduler/constraint_test.go @@ -54,7 +54,7 @@ func setupEnv() { Addr: "186.17.9.41", }, }, - Tasks: make(map[string]*api.Task), + Tasks: make(map[string]*api.Task), ActiveTasksCountByService: make(map[string]int), } } diff --git a/manager/scheduler/nodeinfo.go b/manager/scheduler/nodeinfo.go index 78fa630ca3..7f49058697 100644 --- a/manager/scheduler/nodeinfo.go +++ b/manager/scheduler/nodeinfo.go @@ -45,8 +45,8 @@ type NodeInfo struct { func newNodeInfo(n *api.Node, tasks map[string]*api.Task, availableResources api.Resources) NodeInfo { nodeInfo := NodeInfo{ - Node: n, - Tasks: make(map[string]*api.Task), + Node: n, + Tasks: make(map[string]*api.Task), ActiveTasksCountByService: make(map[string]int), AvailableResources: availableResources.Copy(), usedHostPorts: make(map[hostPortSpec]struct{}), diff --git a/manager/state/raft/raft.go b/manager/state/raft/raft.go index 9eec8d4dfb..2fc433162f 100644 --- a/manager/state/raft/raft.go +++ b/manager/state/raft/raft.go @@ -1172,11 +1172,8 @@ func (n *Node) CanRemoveMember(id uint64) bool { } nquorum := (len(members)-1)/2 + 1 - if nreachable < nquorum { - return false - } - return true + return nreachable >= nquorum } func (n *Node) removeMember(ctx context.Context, id uint64) error { @@ -1581,10 +1578,7 @@ func (n *Node) ProposeValue(ctx context.Context, storeAction []api.StoreAction, defer cancel() _, err := n.processInternalRaftRequest(ctx, &api.InternalRaftRequest{Action: storeAction}, cb) - if err != nil { - return err - } - return nil + return err } // GetVersion returns the sequence information for the current raft round. diff --git a/manager/state/store/memory.go b/manager/state/store/memory.go index e64565fae8..d582679f5a 100644 --- a/manager/state/store/memory.go +++ b/manager/state/store/memory.go @@ -689,7 +689,7 @@ func (tx readTx) findIterators(table string, by By, checkType func(By) error) ([ } return []memdb.ResultIterator{it}, nil case bySlot: - it, err := tx.memDBTx.Get(table, indexSlot, v.serviceID+"\x00"+strconv.FormatUint(uint64(v.slot), 10)) + it, err := tx.memDBTx.Get(table, indexSlot, v.serviceID+"\x00"+strconv.FormatUint(v.slot, 10)) if err != nil { return nil, err } diff --git a/node/node.go b/node/node.go index 9845192c47..e664d985f4 100644 --- a/node/node.go +++ b/node/node.go @@ -1061,19 +1061,16 @@ func (s *persistentRemotes) Observe(peer api.Peer, weight int) { s.c.Broadcast() if err := s.save(); err != nil { logrus.Errorf("error writing cluster state file: %v", err) - return } - return } + func (s *persistentRemotes) Remove(peers ...api.Peer) { s.Lock() defer s.Unlock() s.Remotes.Remove(peers...) if err := s.save(); err != nil { logrus.Errorf("error writing cluster state file: %v", err) - return } - return } func (s *persistentRemotes) save() error { diff --git a/protobuf/plugin/raftproxy/test/service.pb.go b/protobuf/plugin/raftproxy/test/service.pb.go index 7cdd61f1dd..7f5a144f28 100644 --- a/protobuf/plugin/raftproxy/test/service.pb.go +++ b/protobuf/plugin/raftproxy/test/service.pb.go @@ -1091,7 +1091,7 @@ func (p *raftProxyRouteGuideServer) ListFeatures(r *Rectangle, stream RouteGuide } streamWrapper := RouteGuide_ListFeaturesServerWrapper{ RouteGuide_ListFeaturesServer: stream, - ctx: ctx, + ctx: ctx, } return p.local.ListFeatures(r, streamWrapper) } @@ -1142,7 +1142,7 @@ func (p *raftProxyRouteGuideServer) RecordRoute(stream RouteGuide_RecordRouteSer } streamWrapper := RouteGuide_RecordRouteServerWrapper{ RouteGuide_RecordRouteServer: stream, - ctx: ctx, + ctx: ctx, } return p.local.RecordRoute(streamWrapper) } @@ -1199,7 +1199,7 @@ func (p *raftProxyRouteGuideServer) RouteChat(stream RouteGuide_RouteChatServer) } streamWrapper := RouteGuide_RouteChatServerWrapper{ RouteGuide_RouteChatServer: stream, - ctx: ctx, + ctx: ctx, } return p.local.RouteChat(streamWrapper) } diff --git a/watch/sinks_test.go b/watch/sinks_test.go index 867b2f7c42..69593885eb 100644 --- a/watch/sinks_test.go +++ b/watch/sinks_test.go @@ -39,8 +39,7 @@ func TestTimeoutDropErrSinkGen(t *testing.T) { <-ch2.Done() // Make sure that closing a sink closes the channel - var errClose error - errClose = sink.Close() + errClose := sink.Close() <-ch.Done() require.NoError(errClose)