From 2fe42c11c7339ce53d68285445f723ad6c5b527d Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Mon, 24 Jun 2019 16:57:03 +0900 Subject: [PATCH] Fix golint error and make it enable again in travis --- .travis.yml | 2 +- checkpoint/checkpoint.go | 5 +++-- k8sclient/k8sclient.go | 19 +++++++++++++------ kubeletclient/kubeletclient.go | 8 ++++---- logging/logging.go | 25 +++++++++++++++++-------- testing/testing.go | 17 ++++++++++++++--- types/conf.go | 10 +++++++--- types/types.go | 6 ++++++ 8 files changed, 65 insertions(+), 27 deletions(-) diff --git a/.travis.yml b/.travis.yml index 42befdfaf..ca048de98 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,7 +24,7 @@ install: before_script: # Make gopath... to run golint/go fmt/go vet # Suppress golint for fixing lint later. - #- golint ./... | grep -v vendor | grep -v ALL_CAPS | xargs -r false + - golint ./... | grep -v vendor | grep -v ALL_CAPS | xargs -r false - go fmt ./... - go vet ./... # - gocyclo -over 15 ./multus diff --git a/checkpoint/checkpoint.go b/checkpoint/checkpoint.go index 98a6b221f..b70f31229 100644 --- a/checkpoint/checkpoint.go +++ b/checkpoint/checkpoint.go @@ -28,6 +28,7 @@ const ( checkPointfile = "/var/lib/kubelet/device-plugins/kubelet_internal_checkpoint" ) +// PodDevicesEntry maps PodUID, resource name and allocated device id type PodDevicesEntry struct { PodUID string ContainerName string @@ -41,7 +42,7 @@ type checkpointData struct { RegisteredDevices map[string][]string } -type Data struct { +type checkpointFileData struct { Data checkpointData Checksum uint64 } @@ -70,7 +71,7 @@ func getCheckpoint(filePath string) (types.ResourceClient, error) { // getPodEntries gets all Pod device allocation entries from checkpoint file func (cp *checkpoint) getPodEntries() error { - cpd := &Data{} + cpd := &checkpointFileData{} rawBytes, err := ioutil.ReadFile(cp.fileName) if err != nil { return logging.Errorf("getPodEntries(): error reading file %s\n%v\n", checkPointfile, err) diff --git a/k8sclient/k8sclient.go b/k8sclient/k8sclient.go index dd2489509..400dfb361 100644 --- a/k8sclient/k8sclient.go +++ b/k8sclient/k8sclient.go @@ -47,7 +47,8 @@ type NoK8sNetworkError struct { message string } -type clientInfo struct { +// ClientInfo contains information given from k8s client +type ClientInfo struct { Client KubeClient Podnamespace string Podname string @@ -74,13 +75,14 @@ func (d *defaultKubeClient) UpdatePodStatus(pod *v1.Pod) (*v1.Pod, error) { return d.client.Core().Pods(pod.Namespace).UpdateStatus(pod) } -func setKubeClientInfo(c *clientInfo, client KubeClient, k8sArgs *types.K8sArgs) { +func setKubeClientInfo(c *ClientInfo, client KubeClient, k8sArgs *types.K8sArgs) { logging.Debugf("setKubeClientInfo: %v, %v, %v", c, client, k8sArgs) c.Client = client c.Podnamespace = string(k8sArgs.K8S_POD_NAMESPACE) c.Podname = string(k8sArgs.K8S_POD_NAME) } +// SetNetworkStatus sets network status into Pod annotation func SetNetworkStatus(client KubeClient, k8sArgs *types.K8sArgs, netStatus []*types.NetworkStatus, conf *types.NetConf) error { logging.Debugf("SetNetworkStatus: %v, %v, %v, %v", client, k8sArgs, netStatus, conf) @@ -392,12 +394,14 @@ func getKubernetesDelegate(client KubeClient, net *types.NetworkSelectionElement return delegate, resourceMap, nil } +// KubeClient is abstraction layer for k8s client (used testing package) type KubeClient interface { GetRawWithPath(path string) ([]byte, error) GetPod(namespace, name string) (*v1.Pod, error) UpdatePodStatus(pod *v1.Pod) (*v1.Pod, error) } +// GetK8sArgs gets k8s related args from CNI args func GetK8sArgs(args *skel.CmdArgs) (*types.K8sArgs, error) { k8sArgs := &types.K8sArgs{} @@ -410,11 +414,11 @@ func GetK8sArgs(args *skel.CmdArgs) (*types.K8sArgs, error) { return k8sArgs, nil } -// Attempts to load Kubernetes-defined delegates and add them to the Multus config. +// TryLoadPodDelegates attempts to load Kubernetes-defined delegates and add them to the Multus config. // Returns the number of Kubernetes-defined delegates added or an error. -func TryLoadPodDelegates(k8sArgs *types.K8sArgs, conf *types.NetConf, kubeClient KubeClient) (int, *clientInfo, error) { +func TryLoadPodDelegates(k8sArgs *types.K8sArgs, conf *types.NetConf, kubeClient KubeClient) (int, *ClientInfo, error) { var err error - clientInfo := &clientInfo{} + clientInfo := &ClientInfo{} logging.Debugf("TryLoadPodDelegates: %v, %v, %v", k8sArgs, conf, kubeClient) kubeClient, err = GetK8sClient(conf.Kubeconfig, kubeClient) @@ -467,6 +471,7 @@ func TryLoadPodDelegates(k8sArgs *types.K8sArgs, conf *types.NetConf, kubeClient return 0, clientInfo, nil } +// GetK8sClient gets client info from kubeconfig func GetK8sClient(kubeconfig string, kubeClient KubeClient) (KubeClient, error) { logging.Debugf("GetK8sClient: %s, %v", kubeconfig, kubeClient) // If we get a valid kubeClient (eg from testcases) just return that @@ -509,6 +514,7 @@ func GetK8sClient(kubeconfig string, kubeClient KubeClient) (KubeClient, error) return &defaultKubeClient{client: client}, nil } +// GetPodNetwork gets net-attach-def annotation from pod func GetPodNetwork(pod *v1.Pod) ([]*types.NetworkSelectionElement, error) { logging.Debugf("GetPodNetwork: %v", pod) @@ -526,6 +532,7 @@ func GetPodNetwork(pod *v1.Pod) ([]*types.NetworkSelectionElement, error) { return networks, nil } +// GetNetworkDelegates returns delegatenetconf from net-attach-def annotation in pod func GetNetworkDelegates(k8sclient KubeClient, pod *v1.Pod, networks []*types.NetworkSelectionElement, confdir string, confnamespaceIsolation bool) ([]*types.DelegateNetConf, error) { logging.Debugf("GetNetworkDelegates: %v, %v, %v, %v, %v", k8sclient, pod, networks, confdir, confnamespaceIsolation) // resourceMap holds Pod device allocation information; only initizized if CRD contains 'resourceName' annotation. @@ -627,7 +634,7 @@ func getNetDelegate(client KubeClient, netname, confdir, namespace string) (*typ return nil, logging.Errorf("getNetDelegate: cannot find network: %v", netname) } -// GetDefaultNetwork parses 'defaultNetwork' config, gets network json and put it into netconf.Delegates. +// GetDefaultNetworks parses 'defaultNetwork' config, gets network json and put it into netconf.Delegates. func GetDefaultNetworks(k8sArgs *types.K8sArgs, conf *types.NetConf, kubeClient KubeClient) error { logging.Debugf("GetDefaultNetworks: %v, %v, %v", k8sArgs, conf, kubeClient) var delegates []*types.DelegateNetConf diff --git a/kubeletclient/kubeletclient.go b/kubeletclient/kubeletclient.go index 4e1df566b..0585150ba 100644 --- a/kubeletclient/kubeletclient.go +++ b/kubeletclient/kubeletclient.go @@ -30,12 +30,12 @@ func GetResourceClient() (types.ResourceClient, error) { // If Kubelet resource API endpoint exist use that by default // Or else fallback with checkpoint file if hasKubeletAPIEndpoint() { - logging.Printf(logging.VerboseLevel, "GetResourceClient(): using Kubelet resource API endpoint") + logging.Verbosef("GetResourceClient(): using Kubelet resource API endpoint") return getKubeletClient() - } else { - logging.Printf(logging.VerboseLevel, "GetResourceClient(): using Kubelet device plugin checkpoint") - return checkpoint.GetCheckpoint() } + + logging.Verbosef("GetResourceClient(): using Kubelet device plugin checkpoint") + return checkpoint.GetCheckpoint() } func getKubeletClient() (types.ResourceClient, error) { diff --git a/logging/logging.go b/logging/logging.go index 302dcea3e..895f012e0 100644 --- a/logging/logging.go +++ b/logging/logging.go @@ -26,6 +26,7 @@ import ( // Level type type Level uint32 +// PanicLevel...MaxLevel indicates the logging level const ( PanicLevel Level = iota ErrorLevel @@ -55,7 +56,7 @@ func (l Level) String() string { return "unknown" } -func Printf(level Level, format string, a ...interface{}) { +func printf(level Level, format string, a ...interface{}) { header := "%s [%s] " t := time.Now() if level > loggingLevel { @@ -75,26 +76,31 @@ func Printf(level Level, format string, a ...interface{}) { } } +// Debugf prints logging if logging level >= debug func Debugf(format string, a ...interface{}) { - Printf(DebugLevel, format, a...) + printf(DebugLevel, format, a...) } +// Verbosef prints logging if logging level >= verbose func Verbosef(format string, a ...interface{}) { - Printf(VerboseLevel, format, a...) + printf(VerboseLevel, format, a...) } +// Errorf prints logging if logging level >= error func Errorf(format string, a ...interface{}) error { - Printf(ErrorLevel, format, a...) + printf(ErrorLevel, format, a...) return fmt.Errorf(format, a...) } +// Panicf prints logging plus stack trace. This should be used only for unrecoverble error func Panicf(format string, a ...interface{}) { - Printf(PanicLevel, format, a...) - Printf(PanicLevel, "========= Stack trace output ========") - Printf(PanicLevel, "%+v", errors.New("Multus Panic")) - Printf(PanicLevel, "========= Stack trace output end ========") + printf(PanicLevel, format, a...) + printf(PanicLevel, "========= Stack trace output ========") + printf(PanicLevel, "%+v", errors.New("Multus Panic")) + printf(PanicLevel, "========= Stack trace output end ========") } +// GetLoggingLevel gets current logging level func GetLoggingLevel() Level { return loggingLevel } @@ -114,6 +120,7 @@ func getLoggingLevel(levelStr string) Level { return UnknownLevel } +// SetLogLevel sets logging level func SetLogLevel(levelStr string) { level := getLoggingLevel(levelStr) if level < MaxLevel { @@ -121,10 +128,12 @@ func SetLogLevel(levelStr string) { } } +// SetLogStderr sets flag for logging stderr output func SetLogStderr(enable bool) { loggingStderr = enable } +// SetLogFile sets logging file func SetLogFile(filename string) { if filename == "" { return diff --git a/testing/testing.go b/testing/testing.go index 35652e121..7f0db694e 100644 --- a/testing/testing.go +++ b/testing/testing.go @@ -24,9 +24,10 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - . "github.com/onsi/gomega" + "github.com/onsi/gomega" ) +// FakeKubeClient is stub KubeClient for testing type FakeKubeClient struct { pods map[string]*v1.Pod PodCount int @@ -34,6 +35,7 @@ type FakeKubeClient struct { NetCount int } +// NewFakeKubeClient creates FakeKubeClient for testing func NewFakeKubeClient() *FakeKubeClient { return &FakeKubeClient{ pods: make(map[string]*v1.Pod), @@ -41,6 +43,7 @@ func NewFakeKubeClient() *FakeKubeClient { } } +// GetRawWithPath returns k8s raw data from its path func (f *FakeKubeClient) GetRawWithPath(path string) ([]byte, error) { obj, ok := f.nets[path] if !ok { @@ -50,6 +53,7 @@ func (f *FakeKubeClient) GetRawWithPath(path string) ([]byte, error) { return []byte(obj), nil } +// AddNetConfig adds net-attach-def into its client func (f *FakeKubeClient) AddNetConfig(namespace, name, data string) { cr := fmt.Sprintf(`{ "apiVersion": "k8s.cni.cncf.io/v1", @@ -67,6 +71,7 @@ func (f *FakeKubeClient) AddNetConfig(namespace, name, data string) { f.nets[fmt.Sprintf("/apis/k8s.cni.cncf.io/v1/namespaces/%s/network-attachment-definitions/%s", namespace, name)] = cr } +// AddNetFile puts config file as net-attach-def func (f *FakeKubeClient) AddNetFile(namespace, name, filePath, fileData string) { cr := fmt.Sprintf(`{ "apiVersion": "k8s.cni.cncf.io/v1", @@ -79,9 +84,10 @@ func (f *FakeKubeClient) AddNetFile(namespace, name, filePath, fileData string) f.nets[fmt.Sprintf("/apis/k8s.cni.cncf.io/v1/namespaces/%s/network-attachment-definitions/%s", namespace, name)] = cr err := ioutil.WriteFile(filePath, []byte(fileData), 0600) - Expect(err).NotTo(HaveOccurred()) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) } +// GetPod query pod by namespace/pod and return it if exists func (f *FakeKubeClient) GetPod(namespace, name string) (*v1.Pod, error) { key := fmt.Sprintf("%s/%s", namespace, name) pod, ok := f.pods[key] @@ -92,22 +98,26 @@ func (f *FakeKubeClient) GetPod(namespace, name string) (*v1.Pod, error) { return pod, nil } +// UpdatePodStatus update pod status func (f *FakeKubeClient) UpdatePodStatus(pod *v1.Pod) (*v1.Pod, error) { key := fmt.Sprintf("%s/%s", pod.Namespace, pod.Name) f.pods[key] = pod return f.pods[key], nil } +// AddPod adds pod into fake client func (f *FakeKubeClient) AddPod(pod *v1.Pod) { key := fmt.Sprintf("%s/%s", pod.ObjectMeta.Namespace, pod.ObjectMeta.Name) f.pods[key] = pod } +// DeletePod remove pod from fake client func (f *FakeKubeClient) DeletePod(pod *v1.Pod) { key := fmt.Sprintf("%s/%s", pod.ObjectMeta.Namespace, pod.ObjectMeta.Name) delete(f.pods, key) } +// NewFakePod creates fake Pod object func NewFakePod(name string, netAnnotation string, defaultNetAnnotation string) *v1.Pod { pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -136,9 +146,10 @@ func NewFakePod(name string, netAnnotation string, defaultNetAnnotation string) return pod } +// EnsureCIDR parses/verify CIDR ip string and convert to net.IPNet func EnsureCIDR(cidr string) *net.IPNet { ip, net, err := net.ParseCIDR(cidr) - Expect(err).NotTo(HaveOccurred()) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) net.IP = ip return net } diff --git a/types/conf.go b/types/conf.go index a4e2e8d1c..7d8ce8e09 100644 --- a/types/conf.go +++ b/types/conf.go @@ -34,9 +34,10 @@ const ( defaultMultusNamespace = "kube-system" ) +// LoadDelegateNetConfList reads DelegateNetConf from bytes func LoadDelegateNetConfList(bytes []byte, delegateConf *DelegateNetConf) error { - logging.Debugf("LoadDelegateNetConfList: %s, %v", string(bytes), delegateConf) + if err := json.Unmarshal(bytes, &delegateConf.ConfList); err != nil { return logging.Errorf("err in unmarshalling delegate conflist: %v", err) } @@ -51,7 +52,7 @@ func LoadDelegateNetConfList(bytes []byte, delegateConf *DelegateNetConf) error return nil } -// Convert raw CNI JSON into a DelegateNetConf structure +// LoadDelegateNetConf converts raw CNI JSON into a DelegateNetConf structure func LoadDelegateNetConf(bytes []byte, net *NetworkSelectionElement, deviceID string) (*DelegateNetConf, error) { var err error logging.Debugf("LoadDelegateNetConf: %s, %v, %s", string(bytes), net, deviceID) @@ -98,9 +99,10 @@ func LoadDelegateNetConf(bytes []byte, net *NetworkSelectionElement, deviceID st return delegateConf, nil } +// CreateCNIRuntimeConf create CNI RuntimeConf func CreateCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, rc *RuntimeConfig) *libcni.RuntimeConf { - logging.Debugf("LoadCNIRuntimeConf: %v, %v, %s, %v", args, k8sArgs, ifName, rc) + // In part, adapted from K8s pkg/kubelet/dockershim/network/cni/cni.go#buildCNIRuntimeConf // Todo // ingress, egress and bandwidth capability features as same as kubelet. @@ -124,6 +126,7 @@ func CreateCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, r return rt } +// LoadNetworkStatus create network status from CNI result func LoadNetworkStatus(r types.Result, netName string, defaultNet bool) (*NetworkStatus, error) { logging.Debugf("LoadNetworkStatus: %v, %s, %t", r, netName, defaultNet) @@ -162,6 +165,7 @@ func LoadNetworkStatus(r types.Result, netName string, defaultNet bool) (*Networ } +// LoadNetConf converts inputs (i.e. stdin) to NetConf func LoadNetConf(bytes []byte) (*NetConf, error) { netconf := &NetConf{} diff --git a/types/types.go b/types/types.go index 398055655..91d716e7a 100644 --- a/types/types.go +++ b/types/types.go @@ -56,10 +56,12 @@ type NetConf struct { MultusNamespace string `json:"multusNamespace"` } +// RuntimeConfig specifies CNI RuntimeConfig type RuntimeConfig struct { PortMaps []PortMapEntry `json:"portMappings,omitempty"` } +// PortMapEntry for CNI PortMapEntry type PortMapEntry struct { HostPort int `json:"hostPort"` ContainerPort int `json:"containerPort"` @@ -67,6 +69,7 @@ type PortMapEntry struct { HostIP string `json:"hostIP,omitempty"` } +// NetworkStatus is for network status annotation for pod type NetworkStatus struct { Name string `json:"name"` Interface string `json:"interface,omitempty"` @@ -76,6 +79,7 @@ type NetworkStatus struct { DNS types.DNS `json:"dns,omitempty"` } +// DelegateNetConf for net-attach-def for pod type DelegateNetConf struct { Conf types.NetConf ConfList types.NetConfList @@ -91,6 +95,7 @@ type DelegateNetConf struct { Bytes []byte } +// NetworkAttachmentDefinition represents net-attach-def of K8s NPWG spec type NetworkAttachmentDefinition struct { metav1.TypeMeta `json:",inline"` // Note that ObjectMeta is mandatory, as an object @@ -107,6 +112,7 @@ type NetworkAttachmentDefinition struct { Spec NetworkAttachmentDefinitionSpec `json:"spec"` } +// NetworkAttachmentDefinitionSpec represents net-attach-def spec of K8s NPWG spec type NetworkAttachmentDefinitionSpec struct { // Config contains a standard JSON-encoded CNI configuration // or configuration list which defines the plugin chain to