From 00a87922e69fce585d519f9295a08fe779558486 Mon Sep 17 00:00:00 2001 From: Alessandro Puccetti Date: Fri, 2 Sep 2016 09:30:38 +0200 Subject: [PATCH] plugins/traffic-control: code refactoring Substitute `split()` with `strings.Fileds()`. Remove unnecessary functions and commented code. Unabbreviate variable and function names. --- examples/plugins/traffic-control/main.go | 21 ++--- examples/plugins/traffic-control/report.go | 10 +-- examples/plugins/traffic-control/tc.go | 95 +++++++--------------- 3 files changed, 45 insertions(+), 81 deletions(-) diff --git a/examples/plugins/traffic-control/main.go b/examples/plugins/traffic-control/main.go index 162d90f58..25c257799 100644 --- a/examples/plugins/traffic-control/main.go +++ b/examples/plugins/traffic-control/main.go @@ -40,8 +40,6 @@ import ( // // port to eBPF? -// TODO alepuccetti: write meaningful comments - type containerClient interface { Start() } @@ -54,8 +52,14 @@ type Plugin struct { } type trafficControlStatus struct { - latency string - pktLoss string + latency string + packetLoss string +} + +// String is useful to easily create a string of the traffic control plugin internal status. +// Useful for debugging +func (tcs trafficControlStatus) String() string { + return fmt.Sprintf("%s %s", tcs.latency, tcs.packetLoss) } var trafficControlStatusCache map[string]trafficControlStatus @@ -64,16 +68,13 @@ var emptyTrafficControlStatus trafficControlStatus func main() { const socket = "/var/run/scope/plugins/traffic-control.sock" + // Handle the exit signal setupSignals(socket) listener, err := setupSocket(socket) if err != nil { log.Fatalf("Failed to setup socket: %v", err) } - defer func() { - listener.Close() - os.Remove(socket) - }() plugin, err := NewPlugin() if err != nil { @@ -81,8 +82,8 @@ func main() { } trafficControlStatusCache = make(map[string]trafficControlStatus) emptyTrafficControlStatus = trafficControlStatus{ - latency: "-", - pktLoss: "-", + latency: "-", + packetLoss: "-", } if err := plugin.Serve(listener); err != nil { log.Fatalf("failed to serve: %v", err) diff --git a/examples/plugins/traffic-control/report.go b/examples/plugins/traffic-control/report.go index 50e7cc958..aa6af76d7 100644 --- a/examples/plugins/traffic-control/report.go +++ b/examples/plugins/traffic-control/report.go @@ -153,7 +153,7 @@ func (r *Reporter) getContainerNodes() map[string]node { case Running: nodeID := containerIDToNodeID(containerID) latency, _ := getLatency(container.PID) - pktLoss, _ := getPktLoss(container.PID) + packetLoss, _ := getPacketLoss(container.PID) nodes[nodeID] = node{ LatestControls: getTrafficNodeControls(timestamp, dead), Latest: map[string]stringEntry{ @@ -163,7 +163,7 @@ func (r *Reporter) getContainerNodes() map[string]node { }, fmt.Sprintf("%s%s", trafficControlTablePrefix, "pktloss"): { Timestamp: timestamp, - Value: pktLoss, + Value: packetLoss, }, }, } @@ -268,7 +268,7 @@ func getLatencyControls() []extControl { } } -func getPktLossControls() []extControl { +func getPacketLossControls() []extControl { return []extControl{ { control: control{ @@ -302,8 +302,8 @@ func getGeneralControls() []extControl { func getControls() []extControl { controls := getLatencyControls() - // TODO alepuccetti why append(controls, getPktLossControls()) does not work? - for _, ctrl := range getPktLossControls() { + // TODO alepuccetti why append(controls, getPacketLossControls()) does not work? + for _, ctrl := range getPacketLossControls() { controls = append(controls, ctrl) } for _, ctrl := range getGeneralControls() { diff --git a/examples/plugins/traffic-control/tc.go b/examples/plugins/traffic-control/tc.go index e2bd9505e..dfbce8c09 100644 --- a/examples/plugins/traffic-control/tc.go +++ b/examples/plugins/traffic-control/tc.go @@ -11,25 +11,25 @@ import ( ) // DoTrafficControl is the function that set the parameters of the qdisc with tc -func DoTrafficControl(pid int, latency string, pktLoss string) error { - if latency == "" && pktLoss == "" { +func DoTrafficControl(pid int, latency string, packetLoss string) error { + if latency == "" && packetLoss == "" { // TODO @alepuccetti: return a warning message: "Nothing to do" return nil } var err error cmds := [][]string{ - split("tc qdisc replace dev eth0 root handle 1: netem"), + strings.Fields("tc qdisc replace dev eth0 root handle 1: netem"), // These steps are not required, since we don't do // ingress traffic control, only egress, see the TODO // at the beginning of the file. - //split("ip link add ifb0 type ifb"), - //split("ip link set ifb0 up"), - //split("tc qdisc add dev eth0 handle ffff: ingress"), - //split("tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0 action mirred egress redirect dev ifb0"), - //split("tc qdisc replace dev ifb0 handle 1:0 root netem"), + //strings.Fields("ip link add ifb0 type ifb"), + //strings.Fields("ip link set ifb0 up"), + //strings.Fields("tc qdisc add dev eth0 handle ffff: ingress"), + //strings.Fields("tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0 action mirred egress redirect dev ifb0"), + //strings.Fields("tc qdisc replace dev ifb0 handle 1:0 root netem"), // Add "loss %d%% rate %dkbit" when we add the // possibility to control the packet loss and @@ -37,12 +37,12 @@ func DoTrafficControl(pid int, latency string, pktLoss string) error { // file. } - cmd := split("tc qdisc change dev eth0 root handle 1: netem") + cmd := strings.Fields("tc qdisc change dev eth0 root handle 1: netem") // TODO @alepuccetti: refactor this code if latency == "" { - // pktLoss cannot be empty + // packetLoss cannot be empty cmd = append(cmd, "loss") - cmd = append(cmd, pktLoss) + cmd = append(cmd, packetLoss) // get latency from the cache if latency, err = getLatency(pid); err != nil { return err @@ -50,23 +50,23 @@ func DoTrafficControl(pid int, latency string, pktLoss string) error { cmd = append(cmd, "delay") cmd = append(cmd, latency) } - } else if pktLoss == "" { + } else if packetLoss == "" { // latency cannot be empty cmd = append(cmd, "delay") cmd = append(cmd, latency) - // get pktLoss from the cache - if pktLoss, err = getPktLoss(pid); err != nil { + // get packetLoss from the cache + if packetLoss, err = getPacketLoss(pid); err != nil { return err - } else if pktLoss != "-" { + } else if packetLoss != "-" { cmd = append(cmd, "loss") - cmd = append(cmd, pktLoss) + cmd = append(cmd, packetLoss) } } else { // latency and pckLoss are both new cmd = append(cmd, "delay") cmd = append(cmd, latency) cmd = append(cmd, "loss") - cmd = append(cmd, pktLoss) + cmd = append(cmd, packetLoss) } cmds = append(cmds, cmd) @@ -96,12 +96,12 @@ func DoTrafficControl(pid int, latency string, pktLoss string) error { } return latency }(latency), - pktLoss: func(pktLoss string) string { - if pktLoss == "" { + packetLoss: func(packetLoss string) string { + if packetLoss == "" { return "-" } - return pktLoss - }(pktLoss), + return packetLoss + }(packetLoss), } return nil } @@ -109,23 +109,7 @@ func DoTrafficControl(pid int, latency string, pktLoss string) error { // ClearTrafficControlSettings clear all parameters of the qdisc with tc func ClearTrafficControlSettings(pid int) error { cmds := [][]string{ - split("tc qdisc replace dev eth0 root handle 1: netem"), - - // These steps are not required, since we don't do - // ingress traffic control, only egress, see the TODO - // at the beginning of the file. - - //split("ip link add ifb0 type ifb"), - //split("ip link set ifb0 up"), - //split("tc qdisc add dev eth0 handle ffff: ingress"), - //split("tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0 action mirred egress redirect dev ifb0"), - //split("tc qdisc replace dev ifb0 handle 1:0 root netem"), - - // Add "loss %d%% rate %dkbit" when we add the - // possibility to control the packet loss and - // bandwidth. See the TODO at the beginning of the - // file. - + strings.Fields("tc qdisc replace dev eth0 root handle 1: netem"), } netNS := fmt.Sprintf("/proc/%d/ns/net", pid) err := ns.WithNetNSPath(netNS, func(hostNS ns.NetNS) error { @@ -146,9 +130,6 @@ func ClearTrafficControlSettings(pid int) error { log.Error(netNSID) return fmt.Errorf("failed to get network namespace ID: %v", err) } - //trafficControlStatusCache[netNSID] = trafficControlStatus{ - // latency: "-", - // pktLoss: "-", delete(trafficControlStatusCache, netNSID) return nil } @@ -164,7 +145,7 @@ func getLatency(pid int) (string, error) { return status.latency, nil } -func getPktLoss(pid int) (string, error) { +func getPacketLoss(pid int) (string, error) { var status *trafficControlStatus var err error if status, err = getStatus(pid); err != nil { @@ -172,7 +153,7 @@ func getPktLoss(pid int) (string, error) { } else if status == nil { return "-", fmt.Errorf("status for PID %d does not exist", pid) } - return status.pktLoss, nil + return status.packetLoss, nil } func getStatus(pid int) (*trafficControlStatus, error) { @@ -185,7 +166,7 @@ func getStatus(pid int) (*trafficControlStatus, error) { if status, ok := trafficControlStatusCache[netNSID]; ok { return &status, nil } - cmd := split("tc qdisc show dev eth0") + cmd := strings.Fields("tc qdisc show dev eth0") var output string err = ns.WithNetNSPath(netNS, func(hostNS ns.NetNS) error { cmdOut, err := exec.Command(cmd[0], cmd[1:]...).CombinedOutput() @@ -198,11 +179,9 @@ func getStatus(pid int) (*trafficControlStatus, error) { return nil }) // cache parameters - lat, _ := parseLatency(output) - pktL, _ := parsePktLoss(output) trafficControlStatusCache[netNSID] = trafficControlStatus{ - latency: lat, - pktLoss: pktL, + latency: parseLatency(output), + packetLoss: parsePacketLoss(output), } status, _ := trafficControlStatusCache[netNSID] return &status, err @@ -212,11 +191,11 @@ func parseLatency(statusString string) (string, error) { return parseAttribute(statusString, "delay") } -func parsePktLoss(statusString string) (string, error) { +func parsePacketLoss(statusString string) (string, error) { return parseAttribute(statusString, "loss") } func parseAttribute(statusString string, attribute string) (string, error) { - statusStringSplited := split(statusString) + statusStringSplited := strings.Fields(statusString) for i, s := range statusStringSplited { if s == attribute { if i < len(statusStringSplited)-1 { @@ -236,19 +215,3 @@ func getNSID(nsPath string) (string, error) { } return nsID[5 : len(nsID)-1], nil } - -func split(cmd string) []string { - return strings.Split(cmd, " ") -} - -func printTrafficControlStatusCache() { - log.Info(stringTrafficControlStatusCache()) -} - -func stringTrafficControlStatusCache() string { - output := "" - for key, val := range trafficControlStatusCache { - output = fmt.Sprintf("\n%s %s %s %s \n", output, key, val.latency, val.pktLoss) - } - return output -}