From 8ca98b3643c0f0390ee83e34f76faf52eef2dc07 Mon Sep 17 00:00:00 2001 From: Tom Flynn Date: Fri, 7 May 2021 11:45:46 -0700 Subject: [PATCH] Addressing some golangci-lint complaints Adding makefile target but not making it part of the build yet Signed-off-by: Tom Flynn --- Makefile | 4 ++++ cmd/acikubectl/cmd/debug.go | 21 --------------------- cmd/acikubectl/cmd/default.go | 4 ++-- pkg/controller/annotation_test.go | 20 -------------------- pkg/controller/controller.go | 3 +-- pkg/controller/erspan_test.go | 11 +++-------- pkg/controller/netflow.go | 10 ---------- pkg/controller/netflow_test.go | 19 ------------------- pkg/controller/network_policy.go | 5 ----- pkg/controller/nodes.go | 3 +-- pkg/controller/pods.go | 5 +---- pkg/controller/qos_test.go | 20 -------------------- pkg/controller/services.go | 8 ++------ pkg/controller/snatglobalinfo.go | 3 --- pkg/gbpserver/gbp_common.go | 1 - pkg/gbpserver/grpc_server.go | 5 ----- pkg/gbpserver/integ_test.go | 31 ------------------------------- pkg/hostagent/agent.go | 6 ------ pkg/hostagent/pods.go | 5 +---- pkg/hostagent/snats.go | 8 -------- pkg/metrics/prom_exporter.go | 2 +- pkg/objdb/etcd_client.go | 10 ++-------- 22 files changed, 18 insertions(+), 186 deletions(-) diff --git a/Makefile b/Makefile index 02fc391069..909f26f13f 100644 --- a/Makefile +++ b/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} diff --git a/cmd/acikubectl/cmd/debug.go b/cmd/acikubectl/cmd/debug.go index 7ca6a8a889..492bd26594 100644 --- a/cmd/acikubectl/cmd/debug.go +++ b/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", diff --git a/cmd/acikubectl/cmd/default.go b/cmd/acikubectl/cmd/default.go index 1c29bcc767..808fefbc94 100644 --- a/cmd/acikubectl/cmd/default.go +++ b/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) } diff --git a/pkg/controller/annotation_test.go b/pkg/controller/annotation_test.go index a8e7a2c833..e974891fd7 100644 --- a/pkg/controller/annotation_test.go +++ b/pkg/controller/annotation_test.go @@ -21,13 +21,8 @@ import ( ) type podTest struct { - uuid string - cont string - veth string namespace string name string - ip string - mac string eg string sg string qp string @@ -35,13 +30,8 @@ type podTest struct { 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 diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index a3160b098f..220bb635e2 100644 --- a/pkg/controller/controller.go +++ b/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 diff --git a/pkg/controller/erspan_test.go b/pkg/controller/erspan_test.go index 05c25f86e4..990f772806 100644 --- a/pkg/controller/erspan_test.go +++ b/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() diff --git a/pkg/controller/netflow.go b/pkg/controller/netflow.go index 5dcf13df06..00fbac3f63 100644 --- a/pkg/controller/netflow.go +++ b/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 { diff --git a/pkg/controller/netflow_test.go b/pkg/controller/netflow_test.go index ac7065ba32..0ac78f010a 100644 --- a/pkg/controller/netflow_test.go +++ b/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) { diff --git a/pkg/controller/network_policy.go b/pkg/controller/network_policy.go index 658e58cf82..884135506f 100644 --- a/pkg/controller/network_policy.go +++ b/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 { diff --git a/pkg/controller/nodes.go b/pkg/controller/nodes.go index 2a6ceaa2cc..564dfcf835 100644 --- a/pkg/controller/nodes.go +++ b/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 diff --git a/pkg/controller/pods.go b/pkg/controller/pods.go index 03d968a0ef..04157a122b 100644 --- a/pkg/controller/pods.go +++ b/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) { diff --git a/pkg/controller/qos_test.go b/pkg/controller/qos_test.go index af153c773c..977e9e999e 100644 --- a/pkg/controller/qos_test.go +++ b/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) { diff --git a/pkg/controller/services.go b/pkg/controller/services.go index 7d06ffcb08..333338b90e 100644 --- a/pkg/controller/services.go +++ b/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 diff --git a/pkg/controller/snatglobalinfo.go b/pkg/controller/snatglobalinfo.go index 9cdd360048..a77f863475 100644 --- a/pkg/controller/snatglobalinfo.go +++ b/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, diff --git a/pkg/gbpserver/gbp_common.go b/pkg/gbpserver/gbp_common.go index 700b2967c0..29f0d45402 100644 --- a/pkg/gbpserver/gbp_common.go +++ b/pkg/gbpserver/gbp_common.go @@ -64,7 +64,6 @@ const ( propGw = "virtualRouterIp" propPrefix = "prefixLen" propNw = "address" - propMac = "macAddress" propEther = "etherT" propProt = "prot" propDToPort = "dToPort" diff --git a/pkg/gbpserver/grpc_server.go b/pkg/gbpserver/grpc_server.go index 81674a71d3..49ee2fd232 100644 --- a/pkg/gbpserver/grpc_server.go +++ b/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) diff --git a/pkg/gbpserver/integ_test.go b/pkg/gbpserver/integ_test.go index 885ed6eadb..f54afc7ca3 100644 --- a/pkg/gbpserver/integ_test.go +++ b/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) - } -} diff --git a/pkg/hostagent/agent.go b/pkg/hostagent/agent.go index b183eb18e4..ccaa436892 100644 --- a/pkg/hostagent/agent.go +++ b/pkg/hostagent/agent.go @@ -84,7 +84,6 @@ type HostAgent struct { ignoreOvsPorts map[string][]string netNsFuncChan chan func() vtepIP string - vtepIface string gbpServerIP string opflexSnatGlobalInfos map[string][]*opflexSnatGlobalInfo opflexSnatLocalInfos map[string]*opflexSnatLocalInfo @@ -103,11 +102,6 @@ type HostAgent struct { podtoServiceUids map[string]map[string]string } -type Vtep struct { - vtepIP string - vtepIface string -} - type ServiceEndPointType interface { InitClientInformer(kubeClient *kubernetes.Clientset) Run(stopCh <-chan struct{}) diff --git a/pkg/hostagent/pods.go b/pkg/hostagent/pods.go index 6f970c6736..946d606cf6 100644 --- a/pkg/hostagent/pods.go +++ b/pkg/hostagent/pods.go @@ -433,10 +433,7 @@ func (agent *HostAgent) creatNullMacEp() { } func podFilter(pod *v1.Pod) bool { - if pod.Spec.HostNetwork { - return false - } - return true + return !(pod.Spec.HostNetwork) } func (agent *HostAgent) podUpdated(obj interface{}) { diff --git a/pkg/hostagent/snats.go b/pkg/hostagent/snats.go index 3d459f0bb8..e13c06cb60 100644 --- a/pkg/hostagent/snats.go +++ b/pkg/hostagent/snats.go @@ -126,14 +126,6 @@ func (agent *HostAgent) initSnatPolicyInformerFromClient( }) } -func getsnat(snatfile string) (string, error) { - raw, err := ioutil.ReadFile(snatfile) - if err != nil { - return "", err - } - return string(raw), err -} - func writeSnat(snatfile string, snat *OpflexSnatIp) (bool, error) { newdata, err := json.MarshalIndent(snat, "", " ") if err != nil { diff --git a/pkg/metrics/prom_exporter.go b/pkg/metrics/prom_exporter.go index 6932d0e75e..fe54cbf092 100644 --- a/pkg/metrics/prom_exporter.go +++ b/pkg/metrics/prom_exporter.go @@ -117,7 +117,7 @@ func (pso *PodStatsObj) getLabels(stats *PodStatsType, tuple Tuple, dir string) l[srcKey] = strings.Join(svcList, "|") l[srcTypeKey] = "service" } else { - l[srcKey] = fmt.Sprintf("%s", tuple.srcIP) + l[srcKey] = tuple.srcIP l[srcTypeKey] = "cidr" } } diff --git a/pkg/objdb/etcd_client.go b/pkg/objdb/etcd_client.go index 470a1185f6..83f9ae7e03 100644 --- a/pkg/objdb/etcd_client.go +++ b/pkg/objdb/etcd_client.go @@ -47,9 +47,6 @@ type EtcdClient struct { root string } -// Max retry count -const maxEtcdRetries = 10 - // Initialize the etcd client func NewClient(endpoints []string, root string) (API, error) { var err error @@ -161,11 +158,8 @@ func (ec *EtcdClient) DelObj(key string) error { defer cancel() _, err := ec.kapi.Delete(ctx, keyName, nil) if err != nil { - // Retry few times if cluster is unavailable - if err != nil { - log.Errorf("Error removing key %s, Err: %v", keyName, err) - return err - } + log.Errorf("Error removing key %s, Err: %v", keyName, err) + return err } return nil