From 71f9d8874f8d831ebf558d31c0545ffed64b748e Mon Sep 17 00:00:00 2001 From: Alessandro Puccetti Date: Wed, 31 Aug 2016 16:42:39 +0200 Subject: [PATCH] plugins/traffic-control: Some style fixes Comment before exported functions. When `if` block ends with a return statement, drop the else and outdent its block. --- examples/plugins/traffic-control/docker.go | 3 + examples/plugins/traffic-control/main.go | 5 ++ examples/plugins/traffic-control/report.go | 15 +++-- examples/plugins/traffic-control/store.go | 12 ++++ examples/plugins/traffic-control/tc.go | 74 ++++++++++------------ 5 files changed, 63 insertions(+), 46 deletions(-) diff --git a/examples/plugins/traffic-control/docker.go b/examples/plugins/traffic-control/docker.go index 07852c27b..492427d7b 100644 --- a/examples/plugins/traffic-control/docker.go +++ b/examples/plugins/traffic-control/docker.go @@ -8,11 +8,13 @@ import ( docker "github.com/fsouza/go-dockerclient" ) +// DockerClient internal data structure type DockerClient struct { store *Store client *docker.Client } +// NewDockerClient instantiates a new DockerClient func NewDockerClient(store *Store) (*DockerClient, error) { dc, err := docker.NewClient("unix:///var/run/docker.sock") if err != nil { @@ -24,6 +26,7 @@ func NewDockerClient(store *Store) (*DockerClient, error) { }, nil } +// Start docker client func (c *DockerClient) Start() { for { c.loopIteration() diff --git a/examples/plugins/traffic-control/main.go b/examples/plugins/traffic-control/main.go index 8339b87ee..162d90f58 100644 --- a/examples/plugins/traffic-control/main.go +++ b/examples/plugins/traffic-control/main.go @@ -40,10 +40,13 @@ import ( // // port to eBPF? +// TODO alepuccetti: write meaningful comments + type containerClient interface { Start() } +// Plugin is the internal data structure type Plugin struct { reporter *Reporter @@ -110,6 +113,7 @@ func setupSocket(socket string) (net.Listener, error) { return listener, nil } +// NewPlugin instantiates a new plugin func NewPlugin() (*Plugin, error) { store := NewStore() dockerClient, err := NewDockerClient(store) @@ -129,6 +133,7 @@ func NewPlugin() (*Plugin, error) { return plugin, nil } +// Serve is a wrapper to http.ServeMux to serve the request supported by the plugin func (p *Plugin) Serve(listener net.Listener) error { http.HandleFunc("/report", p.report) http.HandleFunc("/control", p.control) diff --git a/examples/plugins/traffic-control/report.go b/examples/plugins/traffic-control/report.go index cdda383e7..3c0e5322d 100644 --- a/examples/plugins/traffic-control/report.go +++ b/examples/plugins/traffic-control/report.go @@ -72,16 +72,19 @@ type pluginSpec struct { APIVersion string `json:"api_version,omitempty"` } +// Reporter internal data structure type Reporter struct { store *Store } +// NewReporter instantiates a new Reporter func NewReporter(store *Store) *Reporter { return &Reporter{ store: store, } } +// RawReport returns a report func (r *Reporter) RawReport() ([]byte, error) { rpt := &report{ Container: topology{ @@ -107,14 +110,15 @@ func (r *Reporter) RawReport() ([]byte, error) { return raw, nil } +// GetHandler returns the function performing the action specified by controlID func (r *Reporter) GetHandler(nodeID, controlID string) (func() error, error) { containerID, err := nodeIDToContainerID(nodeID) if err != nil { - return nil, fmt.Errorf("failed to get container ID from node ID %q: %v", nodeID) + return nil, fmt.Errorf("failed to get container ID from node ID %q: %v", nodeID, err) } container, found := r.store.Container(containerID) if !found { - return nil, fmt.Errorf("container %s not found") + return nil, fmt.Errorf("container %s not found", containerID) } var handler func(pid int) error for _, c := range getControls() { @@ -134,7 +138,6 @@ func (r *Reporter) GetHandler(nodeID, controlID string) (func() error, error) { // states: // created, destroyed - don't create any node // running, not running - create node with controls - func (r *Reporter) getContainerNodes() map[string]node { nodes := map[string]node{} timestamp := time.Now() @@ -171,7 +174,7 @@ func (r *Reporter) getContainerNodes() map[string]node { func getMetadataTemplate() map[string]metadataTemplate { return map[string]metadataTemplate{ - "traffic-control-latency": metadataTemplate{ + "traffic-control-latency": { ID: "traffic-control-latency", Label: "Latency", Truncate: 0, @@ -179,7 +182,7 @@ func getMetadataTemplate() map[string]metadataTemplate { Priority: 13.5, From: "latest", }, - "traffic-control-pktloss": metadataTemplate{ + "traffic-control-pktloss": { ID: "traffic-control-pktloss", Label: "Packet Loss", Truncate: 0, @@ -192,7 +195,7 @@ func getMetadataTemplate() map[string]metadataTemplate { func getTableTemplate() map[string]tableTemplate { return map[string]tableTemplate{ - "traffic-control-table": tableTemplate{ + "traffic-control-table": { ID: "traffic-control-table", Label: "Traffic Control", Prefix: trafficControlTablePrefix, diff --git a/examples/plugins/traffic-control/store.go b/examples/plugins/traffic-control/store.go index c8b60db4c..65c27d0ce 100644 --- a/examples/plugins/traffic-control/store.go +++ b/examples/plugins/traffic-control/store.go @@ -4,31 +4,40 @@ import ( "sync" ) +// State is the container internal state type State int const ( + // Created state Created State = iota + // Running state Running + // Stopped state Stopped + // Destroyed state Destroyed ) +// Container data structure type Container struct { State State PID int } +// Store data structure type Store struct { lock sync.Mutex containers map[string]Container } +// NewStore instantiates a new Store func NewStore() *Store { return &Store{ containers: map[string]Container{}, } } +// Container returns a container form its ID func (s *Store) Container(containerID string) (Container, bool) { s.lock.Lock() defer s.lock.Unlock() @@ -36,18 +45,21 @@ func (s *Store) Container(containerID string) (Container, bool) { return container, found } +// SetContainer sets a container into the store func (s *Store) SetContainer(containerID string, container Container) { s.lock.Lock() defer s.lock.Unlock() s.containers[containerID] = container } +// DeleteContainer deletes a container from the store func (s *Store) DeleteContainer(containerID string) { s.lock.Lock() defer s.lock.Unlock() delete(s.containers, containerID) } +// ForEach execute a function on each container in the store func (s *Store) ForEach(callback func(ID string, c Container)) { s.lock.Lock() defer s.lock.Unlock() diff --git a/examples/plugins/traffic-control/tc.go b/examples/plugins/traffic-control/tc.go index b3b245030..e2bd9505e 100644 --- a/examples/plugins/traffic-control/tc.go +++ b/examples/plugins/traffic-control/tc.go @@ -84,24 +84,24 @@ func DoTrafficControl(pid int, latency string, pktLoss string) error { return fmt.Errorf("failed to perform traffic control: %v", err) } // cache parameters - if netNSID, err := getNSID(netNS); err != nil { + netNSID, err := getNSID(netNS) + if err != nil { log.Error(netNSID) return fmt.Errorf("failed to get network namespace ID: %v", err) - } else { - trafficControlStatusCache[netNSID] = trafficControlStatus{ - latency: func(latency string) string { - if latency == "" { - return "-" - } - return latency - }(latency), - pktLoss: func(pktLoss string) string { - if pktLoss == "" { - return "-" - } - return pktLoss - }(pktLoss), - } + } + trafficControlStatusCache[netNSID] = trafficControlStatus{ + latency: func(latency string) string { + if latency == "" { + return "-" + } + return latency + }(latency), + pktLoss: func(pktLoss string) string { + if pktLoss == "" { + return "-" + } + return pktLoss + }(pktLoss), } return nil } @@ -141,15 +141,15 @@ func ClearTrafficControlSettings(pid int) error { return fmt.Errorf("failed to perform traffic control: %v", err) } // clear cached parameters - if netNSID, err := getNSID(netNS); err != nil { + netNSID, err := getNSID(netNS) + if err != nil { log.Error(netNSID) return fmt.Errorf("failed to get network namespace ID: %v", err) - } else { - //trafficControlStatusCache[netNSID] = trafficControlStatus{ - // latency: "-", - // pktLoss: "-", - delete(trafficControlStatusCache, netNSID) } + //trafficControlStatusCache[netNSID] = trafficControlStatus{ + // latency: "-", + // pktLoss: "-", + delete(trafficControlStatusCache, netNSID) return nil } @@ -188,26 +188,21 @@ func getStatus(pid int) (*trafficControlStatus, error) { cmd := split("tc qdisc show dev eth0") var output string err = ns.WithNetNSPath(netNS, func(hostNS ns.NetNS) error { - if cmdOut, err := exec.Command(cmd[0], cmd[1:]...).CombinedOutput(); err != nil { + cmdOut, err := exec.Command(cmd[0], cmd[1:]...).CombinedOutput() + if err != nil { log.Error(string(cmdOut)) output = "-" return fmt.Errorf("failed to execute command: tc qdisc show dev eth0: %v", err) - } else { - output = string(cmdOut) } + output = string(cmdOut) return nil }) // cache parameters - if netNSID, err := getNSID(netNS); err != nil { - log.Error(netNSID) - return &emptyTrafficControlStatus, fmt.Errorf("failed to get network namespace ID: %v", err) - } else { - lat, _ := parseLatency(output) - pktL, _ := parsePktLoss(output) - trafficControlStatusCache[netNSID] = trafficControlStatus{ - latency: lat, - pktLoss: pktL, - } + lat, _ := parseLatency(output) + pktL, _ := parsePktLoss(output) + trafficControlStatusCache[netNSID] = trafficControlStatus{ + latency: lat, + pktLoss: pktL, } status, _ := trafficControlStatusCache[netNSID] return &status, err @@ -226,21 +221,20 @@ func parseAttribute(statusString string, attribute string) (string, error) { if s == attribute { if i < len(statusStringSplited)-1 { return strings.Trim(statusStringSplited[i+1], "\n"), nil - } else { - return "-", nil } + return "-", nil } } return "-", fmt.Errorf("%s not found", attribute) } func getNSID(nsPath string) (string, error) { - if nsID, err := os.Readlink(nsPath); err != nil { + nsID, err := os.Readlink(nsPath) + if err != nil { log.Error(nsID) return "", fmt.Errorf("failed to execute command: tc qdisc show dev eth0: %v", err) - } else { - return nsID[5 : len(nsID)-1], nil } + return nsID[5 : len(nsID)-1], nil } func split(cmd string) []string {