From 65860255c6fa4a06cb70c827db660df5ea5a89ba Mon Sep 17 00:00:00 2001 From: Flavio Crisciani Date: Mon, 12 Jun 2017 11:30:30 -0700 Subject: [PATCH] Fixed code issues Fixed issues highlighted by the new checks Signed-off-by: Flavio Crisciani --- libnetwork/agent.go | 4 ++-- libnetwork/api/api_test.go | 6 ++++++ libnetwork/client/mflag/example/example.go | 2 +- libnetwork/common/setmatrix.go | 2 +- libnetwork/controller.go | 5 +++-- libnetwork/datastore/datastore_test.go | 3 +++ libnetwork/drivers/overlay/ov_network.go | 9 ++------- libnetwork/drivers/solaris/overlay/ov_network.go | 9 ++------- libnetwork/drivers/windows/windows.go | 9 +++++++++ libnetwork/ipam/allocator.go | 2 +- libnetwork/ipam/allocator_test.go | 5 ++--- libnetwork/ipams/remote/api/api.go | 2 +- libnetwork/iptables/firewalld_test.go | 3 +++ libnetwork/iptables/iptables_test.go | 6 ++++++ libnetwork/libnetwork_internal_test.go | 3 +++ libnetwork/network.go | 2 +- libnetwork/osl/namespace_unsupported.go | 1 + libnetwork/portallocator/portallocator_test.go | 5 ++++- libnetwork/service_linux.go | 2 +- libnetwork/testutils/context_solaris.go | 2 -- libnetwork/testutils/context_windows.go | 2 -- 21 files changed, 52 insertions(+), 32 deletions(-) diff --git a/libnetwork/agent.go b/libnetwork/agent.go index 3fa5341481..58913ff664 100644 --- a/libnetwork/agent.go +++ b/libnetwork/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 } diff --git a/libnetwork/api/api_test.go b/libnetwork/api/api_test.go index 1dcc8c6f8c..bb86f4d9c3 100644 --- a/libnetwork/api/api_test.go +++ b/libnetwork/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()} diff --git a/libnetwork/client/mflag/example/example.go b/libnetwork/client/mflag/example/example.go index b93a37c1d8..3c2ecac93d 100644 --- a/libnetwork/client/mflag/example/example.go +++ b/libnetwork/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") diff --git a/libnetwork/common/setmatrix.go b/libnetwork/common/setmatrix.go index 0fdb542be4..52c0d1f908 100644 --- a/libnetwork/common/setmatrix.go +++ b/libnetwork/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 diff --git a/libnetwork/controller.go b/libnetwork/controller.go index ae7dac0b82..1696e07067 100644 --- a/libnetwork/controller.go +++ b/libnetwork/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() diff --git a/libnetwork/datastore/datastore_test.go b/libnetwork/datastore/datastore_test.go index 3cd0aea1ca..909b56bc6a 100644 --- a/libnetwork/datastore/datastore_test.go +++ b/libnetwork/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 { diff --git a/libnetwork/drivers/overlay/ov_network.go b/libnetwork/drivers/overlay/ov_network.go index 6be88d9179..3a4ea41bfc 100644 --- a/libnetwork/drivers/overlay/ov_network.go +++ b/libnetwork/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{} } diff --git a/libnetwork/drivers/solaris/overlay/ov_network.go b/libnetwork/drivers/solaris/overlay/ov_network.go index 5e3dd5abe1..b0567261c7 100644 --- a/libnetwork/drivers/solaris/overlay/ov_network.go +++ b/libnetwork/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{} } diff --git a/libnetwork/drivers/windows/windows.go b/libnetwork/drivers/windows/windows.go index c1c69f2991..eb1522a74a 100644 --- a/libnetwork/drivers/windows/windows.go +++ b/libnetwork/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 diff --git a/libnetwork/ipam/allocator.go b/libnetwork/ipam/allocator.go index 4243d57a74..b3876ffded 100644 --- a/libnetwork/ipam/allocator.go +++ b/libnetwork/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() diff --git a/libnetwork/ipam/allocator_test.go b/libnetwork/ipam/allocator_test.go index 1e1355f223..2ccf88084a 100644 --- a/libnetwork/ipam/allocator_test.go +++ b/libnetwork/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) diff --git a/libnetwork/ipams/remote/api/api.go b/libnetwork/ipams/remote/api/api.go index 23f3eda7d1..543c99bb00 100644 --- a/libnetwork/ipams/remote/api/api.go +++ b/libnetwork/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 == "" } diff --git a/libnetwork/iptables/firewalld_test.go b/libnetwork/iptables/firewalld_test.go index bf36077476..6ce618a49a 100644 --- a/libnetwork/iptables/firewalld_test.go +++ b/libnetwork/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) diff --git a/libnetwork/iptables/iptables_test.go b/libnetwork/iptables/iptables_test.go index 7c12404c19..0664a71afe 100644 --- a/libnetwork/iptables/iptables_test.go +++ b/libnetwork/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) diff --git a/libnetwork/libnetwork_internal_test.go b/libnetwork/libnetwork_internal_test.go index a832d5f7c9..f6cb188ae8 100644 --- a/libnetwork/libnetwork_internal_test.go +++ b/libnetwork/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) diff --git a/libnetwork/network.go b/libnetwork/network.go index ed9a61b456..3798a53900 100644 --- a/libnetwork/network.go +++ b/libnetwork/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 diff --git a/libnetwork/osl/namespace_unsupported.go b/libnetwork/osl/namespace_unsupported.go index e385046121..74372e2492 100644 --- a/libnetwork/osl/namespace_unsupported.go +++ b/libnetwork/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 } diff --git a/libnetwork/portallocator/portallocator_test.go b/libnetwork/portallocator/portallocator_test.go index 40e99c14ed..42d779f417 100644 --- a/libnetwork/portallocator/portallocator_test.go +++ b/libnetwork/portallocator/portallocator_test.go @@ -75,6 +75,9 @@ func TestReuseReleasedPort(t *testing.T) { if err != nil { t.Fatal(err) } + if port != 5000 { + t.Fatalf("Expected port 5000 got %d", port) + } } func TestReleaseUnreadledPort(t *testing.T) { @@ -89,7 +92,7 @@ func TestReleaseUnreadledPort(t *testing.T) { t.Fatalf("Expected port 5000 got %d", port) } - port, err = p.RequestPort(defaultIP, "tcp", 5000) + _, err = p.RequestPort(defaultIP, "tcp", 5000) switch err.(type) { case ErrPortAlreadyAllocated: diff --git a/libnetwork/service_linux.go b/libnetwork/service_linux.go index 70af7e33bc..9e41ba9b7a 100644 --- a/libnetwork/service_linux.go +++ b/libnetwork/service_linux.go @@ -520,7 +520,7 @@ func writePortsToFile(ports []*PortConfig) (string, error) { } defer f.Close() - buf, err := proto.Marshal(&EndpointRecord{ + buf, _ := proto.Marshal(&EndpointRecord{ IngressPorts: ports, }) diff --git a/libnetwork/testutils/context_solaris.go b/libnetwork/testutils/context_solaris.go index 2a9eee2cf2..f072f39b8f 100644 --- a/libnetwork/testutils/context_solaris.go +++ b/libnetwork/testutils/context_solaris.go @@ -1,5 +1,3 @@ -// +build solaris - package testutils import ( diff --git a/libnetwork/testutils/context_windows.go b/libnetwork/testutils/context_windows.go index aa56427f86..f072f39b8f 100644 --- a/libnetwork/testutils/context_windows.go +++ b/libnetwork/testutils/context_windows.go @@ -1,5 +1,3 @@ -// +build windows - package testutils import (