Merge pull request #1776 from fcrisciani/makefile

Makefile

Dockerfile.build

@@ -1,7 +1,9 @@
-FROM golang:1.7.1
+FROM golang:1.8.3
 RUN apt-get update && apt-get -y install iptables
 
 RUN go get github.com/tools/godep \
 		github.com/golang/lint/golint \
-		golang.org/x/tools/cmd/cover\
-		github.com/mattn/goveralls
+		golang.org/x/tools/cmd/cover \
+		github.com/mattn/goveralls \
+		github.com/gordonklaus/ineffassign \
+		github.com/client9/misspell/cmd/misspell

Makefile

@@ -1,4 +1,4 @@
-.PHONY: all all-local build build-local clean cross cross-local check check-code check-format run-tests integration-tests check-local coveralls circle-ci-cross circle-ci-build circle-ci-check circle-ci
+.PHONY: all all-local build build-local clean cross cross-local vet lint misspell check check-code check-format run-tests integration-tests check-local coveralls circle-ci-cross circle-ci-build circle-ci-check circle-ci
 SHELL=/bin/bash
 build_image=libnetworkbuild
 dockerargs = --privileged -v $(shell pwd):/go/src/github.com/docker/libnetwork -w /go/src/github.com/docker/libnetwork
@@ -7,6 +7,7 @@ docker = docker run --rm -it ${dockerargs} $$EXTRA_ARGS ${container_env} ${build
 ciargs = -e CIRCLECI -e "COVERALLS_TOKEN=$$COVERALLS_TOKEN" -e "INSIDECONTAINER=-incontainer=true"
 cidocker = docker run ${dockerargs} ${ciargs} $$EXTRA_ARGS ${container_env} ${build_image}
 CROSS_PLATFORMS = linux/amd64 linux/386 linux/arm windows/amd64
+PACKAGES=$(shell go list ./... | grep -v /vendor/)
 export PATH := $(CURDIR)/bin:$(PATH)
 hostOS = ${shell go env GOHOSTOS}
 ifeq (${hostOS}, solaris)
@@ -22,25 +23,31 @@ all: ${build_image}.created build check integration-tests clean
 all-local: build-local check-local integration-tests-local clean
 
 ${build_image}.created:
+	@echo "🐳 $@"
 	docker build -f Dockerfile.build -t ${build_image} .
 	touch ${build_image}.created
 
 build: ${build_image}.created
-	@echo "Building code... "
+	@echo "🐳 $@"
 	@${docker} ./wrapmake.sh build-local
-	@echo "Done building code"
 
 build-local:
+	@echo "🐳 $@"
 	@mkdir -p "bin"
 	go build -tags experimental -o "bin/dnet" ./cmd/dnet
 	go build -o "bin/docker-proxy" ./cmd/proxy
 
 clean:
+	@echo "🐳 $@"
 	@if [ -d bin ]; then \
 		echo "Removing dnet and proxy binaries"; \
 		rm -rf bin; \
 	fi
 
+force-clean: clean
+	@echo "🐳 $@"
+	@rm -rf ${build_image}.created
+
 cross: ${build_image}.created
 	@mkdir -p "bin"
 	@for platform in ${CROSS_PLATFORMS}; do \
@@ -50,25 +57,19 @@ cross: ${build_image}.created
 	done
 
 cross-local:
+	@echo "🐳 $@"
 	go build -o "bin/dnet-$$GOOS-$$GOARCH" ./cmd/dnet
 	go build -o "bin/docker-proxy-$$GOOS-$$GOARCH" ./cmd/proxy
 
 check: ${build_image}.created
 	@${docker} ./wrapmake.sh check-local
 
-check-code:
-	@echo "Checking code... "
-	test -z "$$(golint ./... | grep -Ev 'vendor|.pb.go:' | tee /dev/stderr)"
-	test -z "$$(go vet ./... 2>&1 > /dev/null | grep -Ev 'vendor|exit' | tee /dev/stderr)"
-	@echo "Done checking code"
+check-code: lint vet ineffassign
 
-check-format:
-	@echo "Checking format... "
-	test -z "$$(gofmt -s -l . | grep -v vendor/ | tee /dev/stderr)"
-	@echo "Done checking format"
+check-format: fmt misspell
 
 run-tests:
-	@echo "Running tests... "
+	@echo "🐳 Running tests... "
 	@echo "mode: count" > coverage.coverprofile
 	@for dir in $$( ${gnufind} . -maxdepth 10 -not -path './.git*' -not -path '*/_*' -not -path './vendor/*' -type d); do \
 	    if [ ${hostOS} == solaris ]; then \
@@ -130,6 +131,28 @@ integration-tests: ./bin/dnet
 coveralls:
 	-@goveralls -service circleci -coverprofile=coverage.coverprofile -repotoken $$COVERALLS_TOKEN
 
+# Depends on binaries because vet will silently fail if it can not load compiled imports
+vet: ## 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)
+
+lint: ## run go lint
+	@echo "🐳 $@"
+	@test -z "$$(golint ./... | grep -v vendor/ | grep -v ".pb.go:" | grep -v ".mock.go" | tee /dev/stderr)"
+
+ineffassign: ## run ineffassign
+	@echo "🐳 $@"
+	@test -z "$$(ineffassign . | grep -v vendor/ | grep -v ".pb.go:" | grep -v ".mock.go" | tee /dev/stderr)"
+
 # CircleCI's Docker fails when cleaning up using the --rm flag
 # The following targets are a workaround for this
 circle-ci-cross: ${build_image}.created

agent.go

@@ -165,13 +165,13 @@ func (c *controller) handleKeyChange(keys []*types.EncryptionKey) error {
 		a.networkDB.SetKey(added)
 	}
 
-	key, tag, err := c.getPrimaryKeyTag(subsysGossip)
+	key, _, err := c.getPrimaryKeyTag(subsysGossip)
 	if err != nil {
 		return err
 	}
 	a.networkDB.SetPrimaryKey(key)
 
-	key, tag, err = c.getPrimaryKeyTag(subsysIPSec)
+	key, tag, err := c.getPrimaryKeyTag(subsysIPSec)
 	if err != nil {
 		return err
 	}

api/api_test.go

@@ -914,6 +914,9 @@ func TestAttachDetachBackend(t *testing.T) {
 
 	cid := "abcdefghi"
 	sbox, err := c.NewSandbox(cid)
+	if err != nil {
+		t.Fatal(err)
+	}
 	sid := sbox.ID()
 	defer sbox.Delete()
 
@@ -1280,6 +1283,9 @@ func TestJoinLeave(t *testing.T) {
 
 	cid := "abcdefghi"
 	sb, err := c.NewSandbox(cid)
+	if err != nil {
+		t.Fatal(err)
+	}
 	defer sb.Delete()
 
 	jl := endpointJoin{SandboxID: sb.ID()}

client/mflag/example/example.go

@@ -13,7 +13,7 @@ var (
 )
 
 func init() {
-	flag.Bool([]string{"#hp", "#-halp"}, false, "display the halp")
+	flag.Bool([]string{"#hp", "#-help"}, false, "display the help")
 	flag.BoolVar(&b, []string{"b", "#bal", "#bol", "-bal"}, false, "a simple bool")
 	flag.BoolVar(&b, []string{"g", "#gil"}, false, "a simple bool")
 	flag.BoolVar(&b2, []string{"#-bool"}, false, "a simple bool")

common/setmatrix.go

@@ -22,7 +22,7 @@ type SetMatrix interface {
 	// returns true if the mapping was deleted, false otherwise
 	// returns also the number of endpoints associated to the IP
 	Remove(key string, value interface{}) (bool, int)
-	// Cardinality returns the number of elements in the set of a specfic key
+	// Cardinality returns the number of elements in the set of a specific key
 	// returns false if the key is not in the map
 	Cardinality(key string) (int, bool)
 	// String returns the string version of the set, empty otherwise

controller.go

@@ -1014,7 +1014,7 @@ func (c *controller) NetworkByID(id string) (Network, error) {
 }
 
 // NewSandbox creates a new sandbox for the passed container id
-func (c *controller) NewSandbox(containerID string, options ...SandboxOption) (sBox Sandbox, err error) {
+func (c *controller) NewSandbox(containerID string, options ...SandboxOption) (Sandbox, error) {
 	if containerID == "" {
 		return nil, types.BadRequestErrorf("invalid container ID")
 	}
@@ -1054,7 +1054,6 @@ func (c *controller) NewSandbox(containerID string, options ...SandboxOption) (s
 			extDNS:             []extDNSEntry{},
 		}
 	}
-	sBox = sb
 
 	heap.Init(&sb.endpoints)
 
@@ -1073,6 +1072,8 @@ func (c *controller) NewSandbox(containerID string, options ...SandboxOption) (s
 		sb.id = "ingress_sbox"
 	}
 	c.Unlock()
+
+	var err error
 	defer func() {
 		if err != nil {
 			c.Lock()

datastore/datastore_test.go

@@ -101,6 +101,9 @@ func TestAtomicKVObjectFlatKey(t *testing.T) {
 	// Get the Object using GetObject, then set again.
 	newObj := dummyObject{}
 	err = store.GetObject(Key(expected.Key()...), &newObj)
+	if err != nil {
+		t.Fatal(err)
+	}
 	assert.True(t, newObj.Exists())
 	err = store.PutObjectAtomic(&n)
 	if err != nil {

drivers/overlay/ov_network.go

@@ -670,7 +670,7 @@ func (n *network) initSandbox(restore bool) error {
 	// In the restore case network sandbox already exist; but we don't know
 	// what epoch number it was created with. It has to be retrieved by
 	// searching the net namespaces.
-	key := ""
+	var key string
 	if restore {
 		key = osl.GenerateKey("-" + n.id)
 	} else {
@@ -872,15 +872,10 @@ func (n *network) Value() []byte {
 		netJSON = append(netJSON, sj)
 	}
 
-	b, err := json.Marshal(netJSON)
-	if err != nil {
-		return []byte{}
-	}
-
 	m["secure"] = n.secure
 	m["subnets"] = netJSON
 	m["mtu"] = n.mtu
-	b, err = json.Marshal(m)
+	b, err := json.Marshal(m)
 	if err != nil {
 		return []byte{}
 	}

drivers/solaris/overlay/ov_network.go

@@ -457,7 +457,7 @@ func (n *network) initSandbox(restore bool) error {
 	// In the restore case network sandbox already exist; but we don't know
 	// what epoch number it was created with. It has to be retrieved by
 	// searching the net namespaces.
-	key := ""
+	var key string
 	if restore {
 		key = osl.GenerateKey("-" + n.id)
 	} else {
@@ -570,15 +570,10 @@ func (n *network) Value() []byte {
 		netJSON = append(netJSON, sj)
 	}
 
-	b, err := json.Marshal(netJSON)
-	if err != nil {
-		return []byte{}
-	}
-
 	m["secure"] = n.secure
 	m["subnets"] = netJSON
 	m["mtu"] = n.mtu
-	b, err = json.Marshal(m)
+	b, err := json.Marshal(m)
 	if err != nil {
 		return []byte{}
 	}

drivers/windows/windows.go

@@ -324,6 +324,9 @@ func (d *driver) CreateNetwork(id string, option map[string]interface{}, nInfo d
 	}
 
 	n, err := d.getNetwork(id)
+	if err != nil {
+		return err
+	}
 	n.created = true
 	return d.storeUpdate(config)
 }
@@ -530,7 +533,13 @@ func (d *driver) CreateEndpoint(nid, eid string, ifInfo driverapi.InterfaceInfo,
 	}
 
 	epOption, err := parseEndpointOptions(epOptions)
+	if err != nil {
+		return err
+	}
 	epConnectivity, err := parseEndpointConnectivity(epOptions)
+	if err != nil {
+		return err
+	}
 
 	macAddress := ifInfo.MacAddress()
 	// Use the macaddress if it was provided

ipam/allocator.go

@@ -448,7 +448,7 @@ func (a *Allocator) RequestAddress(poolID string, prefAddress net.IP, opts map[s
 	c := p
 	for c.Range != nil {
 		k = c.ParentKey
-		c, ok = aSpace.subnets[k]
+		c = aSpace.subnets[k]
 	}
 	aSpace.Unlock()
 

ipam/allocator_test.go

@@ -262,7 +262,7 @@ func TestAddSubnets(t *testing.T) {
 		t.Fatal("returned different pool id for same sub pool requests")
 	}
 
-	pid, _, _, err = a.RequestPool(localAddressSpace, "10.20.2.0/24", "", nil, false)
+	_, _, _, err = a.RequestPool(localAddressSpace, "10.20.2.0/24", "", nil, false)
 	if err == nil {
 		t.Fatal("Failed to detect overlapping subnets")
 	}
@@ -296,7 +296,6 @@ func TestAddReleasePoolID(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	subnets := aSpace.subnets
 	pid0, _, _, err := a.RequestPool(localAddressSpace, "10.0.0.0/8", "", nil, false)
 	if err != nil {
 		t.Fatal("Unexpected failure in adding pool")
@@ -310,7 +309,7 @@ func TestAddReleasePoolID(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	subnets = aSpace.subnets
+	subnets := aSpace.subnets
 
 	if subnets[k0].RefCount != 1 {
 		t.Fatalf("Unexpected ref count for %s: %d", k0, subnets[k0].RefCount)

ipams/remote/api/api.go

@@ -9,7 +9,7 @@ type Response struct {
 	Error string
 }
 
-// IsSuccess returns wheter the plugin response is successful
+// IsSuccess returns whether the plugin response is successful
 func (r *Response) IsSuccess() bool {
 	return r.Error == ""
 }

iptables/firewalld_test.go

@@ -20,6 +20,9 @@ func TestReloaded(t *testing.T) {
 	var fwdChain *ChainInfo
 
 	fwdChain, err = NewChain("FWD", Filter, false)
+	if err != nil {
+		t.Fatal(err)
+	}
 	bridgeName := "lo"
 
 	err = ProgramChain(fwdChain, bridgeName, false, true)

iptables/iptables_test.go

@@ -22,12 +22,18 @@ func TestNewChain(t *testing.T) {
 
 	bridgeName = "lo"
 	natChain, err = NewChain(chainName, Nat, false)
+	if err != nil {
+		t.Fatal(err)
+	}
 	err = ProgramChain(natChain, bridgeName, false, true)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	filterChain, err = NewChain(chainName, Filter, false)
+	if err != nil {
+		t.Fatal(err)
+	}
 	err = ProgramChain(filterChain, bridgeName, false, true)
 	if err != nil {
 		t.Fatal(err)

libnetwork_internal_test.go

@@ -446,6 +446,9 @@ func TestIpamReleaseOnNetDriverFailures(t *testing.T) {
 	}
 
 	cfgOptions, err := OptionBoltdbWithRandomDBFile()
+	if err != nil {
+		t.Fatal(err)
+	}
 	c, err := New(cfgOptions...)
 	if err != nil {
 		t.Fatal(err)

network.go

@@ -1904,7 +1904,7 @@ func (n *network) ResolveIP(ip string) string {
 		return ""
 	}
 	// NOTE it is possible to have more than one element in the Set, this will happen
-	// because of interleave of diffent events from differnt sources (local container create vs
+	// because of interleave of different events from different sources (local container create vs
 	// network db notifications)
 	// In such cases the resolution will be based on the first element of the set, and can vary
 	// during the system stabilitation

osl/namespace_unsupported.go

@@ -7,6 +7,7 @@ package osl
 func GC() {
 }
 
+// GetSandboxForExternalKey returns sandbox object for the supplied path
 func GetSandboxForExternalKey(path string, key string) (Sandbox, error) {
 	return nil, nil
 }