plugins/traffic-control: code refactoring

Substitute `split()` with `strings.Fileds()`.
Remove unnecessary functions and commented code.
Unabbreviate variable and function names.
This commit is contained in:
Alessandro Puccetti
2016-09-02 09:30:38 +02:00
parent 9b86143deb
commit 00a87922e6
3 changed files with 45 additions and 81 deletions

View File

@@ -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)

View File

@@ -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() {

View File

@@ -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
}