Addressing some golangci-lint complaints

Adding makefile target but not making it part of the build yet

Signed-off-by: Tom Flynn <tom.flynn@gmail.com>

Makefile

@@ -41,6 +41,7 @@ TEST_ARGS ?=
 INSTALL_CMD ?= go install -v
 GOFMT_CHK_CMD := gofmt -s -l
 GOFMT_FIX_CMD := gofmt -s -w
+LINT_CMD := golangci-lint
 GIT_COMMIT=$(shell scripts/getGitCommit.sh)
 PKG_NAME_CONTROLLER=github.com/noironetworks/aci-containers/pkg/controller
 PKG_NAME_GBPSERVER=github.com/noironetworks/aci-containers/pkg/gbpserver
@@ -196,6 +197,9 @@ check-gofmt:
 fix-gofmt:
 	@${GOFMT_FIX_CMD} pkg
 
+lint:
+	@${LINT_CMD} run
+
 check: check-gofmt check-ipam check-index check-apicapi check-controller check-hostagent check-gbpserver
 check-ipam:
 	${TEST_CMD} -coverprofile=covprof-ipam ${BASE}/pkg/ipam ${TEST_ARGS}

cmd/acikubectl/cmd/debug.go

@@ -427,27 +427,6 @@ func getOutfile(output string) (string, *os.File, error) {
 	}
 }
 
-func outputCmd(cmd *cobra.Command, cmdArgs []string) {
-	output, err := cmd.PersistentFlags().GetString("output")
-	if err != nil {
-		fmt.Fprintln(os.Stderr, err)
-		return
-	}
-	outfile := os.Stdout
-	if output != "" {
-		outfile, err := os.Create(output)
-		if err != nil {
-			fmt.Fprintln(os.Stderr, err)
-			return
-		}
-		defer outfile.Close()
-	}
-	err = execKubectl(cmdArgs, outfile)
-	if err != nil {
-		fmt.Fprintln(os.Stderr, err)
-	}
-}
-
 func accLogCmdArgs(systemNamespace string) []string {
 	return []string{"-n", systemNamespace, "logs", "--limit-bytes=10048576",
 		"deployment/aci-containers-controller",

cmd/acikubectl/cmd/default.go

@@ -466,7 +466,7 @@ func init() {
 		}
 		cmd.PersistentFlags().
 			BoolP("raw", "", false,
-				fmt.Sprintf("Get the raw annotation value without formatting"))
+				"Get the raw annotation value without formatting")
 		getDefaultEgCmd.AddCommand(cmd)
 
 		// Set default security group commands
@@ -516,7 +516,7 @@ func init() {
 		}
 		cmd.PersistentFlags().
 			BoolP("raw", "", false,
-				fmt.Sprintf("Get the raw annotation value without formatting"))
+				"Get the raw annotation value without formatting")
 		getDefaultSgCmd.AddCommand(cmd)
 
 	}

pkg/controller/annotation_test.go

@@ -21,27 +21,17 @@ import (
 )
 
 type podTest struct {
-	uuid      string
-	cont      string
-	veth      string
 	namespace string
 	name      string
-	ip        string
-	mac       string
 	eg        string
 	sg        string
 	qp        string
 }
 
 var podTests = []podTest{
 	{
-		"730a8e7a-8455-4d46-8e6e-f4fdf0e3a667",
-		"cont1",
-		"veth1",
 		"testns",
 		"pod1",
-		"10.1.1.1",
-		"00:0c:29:92:fe:d0",
 		egAnnot,
 		sgAnnot,
 		qpAnnot,
@@ -54,8 +44,6 @@ const sgAnnot = "[{\"tenant\": \"testps\", \"name\": \"test-sg\"}]"
 const qpAnnot = "{\"tenant\": \"testps\", " +
 	"\"app-profile\": \"test\", \"name\": \"test-qp\"}"
 
-var vrfEpgDn string = "uni/testps/test/test-eg"
-
 type loginSucc struct {
 	cert bool
 }
@@ -71,14 +59,6 @@ var upgrader = websocket.Upgrader{
 	WriteBufferSize: 1024,
 }
 
-type syncTest struct {
-	desiredState   map[string]apicapi.ApicSlice
-	containerState map[string]apicapi.ApicSlice
-	existing       apicapi.ApicSlice
-	expected       []Request
-	desc           string
-}
-
 type Request struct {
 	method string
 	uri    string

pkg/controller/controller.go

@@ -129,8 +129,7 @@ type AciController struct {
 	// index of pods matched by erspan policies
 	erspanPolPods *index.PodSelectorIndex
 
-	apicConn     *apicapi.ApicConnection
-	tunnelIdBase int64
+	apicConn *apicapi.ApicConnection
 
 	nodeServiceMetaCache map[string]*nodeServiceMeta
 	nodeOpflexDevice     map[string]apicapi.ApicSlice

pkg/controller/erspan_test.go

@@ -34,11 +34,6 @@ type erspanTest struct {
 	erspanPol   *erspanpolicy.ErspanPolicy
 	writeToApic bool
 	desc        string
-	spanDel     bool
-}
-
-func staticErspanKey() string {
-	return "kube_nfp_static"
 }
 
 func erspanpol(name string, namespace string, dest erspanpolicy.ErspanDestType,
@@ -140,11 +135,11 @@ func TestErspanPolicy(t *testing.T) {
 
 	var spanTests = []erspanTest{
 		{erspanpol("test", "testns", dest0, src0, labels),
-			buildSpanObjs(name, "172.51.1.2", 10, "start", "out", macs, vpcs), "test1", true},
+			buildSpanObjs(name, "172.51.1.2", 10, "start", "out", macs, vpcs), "test1"},
 		{erspanpol("test", "testns", dest0, src1, labels),
-			buildSpanObjs(name, "172.51.1.2", 10, "start", "both", macs, vpcs), "test2", true},
+			buildSpanObjs(name, "172.51.1.2", 10, "start", "both", macs, vpcs), "test2"},
 		{erspanpol("test", "testns", dest1, src1, labels),
-			buildSpanObjs(name, "172.51.1.2", 1, "start", "both", macs, vpcs), "test3", true},
+			buildSpanObjs(name, "172.51.1.2", 1, "start", "both", macs, vpcs), "test3"},
 	}
 	initCont := func() *testAciController {
 		cont := testController()

pkg/controller/netflow.go

@@ -105,16 +105,6 @@ func (cont *AciController) queueNetflowUpdateByKey(key string) {
 	cont.netflowQueue.Add(key)
 }
 
-func (cont *AciController) queueNetflowUpdate(netflowpolicy *netflowpolicy.NetflowPolicy) {
-	key, err := cache.MetaNamespaceKeyFunc(netflowpolicy)
-	if err != nil {
-		NetflowPolicyLogger(cont.log, netflowpolicy).
-			Error("Could not create key:" + err.Error())
-		return
-	}
-	cont.netflowQueue.Add(key)
-}
-
 func (cont *AciController) netflowPolicyDelete(obj interface{}) bool {
 	nf, isNf := obj.(*netflowpolicy.NetflowPolicy)
 	if !isNf {

pkg/controller/netflow_test.go

@@ -30,10 +30,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func staticNetflowPolKey() string {
-	return "consul_netflow-policy"
-}
-
 type nfTest struct {
 	netflowPol  *netflowpolicy.NetflowPolicy
 	aciObjSlice apicapi.ApicSlice
@@ -65,21 +61,6 @@ func makeNf(name string, dstAddr string, dstPort int,
 	return apicSlice
 }
 
-func checkNf(t *testing.T, nt *nfTest, category string, cont *testAciController) {
-	tu.WaitFor(t, nt.desc, 500*time.Millisecond,
-		func(last bool) (bool, error) {
-			slice := apicapi.ApicSlice{nt.aciObj}
-			key := cont.aciNameForKey("nf", nt.netflowPol.Name)
-			apicapi.PrepareApicSlice(slice, "kube", key)
-
-			if !tu.WaitEqual(t, last, slice,
-				cont.apicConn.GetDesiredState(key), nt.desc, key) {
-				return false, nil
-			}
-			return true, nil
-		})
-}
-
 func checkDeleteNf(t *testing.T, nt nfTest, cont *testAciController) {
 	tu.WaitFor(t, "delete", 500*time.Millisecond,
 		func(last bool) (bool, error) {

pkg/controller/network_policy.go

@@ -41,7 +41,6 @@ import (
 	"github.com/noironetworks/aci-containers/pkg/apicapi"
 	"github.com/noironetworks/aci-containers/pkg/index"
 	"github.com/noironetworks/aci-containers/pkg/ipam"
-	"github.com/noironetworks/aci-containers/pkg/util"
 	v1beta "k8s.io/api/discovery/v1beta1"
 )
 
@@ -73,10 +72,6 @@ func (cont *AciController) initNetworkPolicyInformerBase(listWatch *cache.ListWa
 		)
 }
 
-func (cont *AciController) getNetPolPolicyTypes(key string) []v1net.PolicyType {
-	return util.GetNetPolPolicyTypes(cont.networkPolicyIndexer, key)
-}
-
 func (cont *AciController) peerPodSelector(np *v1net.NetworkPolicy,
 	peers []v1net.NetworkPolicyPeer) []index.PodSelector {
 

pkg/controller/nodes.go

@@ -136,8 +136,7 @@ func (cont *AciController) createServiceEndpoint(existing *metadata.ServiceEndpo
 		if err == nil {
 			ep.Mac = existing.Mac
 		} else {
-			var mac net.HardwareAddr
-			mac = make([]byte, 6)
+			var mac net.HardwareAddr = make([]byte, 6)
 			_, err := rand.Read(mac)
 			if err != nil {
 				return err

pkg/controller/pods.go

@@ -79,10 +79,7 @@ func podLogger(log *logrus.Logger, pod *v1.Pod) *logrus.Entry {
 }
 
 func podFilter(pod *v1.Pod) bool {
-	if pod.Spec.HostNetwork {
-		return false
-	}
-	return true
+	return !(pod.Spec.HostNetwork)
 }
 
 func (cont *AciController) queuePodUpdate(pod *v1.Pod) {

pkg/controller/qos_test.go

@@ -34,10 +34,6 @@ import (
 	"k8s.io/client-go/tools/cache"
 )
 
-func staticQosReqKey() string {
-	return "kube_qr_static"
-}
-
 type qrTestAugment struct {
 	endpoints []*v1.Endpoints
 	services  []*v1.Service
@@ -92,22 +88,6 @@ func makeQr(ingress apicapi.ApicSlice, egress apicapi.ApicSlice, name string, po
 	return qr1
 }
 
-func checkQr(t *testing.T, qt *qrTest, category string, cont *testAciController) {
-	tu.WaitFor(t, category+"/"+qt.desc, 500*time.Millisecond,
-		func(last bool) (bool, error) {
-			slice := apicapi.ApicSlice{qt.aciObj}
-			key := cont.aciNameForKey("qr",
-				qt.qosPol.Namespace+"_"+qt.qosPol.Name)
-			apicapi.PrepareApicSlice(slice, "kube", key)
-
-			if !tu.WaitEqual(t, last, slice,
-				cont.apicConn.GetDesiredState(key), qt.desc, key) {
-				return false, nil
-			}
-			return true, nil
-		})
-}
-
 func checkDeleteQr(t *testing.T, qt qrTest, cont *testAciController) {
 	tu.WaitFor(t, "delete", 500*time.Millisecond,
 		func(last bool) (bool, error) {

pkg/controller/services.go

@@ -672,8 +672,7 @@ func (cont *AciController) updateServiceDeviceInstanceSnat(key string) error {
 	}
 	sort.Strings(nodes)
 	name := cont.aciNameForKey("snat", key)
-	var conScope string
-	conScope = cont.config.SnatSvcContractScope
+	var conScope = cont.config.SnatSvcContractScope
 	sharedSecurity := true
 
 	graphName := cont.aciNameForKey("svc", "global")
@@ -1695,10 +1694,7 @@ func (seps *serviceEndpointSlice) SetServiceApicObject(aobj apicapi.ApicObject,
 				cont.log.Debug("EndPoint added: ", endpoint.TargetRef.Name)
 			}
 		})
-	if epcount == 0 {
-		return false
-	}
-	return true
+	return epcount != 0
 }
 func getProtocolStr(proto v1.Protocol) string {
 	var protostring string

pkg/controller/snatglobalinfo.go

@@ -37,8 +37,6 @@ import (
 	"strconv"
 )
 
-type set map[string]bool
-
 func (cont *AciController) initSnatNodeInformerFromClient(
 	snatClient *nodeinfoclset.Clientset) {
 	cont.initSnatNodeInformerBase(
@@ -377,7 +375,6 @@ func (cont *AciController) getIpAndPortRange(nodename string, snatpolicy *ContSn
 		snatIps = append(snatIps, serviceIp)
 		return cont.allocateIpSnatPortRange(snatIps, nodename, expandedsnatports)
 	}
-	return "", snatglobalinfo.PortRange{}, false
 }
 
 func (cont *AciController) allocateIpSnatPortRange(snatIps []string, nodename string,

pkg/gbpserver/gbp_common.go

@@ -64,7 +64,6 @@ const (
 	propGw            = "virtualRouterIp"
 	propPrefix        = "prefixLen"
 	propNw            = "address"
-	propMac           = "macAddress"
 	propEther         = "etherT"
 	propProt          = "prot"
 	propDToPort       = "dToPort"

pkg/gbpserver/grpc_server.go

@@ -39,13 +39,8 @@ type gbpWatch struct {
 }
 
 func StartGRPC(port string, gs *Server) (*gbpWatch, error) {
-	level, err := logrus.ParseLevel(gs.config.GRPCLogLevel)
-	if err != nil {
-		panic(err.Error())
-	}
 
 	logger := logrus.New()
-	logger.Level = level
 	log := logger.WithField("mod", "GRPC-S")
 
 	lis, err := net.Listen("tcp", port)

pkg/gbpserver/integ_test.go

@@ -27,7 +27,6 @@ import (
 	"net/url"
 	"os"
 	"reflect"
-	"sort"
 	"strings"
 	"testing"
 	"time"
@@ -724,7 +723,6 @@ func TestGRPC(t *testing.T) {
 			moMap[o.Uri] = o
 		}
 
-		//testPrintSorted(moMap, "testPolicy.json")
 		verifyPolicy(t, moMap)
 	}
 
@@ -849,32 +847,3 @@ func verifyPolicy(t *testing.T, moMap map[string]*GBPObject) {
 		}
 	}
 }
-
-func testPrintSorted(mos map[string]*GBPObject, outFile string) {
-	var keys []string
-
-	for k := range mos {
-		keys = append(keys, k)
-	}
-
-	sort.Strings(keys)
-
-	var sortedMos []*GBPObject
-	for _, kk := range keys {
-		m, ok := mos[kk]
-		if !ok {
-			fmt.Printf("ERROR: missing mo %s", kk)
-			continue
-		}
-		sortedMos = append(sortedMos, m)
-	}
-
-	policyJson, err := json.MarshalIndent(sortedMos, "", "    ")
-	if err != nil {
-		fmt.Printf("ERROR: %v", err)
-	}
-	err = ioutil.WriteFile(outFile, policyJson, 0644)
-	if err != nil {
-		log.Errorf("%s - %v", outFile, err)
-	}
-}