From 748751b900c8cd16b32c36debce19022cd942dd2 Mon Sep 17 00:00:00 2001 From: Jean Rouge Date: Mon, 10 Sep 2018 13:37:08 -0700 Subject: [PATCH 01/12] Adding a Dockerfile and making it easy to use it for dev Credit goes to dperny (https://github.com/docker/swarmkit/pull/2687) **- What I did** Adds a Dockerfile for the swarmkit project, to easily get off the ground. Modifies the Makefile to make intelligent use of Docker. Also made small clean up changes to the Makefile. **- How I did it** Modifies the Makefile to have two paths: containerized.mk, which builds the docker image and forwards any make targets to a container, and direct.mk, which encompasses the old Makefile's workflow. By default, nothing will run inside a container. Set the environment variable `DOCKER_SWARMKIT_USE_CONTAINER` to use dockerized making. Also leverages docker-sync for synchronizing code to the container if the `DOCKER_SWARMKIT_USE_DOCKER_SYNC` env variable is set; comes in handy on Macs, for example. **- How to test it** Set `DOCKER_SWARMKIT_USE_CONTAINER` and verify that your favorite make targets all work! Signed-off-by: Jean Rouge (cherry picked from commit f8c048cd8c00d3530bbdd87e160c6d0a6e30ac42) Signed-off-by: Sebastiaan van Stijn --- .gitignore | 3 + BUILDING.md | 12 ++++ Dockerfile | 30 +++++++++ Makefile | 145 ++++--------------------------------------- containerized.mk | 49 +++++++++++++++ direct.mk | 156 +++++++++++++++++++++++++++++++++++++++++++++++ docker-sync.yml | 9 +++ 7 files changed, 271 insertions(+), 133 deletions(-) create mode 100644 Dockerfile create mode 100644 containerized.mk create mode 100644 direct.mk create mode 100644 docker-sync.yml 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/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..4a951ecc69 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,30 @@ +# NOTE(dperny): for some reason, alpine was giving me trouble +FROM golang:1.10.3-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..8696f2dd64 100644 --- a/Makefile +++ b/Makefile @@ -24,136 +24,15 @@ 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/containerized.mk b/containerized.mk new file mode 100644 index 0000000000..4910352242 --- /dev/null +++ b/containerized.mk @@ -0,0 +1,49 @@ +IMAGE_NAME=docker/swarmkit +GOPATH=/go +DOCKER_IMAGE_DIR=${GOPATH}/src/${PROJECT_ROOT} + +# 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="$$DOCKER_RUN_COMMAND -v swarmkit-sync:${DOCKER_IMAGE_DIR}"; \ + else \ + DOCKER_RUN_COMMAND="$$DOCKER_RUN_COMMAND -v ${ROOTDIR}:${DOCKER_IMAGE_DIR}"; \ + fi \ + && DOCKER_RUN_COMMAND="$$DOCKER_RUN_COMMAND $$DOCKER_SWARMKIT_DOCKER_RUN_FLAGS ${IMAGE_NAME} $$DOCKER_SWARMKIT_DOCKER_RUN_CMD" \ + && echo $$DOCKER_RUN_COMMAND \ + && $$DOCKER_RUN_COMMAND diff --git a/direct.mk b/direct.mk new file mode 100644 index 0000000000..334d3fc81b --- /dev/null +++ b/direct.mk @@ -0,0 +1,156 @@ +.DEFAULT_GOAL = all +.PHONY: all +all: check binaries test integration-tests ## run fmt, vet, lint, build the binaries and run the tests + +.PHONY: check +check: fmt vet lint ineffassign misspell + +.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 + @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 + +.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)) + +# Depends on binaries because vet will silently fail if it can't load compiled +# imports +.PHONY: vet +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)" + +.PHONY: misspell +misspell: + @echo "🐳 $@" + @test -z "$$(find . -type f | grep -v vendor/ | grep -v bin/ | grep -v .git/ | grep -v MAINTAINERS | xargs misspell | tee /dev/stderr)" + +.PHONY: fmt +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) + +.PHONY: lint +lint: ## run go lint + @echo "🐳 $@" + @test -z "$$(golint ./... | grep -v vendor/ | grep -v ".pb.go:" | tee /dev/stderr)" + +.PHONY: ineffassign +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)" + +.PHONY: build +build: ## build the go packages + @echo "🐳 $@" + @go build -i -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 -i -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 -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 ) + +.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'] From 344718a037d6724015c1b2adfbb88f4893eed5bb Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 18 Sep 2018 12:27:00 -0700 Subject: [PATCH 02/12] go {build,test}: rm -i option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Looks like it was used to get faster incremental builds. Nowdays (since Go 1.10) there is no need to use it, because go build cache is used [1]. This fixes `make binaries` on my system where golang is installed as read-only snap: > $ make binaries > 🐳 bin/swarmd > go build runtime/cgo: open /snap/go/2635/pkg/linux_amd64/runtime/cgo.a: read-only file system > direct.mk:100: recipe for target 'bin/swarmd' failed [1] https://groups.google.com/forum/#!msg/golang-dev/qfa3mHN4ZPA/X2UzjNV1BAAJ Signed-off-by: Kir Kolyshkin (cherry picked from commit cb50952a3a15a58bbbb76003994a694d7a658872) Signed-off-by: Sebastiaan van Stijn --- direct.mk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/direct.mk b/direct.mk index 334d3fc81b..52d80df633 100644 --- a/direct.mk +++ b/direct.mk @@ -83,7 +83,7 @@ ineffassign: ## run ineffassign .PHONY: build build: ## build the go packages @echo "🐳 $@" - @go build -i -tags "${DOCKER_BUILDTAGS}" -v ${GO_LDFLAGS} ${GO_GCFLAGS} ${PACKAGES} + @go build -tags "${DOCKER_BUILDTAGS}" -v ${GO_LDFLAGS} ${GO_GCFLAGS} ${PACKAGES} .PHONY: test test: ## run tests, except integration tests @@ -100,7 +100,7 @@ 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} ./$< + @go build -tags "${DOCKER_BUILDTAGS}" -o $@ ${GO_LDFLAGS} ${GO_GCFLAGS} ./$< .PHONY: .FORCE .FORCE: @@ -129,7 +129,7 @@ uninstall: 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; \ go test ${RACE} -tags "${DOCKER_BUILDTAGS}" -test.short -coverprofile="../../../$$pkg/coverage.txt" -covermode=atomic $$pkg || exit; \ done ) From 24212502490316923e5a01d45eb2466c95703c93 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 18 Sep 2018 02:17:17 -0700 Subject: [PATCH 03/12] Makefile: use gometalinter Instead of running many small source code checkers and linters one by one, let's use gometalinter that runs them all in parallel. While at it, remove the individual make targets (fmt, vet, lint, ineffassign, and misspell) and replace with a single check target. One thing it provides is faster source validation. BEFORE: real 0m24.025s user 1m2.646s sys 0m3.860s (note these timings are without building binaries, which for some reason was a dependency of the vet target) AFTER: real 0m6.330s user 0m20.255s sys 0m1.019s In addition to this, it is now way easier to add/remove the checks, as well as to filter out some errors from linters that we consider false positives. [v2: add 2m deadline] [v3: use gometalinter --install; move configuration to .json] Signed-off-by: Kir Kolyshkin (cherry picked from commit 318574db9d8b5953f966b7f7e0e8803fb4de03b4) Signed-off-by: Sebastiaan van Stijn --- .gometalinter.json | 14 ++++++++++++++ direct.mk | 46 +++++++++------------------------------------- 2 files changed, 23 insertions(+), 37 deletions(-) create mode 100644 .gometalinter.json diff --git a/.gometalinter.json b/.gometalinter.json new file mode 100644 index 0000000000..28f126de0f --- /dev/null +++ b/.gometalinter.json @@ -0,0 +1,14 @@ +{ + "Vendor": true, + "Exclude": [ + ".*\\.pb\\.go" + ], + "Enable": [ + "vet", + "misspell", + "gofmt", + "golint", + "ineffassign" + ], + "Deadline": "2m" +} diff --git a/direct.mk b/direct.mk index 52d80df633..68c5edfa49 100644 --- a/direct.mk +++ b/direct.mk @@ -1,9 +1,6 @@ .DEFAULT_GOAL = all .PHONY: all -all: check binaries test integration-tests ## run fmt, vet, lint, build the binaries and run the tests - -.PHONY: check -check: fmt vet lint ineffassign misspell +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 @@ -20,10 +17,8 @@ version/version.go: 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/alecthomas/gometalinter + @gometalinter --install @go get -u github.com/lk4d4/vndr @go get -u github.com/stevvooe/protobuild @@ -44,42 +39,19 @@ checkprotos: generate ## check if protobufs needs to be generated again ((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 -.PHONY: vet -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)" - -.PHONY: misspell -misspell: +.PHONY: check +check: fmt-proto +check: ## Run various source code validation tools @echo "🐳 $@" - @test -z "$$(find . -type f | grep -v vendor/ | grep -v bin/ | grep -v .git/ | grep -v MAINTAINERS | xargs misspell | tee /dev/stderr)" + @gometalinter ./... -.PHONY: fmt -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) +.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: lint -lint: ## run go lint - @echo "🐳 $@" - @test -z "$$(golint ./... | grep -v vendor/ | grep -v ".pb.go:" | tee /dev/stderr)" - -.PHONY: ineffassign -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)" - .PHONY: build build: ## build the go packages @echo "🐳 $@" From 262160c49afc4218bd1a1aea5c2cd9bbf0ba7eea Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 18 Sep 2018 09:50:22 -0700 Subject: [PATCH 04/12] gometalinter: add deadcode linter ...and fix the following initial bunch of warnings: agent/session.go:18:1:warning: errSessionDisconnect is unused (deadcode) agent/errors.go:7:1:warning: errTaskNoController is unused (deadcode) agent/errors.go:7:1:warning: errTaskStatusUpdateNoChange is unused (deadcode) agent/errors.go:7:1:warning: errTaskNotAssigned is unused (deadcode) agent/errors.go:7:1:warning: errTaskInvalid is unused (deadcode) ca/transport.go:21:1:warning: timeoutError is unused (deadcode) ca/config.go:29:1:warning: nodeCSRFilename is unused (deadcode) cmd/swarmctl/node/common.go:58:1:warning: changeNodeMembership is unused (deadcode) integration/cluster.go:44:1:warning: newTestCluster is unused (deadcode) manager/orchestrator/global/global.go:590:1:warning: isTaskCompleted is unused (deadcode) Signed-off-by: Kir Kolyshkin (cherry picked from commit 04ae7e3c35c50f0d28c27b79ccbb45db2c28f780) Signed-off-by: Sebastiaan van Stijn --- .gometalinter.json | 3 ++- agent/errors.go | 7 +---- agent/session.go | 1 - ca/config.go | 1 - ca/transport.go | 6 ----- cmd/swarmctl/node/common.go | 38 --------------------------- integration/cluster.go | 17 ------------ integration/integration_test.go | 17 ++++++++++++ manager/orchestrator/global/global.go | 8 ------ 9 files changed, 20 insertions(+), 78 deletions(-) diff --git a/.gometalinter.json b/.gometalinter.json index 28f126de0f..efc9579617 100644 --- a/.gometalinter.json +++ b/.gometalinter.json @@ -8,7 +8,8 @@ "misspell", "gofmt", "golint", - "ineffassign" + "ineffassign", + "deadcode" ], "Deadline": "2m" } 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/session.go b/agent/session.go index 9bb9773a6c..98b580df30 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") ) diff --git a/ca/config.go b/ca/config.go index 4a7230ac2f..cd53e6b4df 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" 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/swarmctl/node/common.go b/cmd/swarmctl/node/common.go index 2d0beb9ad3..f5e51fa887 100644 --- a/cmd/swarmctl/node/common.go +++ b/cmd/swarmctl/node/common.go @@ -56,44 +56,6 @@ func changeNodeAvailability(cmd *cobra.Command, args []string, availability api. 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 -} - func changeNodeRole(cmd *cobra.Command, args []string, role api.NodeRole) error { if len(args) == 0 { return errors.New("missing node ID") 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/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) -} From 17d7a875a7741088fd609e0ca61992e3b1c0cc34 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 18 Sep 2018 10:15:17 -0700 Subject: [PATCH 05/12] gometalinter: add deadcode, goimports, unconvert ... and fix some warnings from unconvert: ca/config.go:627:58:warning: unnecessary conversion (unconvert) manager/allocator/cnmallocator/portallocator.go:410:38:warning: unnecessary conversion (unconvert) manager/allocator/cnmallocator/portallocator.go:415:50:warning: unnecessary conversion (unconvert) manager/controlapi/service.go:200:47:warning: unnecessary conversion (unconvert) manager/controlapi/service.go:210:45:warning: unnecessary conversion (unconvert) manager/controlapi/service.go:220:35:warning: unnecessary conversion (unconvert) manager/dispatcher/nodes.go:159:33:warning: unnecessary conversion (unconvert) manager/state/store/memory.go:692:91:warning: unnecessary conversion (unconvert) Signed-off-by: Kir Kolyshkin (cherry picked from commit 278edc28b8c4330496c01f39b72c32a054af766d) Signed-off-by: Sebastiaan van Stijn --- .gometalinter.json | 4 +++- ca/config.go | 2 +- manager/allocator/cnmallocator/portallocator.go | 4 ++-- manager/controlapi/service.go | 6 +++--- manager/dispatcher/nodes.go | 2 +- manager/state/store/memory.go | 2 +- 6 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.gometalinter.json b/.gometalinter.json index efc9579617..6710a180dc 100644 --- a/.gometalinter.json +++ b/.gometalinter.json @@ -7,9 +7,11 @@ "vet", "misspell", "gofmt", + "goimports", "golint", "ineffassign", - "deadcode" + "deadcode", + "unconvert" ], "Deadline": "2m" } diff --git a/ca/config.go b/ca/config.go index cd53e6b4df..bb54385409 100644 --- a/ca/config.go +++ b/ca/config.go @@ -625,7 +625,7 @@ 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()) 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/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/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/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 } From 45714ba50d7c140eeb157dee912e6b71f98740f7 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 18 Sep 2018 10:45:05 -0700 Subject: [PATCH 06/12] gometalinter: add gosimple ... and fix warnings reported by it: agent/exec/dockerapi/adapter.go:147:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) agent/testutils/fakes.go:143:2:warning: should use a simple channel send/receive instead of select with a single case (S1000) (gosimple) agent/testutils/fakes.go:151:2:warning: should use a simple channel send/receive instead of select with a single case (S1000) (gosimple) ca/certificates_test.go:708:2:warning: should use for range instead of for { select {} } (S1000) (gosimple) ca/config.go:630:12:warning: should use time.Until instead of t.Sub(time.Now()) (S1024) (gosimple) ca/config_test.go:790:3:warning: should use a simple channel send/receive instead of select with a single case (S1000) (gosimple) ca/external_test.go:116:3:warning: should use a simple channel send/receive instead of select with a single case (S1000) (gosimple) cmd/swarm-bench/collector.go:26:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) cmd/swarmctl/node/common.go:51:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) cmd/swarmctl/node/common.go:89:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) cmd/swarmctl/node/common.go:172:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) ioutils/ioutils_test.go:28:5:warning: should use !bytes.Equal(actual, expected) instead (S1004) (gosimple) manager/allocator/cnmallocator/networkallocator.go:818:3:warning: should merge variable declaration with assignment on next line (S1021) (gosimple) manager/constraint/constraint.go:59:7:warning: should omit comparison to bool constant, can be simplified to !matched (S1002) (gosimple) manager/constraint/constraint.go:67:7:warning: should omit comparison to bool constant, can be simplified to !matched (S1002) (gosimple) manager/dispatcher/dispatcher.go:1095:5:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) manager/dispatcher/dispatcher.go:1095:5:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) manager/dispatcher/dispatcher.go:1095:5:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) manager/dispatcher/dispatcher_test.go:2090:2:warning: redundant return statement (S1023) (gosimple) manager/manager.go:1005:4:warning: should replace loop with m.config.NetworkConfig.DefaultAddrPool = append(m.config.NetworkConfig.DefaultAddrPool, cluster.DefaultAddressPool...) (S1011) (gosimple) manager/metrics/collector.go:191:2:warning: redundant return statement (S1023) (gosimple) manager/metrics/collector.go:222:2:warning: redundant return statement (S1023) (gosimple) manager/orchestrator/replicated/update_test.go:53:3:warning: should use for range instead of for { select {} } (S1000) (gosimple) manager/orchestrator/taskinit/init.go:83:32:warning: should use time.Until instead of t.Sub(time.Now()) (S1024) (gosimple) manager/state/raft/raft.go:1185:2:warning: should use 'return ' instead of 'if { return }; return ' (S1008) (gosimple) manager/state/raft/raft.go:1594:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) node/node.go:1209:2:warning: redundant return statement (S1023) (gosimple) node/node.go:1219:2:warning: redundant return statement (S1023) (gosimple) watch/sinks_test.go:42:2:warning: should merge variable declaration with assignment on next line (S1021) (gosimple) Signed-off-by: Kir Kolyshkin (cherry picked from commit 0e8bb705bab9c06e98dc1f23e8a5be123b002a8b) Signed-off-by: Sebastiaan van Stijn --- .gometalinter.json | 1 + agent/exec/dockerapi/adapter.go | 8 +-- agent/testutils/fakes.go | 8 +-- ca/certificates_test.go | 10 ++-- ca/config.go | 2 +- ca/config_test.go | 34 +++++------ ca/external_test.go | 4 +- cmd/swarm-bench/collector.go | 5 +- cmd/swarmctl/node/common.go | 18 +----- ioutils/ioutils_test.go | 2 +- .../cnmallocator/networkallocator.go | 3 +- manager/constraint/constraint.go | 4 +- manager/dispatcher/dispatcher.go | 12 ++-- manager/dispatcher/dispatcher_test.go | 1 - manager/metrics/collector.go | 3 - .../orchestrator/replicated/update_test.go | 60 +++++++++---------- manager/orchestrator/taskinit/init.go | 2 +- manager/state/raft/raft.go | 10 +--- node/node.go | 5 +- watch/sinks_test.go | 3 +- 20 files changed, 74 insertions(+), 121 deletions(-) diff --git a/.gometalinter.json b/.gometalinter.json index 6710a180dc..c20515ad37 100644 --- a/.gometalinter.json +++ b/.gometalinter.json @@ -9,6 +9,7 @@ "gofmt", "goimports", "golint", + "gosimple", "ineffassign", "deadcode", "unconvert" 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/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/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 bb54385409..0df453e196 100644 --- a/ca/config.go +++ b/ca/config.go @@ -628,7 +628,7 @@ func calculateRandomExpiry(validFrom, validUntil time.Time) time.Duration { 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/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 f5e51fa887..4ce9d59a86 100644 --- a/cmd/swarmctl/node/common.go +++ b/cmd/swarmctl/node/common.go @@ -49,11 +49,7 @@ func changeNodeAvailability(cmd *cobra.Command, args []string, availability api. Spec: spec, }) - if err != nil { - return err - } - - return nil + return err } func changeNodeRole(cmd *cobra.Command, args []string, role api.NodeRole) error { @@ -87,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) { @@ -170,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/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/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/dispatcher/dispatcher.go b/manager/dispatcher/dispatcher.go index 991459574a..d8b0f18409 100644 --- a/manager/dispatcher/dispatcher.go +++ b/manager/dispatcher/dispatcher.go @@ -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/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/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/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/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/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/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) From 80beffd930c21a39e16bd7b198a43e195d751442 Mon Sep 17 00:00:00 2001 From: Jean Rouge Date: Thu, 18 Oct 2018 10:08:22 -0700 Subject: [PATCH 07/12] Adding a `hack/debug` script That's basically just a light wrapper around delve, to make it nicer to use (stops it after the session is done, and doesn't ignore interrupts like the vanilla version does). Also amending the containerized Makefile to add the right run options when using delve. That's controlled by the `DOCKER_SWARMKIT_USE_DELVE` env variable; it's also possible to override the port delve uses via the `DOCKER_SWARMKIT_DELVE_PORT` env variable. Signed-off-by: Jean Rouge (cherry picked from commit 0a88f50964c080e54c0ee78c4639cc53441431a9) Signed-off-by: Sebastiaan van Stijn --- Makefile | 21 -------------- containerized.mk | 16 ++++++++--- direct.mk | 22 +++++++++++++++ hack/debug | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 25 deletions(-) create mode 100755 hack/debug diff --git a/Makefile b/Makefile index 8696f2dd64..c940d00919 100644 --- a/Makefile +++ b/Makefile @@ -1,29 +1,8 @@ # 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)" - SHELL := /bin/bash # stop here. do we want to run everything inside of a container, or do we want diff --git a/containerized.mk b/containerized.mk index 4910352242..063112afa7 100644 --- a/containerized.mk +++ b/containerized.mk @@ -2,6 +2,8 @@ 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 @@ -40,10 +42,16 @@ 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="$$DOCKER_RUN_COMMAND -v swarmkit-sync:${DOCKER_IMAGE_DIR}"; \ + $(MAKE) ensure_sync_started && DOCKER_RUN_COMMAND+=" -v swarmkit-sync:${DOCKER_IMAGE_DIR}"; \ else \ - DOCKER_RUN_COMMAND="$$DOCKER_RUN_COMMAND -v ${ROOTDIR}:${DOCKER_IMAGE_DIR}"; \ + 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_RUN_COMMAND $$DOCKER_SWARMKIT_DOCKER_RUN_FLAGS ${IMAGE_NAME} $$DOCKER_SWARMKIT_DOCKER_RUN_CMD" \ + && DOCKER_RUN_COMMAND+=" $$DOCKER_SWARMKIT_DOCKER_RUN_FLAGS ${IMAGE_NAME} $$DOCKER_SWARMKIT_DOCKER_RUN_CMD" \ && echo $$DOCKER_RUN_COMMAND \ - && $$DOCKER_RUN_COMMAND + && eval $$DOCKER_RUN_COMMAND diff --git a/direct.mk b/direct.mk index 68c5edfa49..0cdeff946a 100644 --- a/direct.mk +++ b/direct.mk @@ -1,3 +1,25 @@ +# 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 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 "$@" From eb383fd34021791f4e9b763a666f24865f37e22e Mon Sep 17 00:00:00 2001 From: Drew Erny Date: Thu, 24 Jan 2019 11:24:23 -0600 Subject: [PATCH 08/12] Fix make check gometalinter dropped support for gosimple, which is deprecated anyway and has been subsumed by staticcheck. This commit removes gosimple from our list of enabled linters (as it's no longer valid). It does not enable staticcheck, because staticcheck throws too many errors. Signed-off-by: Drew Erny (cherry picked from commit 3bfc201ae805c67d604b45c2c4f48350f5873bae) Signed-off-by: Sebastiaan van Stijn --- .gometalinter.json | 1 - 1 file changed, 1 deletion(-) diff --git a/.gometalinter.json b/.gometalinter.json index c20515ad37..6710a180dc 100644 --- a/.gometalinter.json +++ b/.gometalinter.json @@ -9,7 +9,6 @@ "gofmt", "goimports", "golint", - "gosimple", "ineffassign", "deadcode", "unconvert" From ab656b22dec914a659f6d904a1b1603f34a8d156 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 9 Aug 2019 14:29:45 +0200 Subject: [PATCH 09/12] gofmt files Signed-off-by: Sebastiaan van Stijn --- agent/exec/containerd/adapter.go | 2 +- agent/exec/controller_stub.go | 5 +++-- agent/exec/dockerapi/docker_client_stub.go | 9 +++++---- agent/exec/dockerapi/executor.go | 2 +- api/genericresource/resource_management.go | 1 + api/genericresource/validate.go | 1 + 6 files changed, 12 insertions(+), 8 deletions(-) diff --git a/agent/exec/containerd/adapter.go b/agent/exec/containerd/adapter.go index 36365e8887..5b00dccadb 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" 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/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/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" ) From 54035b8d45a8315cc8cbf7b188bd54b66be2b63e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 9 Aug 2019 14:37:25 +0200 Subject: [PATCH 10/12] containerd-adapter: unnecessary conversion (unconvert) ``` agent/exec/containerd/adapter.go:313:26: unnecessary conversion (unconvert) timeout = time.Duration(10 * time.Second) ``` Signed-off-by: Sebastiaan van Stijn --- agent/exec/containerd/adapter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/exec/containerd/adapter.go b/agent/exec/containerd/adapter.go index 5b00dccadb..6f4ba8cc74 100644 --- a/agent/exec/containerd/adapter.go +++ b/agent/exec/containerd/adapter.go @@ -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 ) From 568fb3ba8046494bd8e2ef0d3a79c81fb73a6359 Mon Sep 17 00:00:00 2001 From: Drew Erny Date: Thu, 20 Jun 2019 14:02:39 -0500 Subject: [PATCH 11/12] add golangci-lint gometalinter is deprecated, and golangci-lint is its recommended successor. This commit adds golangci-lint as the linter for swarmkit. In addition, golangci-lint found a few issues in the code that were not yet identified, and so those issues have been fixed. Signed-off-by: Drew Erny (cherry picked from commit 27c2d27e23e76243bb49ec5d803a6e40b3f96f7a) Signed-off-by: Sebastiaan van Stijn --- .golangci.yml | 14 ++++++++++++++ .gometalinter.json | 17 ----------------- agent/agent.go | 2 +- agent/exec/dockerapi/controller.go | 2 +- agent/session.go | 4 ++-- cmd/swarmctl/service/flagparser/tmpfs.go | 2 +- direct.mk | 6 +++--- manager/dispatcher/dispatcher.go | 2 +- manager/drivers/provider.go | 2 +- manager/orchestrator/restart/restart.go | 9 +-------- 10 files changed, 25 insertions(+), 35 deletions(-) create mode 100644 .golangci.yml delete mode 100644 .gometalinter.json 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/.gometalinter.json b/.gometalinter.json deleted file mode 100644 index 6710a180dc..0000000000 --- a/.gometalinter.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "Vendor": true, - "Exclude": [ - ".*\\.pb\\.go" - ], - "Enable": [ - "vet", - "misspell", - "gofmt", - "goimports", - "golint", - "ineffassign", - "deadcode", - "unconvert" - ], - "Deadline": "2m" -} 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/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/session.go b/agent/session.go index 98b580df30..712173eba9 100644 --- a/agent/session.go +++ b/agent/session.go @@ -128,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. @@ -151,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/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/direct.mk b/direct.mk index 0cdeff946a..4e1f4bf216 100644 --- a/direct.mk +++ b/direct.mk @@ -39,8 +39,8 @@ version/version.go: setup: ## install dependencies @echo "🐳 $@" # TODO(stevvooe): Install these from the vendor directory - @go get -u github.com/alecthomas/gometalinter - @gometalinter --install + # 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 @@ -65,7 +65,7 @@ checkprotos: generate ## check if protobufs needs to be generated again check: fmt-proto check: ## Run various source code validation tools @echo "🐳 $@" - @gometalinter ./... + @./bin/golangci-lint run .PHONY: fmt-proto fmt-proto: diff --git a/manager/dispatcher/dispatcher.go b/manager/dispatcher/dispatcher.go index d8b0f18409..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 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/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. From d0282d0d413a6f312cb95f8829f6d687777765f1 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 18 Sep 2018 12:37:33 -0700 Subject: [PATCH 12/12] Switch to go 1.11 Whitespace changes are caused by the fact that gofmt from go-1.11 uses a different heuristic as to how to format the file, making the source code that was OK for go-1.10 causing a warning with go-1.11. NOTE this whitespace change makes the gofmt from go-1.10 complain, so please upgrade your golang. [v2: regen pb files] Signed-off-by: Kir Kolyshkin (cherry picked from commit fd2d7f2ef925282e8cd2920efa253151749102ad) Signed-off-by: Sebastiaan van Stijn --- .circleci/config.yml | 2 +- Dockerfile | 2 +- api/api.pb.txt | 18 +++++++++--------- api/dispatcher.pb.go | 6 +++--- api/logbroker.pb.go | 6 +++--- api/raft.pb.go | 2 +- api/types.pb.go | 2 +- manager/scheduler/constraint_test.go | 2 +- manager/scheduler/nodeinfo.go | 4 ++-- protobuf/plugin/raftproxy/test/service.pb.go | 6 +++--- 10 files changed, 25 insertions(+), 25 deletions(-) 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/Dockerfile b/Dockerfile index 4a951ecc69..924bd00a33 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # NOTE(dperny): for some reason, alpine was giving me trouble -FROM golang:1.10.3-stretch +FROM golang:1.11.0-stretch RUN apt-get update && apt-get install -y make git unzip 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/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/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/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) }