Skip to content

Commit

Permalink
refactor: use ovs.Exec replace raw command
Browse files Browse the repository at this point in the history
  • Loading branch information
@oilbeater
oilbeater committed Jun 2, 2020
1 parent ce5216f commit bd97576
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 31 deletions.
37 changes: 16 additions & 21 deletions pkg/daemon/ovs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@ func (csh cniServerHandler) configureNic(podName, podNamespace, netns, container
ifaceID := fmt.Sprintf("%s.%s", podName, podNamespace)
ovs.CleanDuplicatePort(ifaceID)
// Add veth pair host end to ovs port
output, err := exec.Command(
"ovs-vsctl", "--may-exist", "add-port", "br-int", hostNicName, "--",
"set", "interface", hostNicName, fmt.Sprintf("external_ids:iface-id=%s", ifaceID)).CombinedOutput()
output, err := ovs.Exec("--may-exist", "add-port", "br-int", hostNicName, "--",
"set", "interface", hostNicName, fmt.Sprintf("external_ids:iface-id=%s", ifaceID))
if err != nil {
return fmt.Errorf("add nic to ovs failed %v: %q", err, output)
}
Expand Down Expand Up @@ -71,7 +70,7 @@ func (csh cniServerHandler) configureNic(podName, podNamespace, netns, container
func (csh cniServerHandler) deleteNic(podName, podNamespace, containerID string) error {
hostNicName, _ := generateNicName(containerID)
// Remove ovs port
output, err := exec.Command("ovs-vsctl", "--if-exists", "--with-iface", "del-port", "br-int", hostNicName).CombinedOutput()
output, err := ovs.Exec("--if-exists", "--with-iface", "del-port", "br-int", hostNicName)
if err != nil {
return fmt.Errorf("failed to delete ovs port %v, %q", err, output)
}
Expand Down Expand Up @@ -213,10 +212,9 @@ func waiteNetworkReady(gateway string) error {
}

func configureNodeNic(portName, ip, gw string, macAddr net.HardwareAddr, mtu int) error {
raw, err := exec.Command(
"ovs-vsctl", "--may-exist", "add-port", "br-int", util.NodeNic, "--",
raw, err := ovs.Exec("--may-exist", "add-port", "br-int", util.NodeNic, "--",
"set", "interface", util.NodeNic, "type=internal", "--",
"set", "interface", util.NodeNic, fmt.Sprintf("external_ids:iface-id=%s", portName)).CombinedOutput()
"set", "interface", util.NodeNic, fmt.Sprintf("external_ids:iface-id=%s", portName))
if err != nil {
klog.Errorf("failed to configure node nic %s %q", portName, raw)
return fmt.Errorf(string(raw))
Expand All @@ -239,13 +237,12 @@ func configureNodeNic(portName, ip, gw string, macAddr net.HardwareAddr, mtu int
}

func configureMirror(portName string, mtu int) error {
raw, err := exec.Command(
"ovs-vsctl", "--may-exist", "add-port", "br-int", portName, "--",
raw, err := ovs.Exec("--may-exist", "add-port", "br-int", portName, "--",
"set", "interface", portName, "type=internal", "--",
"clear", "bridge", "br-int", "mirrors", "--",
"--id=@mirror0", "get", "port", portName, "--",
"--id=@m", "create", "mirror", "name=m0", "select_all=true", "output_port=@mirror0", "--",
"add", "bridge", "br-int", "mirrors", "@m").CombinedOutput()
"add", "bridge", "br-int", "mirrors", "@m")
if err != nil {
klog.Errorf("failed to configure mirror nic %s %q", portName, raw)
return fmt.Errorf(string(raw))
Expand Down Expand Up @@ -301,22 +298,20 @@ func configureNic(link, ip string, macAddr net.HardwareAddr, mtu int) error {
}

func configProviderPort(providerInterfaceName string) error {
output, err := exec.Command("ovs-vsctl", "--may-exist", "add-br", "br-provider", "--",
"set", "open", ".", fmt.Sprintf("external-ids:ovn-bridge-mappings=%s:br-provider", providerInterfaceName)).CombinedOutput()
output, err := ovs.Exec("--may-exist", "add-br", "br-provider", "--",
"set", "open", ".", fmt.Sprintf("external-ids:ovn-bridge-mappings=%s:br-provider", providerInterfaceName))
if err != nil {
return fmt.Errorf("failed to create bridge br-provider, %v: %q", err, output)
}

output, err = exec.Command(
"ovs-vsctl", "--may-exist", "add-port", "br-provider", "provider-int", "--",
"set", "interface", "provider-int", "type=patch", "options:peer=int-provider").CombinedOutput()
output, err = ovs.Exec("--may-exist", "add-port", "br-provider", "provider-int", "--",
"set", "interface", "provider-int", "type=patch", "options:peer=int-provider")
if err != nil {
return fmt.Errorf("failed to create patch port provider-int, %v: %q", err, output)
}

output, err = exec.Command(
"ovs-vsctl", "--may-exist", "add-port", "br-int", "int-provider", "--",
"set", "interface", "int-provider", "type=patch", "options:peer=provider-int").CombinedOutput()
output, err = ovs.Exec("--may-exist", "add-port", "br-int", "int-provider", "--",
"set", "interface", "int-provider", "type=patch", "options:peer=provider-int")
if err != nil {
return fmt.Errorf("failed to create patch port int-provider, %v: %q", err, output)
}
Expand All @@ -325,13 +320,13 @@ func configProviderPort(providerInterfaceName string) error {
}

func providerBridgeExists() (bool, error) {
output, err := exec.Command("ovs-vsctl", "list-br").CombinedOutput()
output, err := ovs.Exec("list-br")
if err != nil {
klog.Errorf("failed to list bridge %v", err)
return false, err
}

lines := strings.Split(string(output), "\n")
lines := strings.Split(output, "\n")
for _, l := range lines {
if l == "br-provider" {
return true, nil
Expand All @@ -345,6 +340,6 @@ func providerBridgeExists() (bool, error) {
// A physical Ethernet device that is part of an Open vSwitch bridge should not have an IP address. If one does, then that IP address will not be fully functional.
// More info refer http://docs.openvswitch.org/en/latest/faq/issues
func configProviderNic(nicName string) error {
_, err := exec.Command("ovs-vsctl", "--may-exist", "add-port", "br-provider", nicName).CombinedOutput()
_, err := ovs.Exec("--may-exist", "add-port", "br-provider", nicName)
return err
}
2 changes: 1 addition & 1 deletion pkg/ovs/ovn-nbctl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (c Client) ovnNbCommand(cmdArgs ...string) (string, error) {
cmdArgs = append([]string{fmt.Sprintf("--timeout=%d", c.OvnTimeout)}, cmdArgs...)
raw, err := exec.Command(OvnNbCtl, cmdArgs...).CombinedOutput()
elapsed := float64((time.Since(start)) / time.Millisecond)
klog.Infof("%s command %s in %vms", OvnNbCtl, strings.Join(cmdArgs, " "), elapsed)
klog.Infof("command %s %s in %vms", OvnNbCtl, strings.Join(cmdArgs, " "), elapsed)
if err != nil {
return "", fmt.Errorf("%s, %q", raw, err)
}
Expand Down
21 changes: 12 additions & 9 deletions pkg/ovs/ovs-vsctl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,18 @@ import (
"os/exec"
"strconv"
"strings"
"time"
)

// Glory belongs to openvswitch/ovn-kubernetes
// https://github.com/openvswitch/ovn-kubernetes/blob/master/go-controller/pkg/util/ovs.go

func ovsExec(args ...string) (string, error) {
func Exec(args ...string) (string, error) {
start := time.Now()
args = append([]string{"--timeout=30"}, args...)
output, err := exec.Command("ovs-vsctl", args...).CombinedOutput()

elapsed := float64((time.Since(start)) / time.Millisecond)
klog.Infof("command ovs-vsctl %s in %vms", strings.Join(args, " "), elapsed)
if err != nil {
return "", fmt.Errorf("failed to run 'ovs-vsctl %s': %v\n %q", strings.Join(args, " "), err, output)
}
Expand All @@ -30,23 +33,23 @@ func ovsExec(args ...string) (string, error) {

func ovsCreate(table string, values ...string) (string, error) {
args := append([]string{"create", table}, values...)
return ovsExec(args...)
return Exec(args...)
}

func ovsDestroy(table, record string) error {
_, err := ovsExec("--if-exists", "destroy", table, record)
_, err := Exec("--if-exists", "destroy", table, record)
return err
}

func ovsSet(table, record string, values ...string) error {
args := append([]string{"set", table, record}, values...)
_, err := ovsExec(args...)
_, err := Exec(args...)
return err
}

// Returns the given column of records that match the condition
func ovsFind(table, column, condition string) ([]string, error) {
output, err := ovsExec("--no-heading", "--columns="+column, "find", table, condition)
output, err := Exec("--no-heading", "--columns="+column, "find", table, condition)
if err != nil {
return nil, err
}
Expand All @@ -70,7 +73,7 @@ func ovsFind(table, column, condition string) ([]string, error) {

func ovsClear(table, record string, columns ...string) error {
args := append([]string{"--if-exists", "clear", table, record}, columns...)
_, err := ovsExec(args...)
_, err := Exec(args...)
return err
}

Expand Down Expand Up @@ -165,7 +168,7 @@ func CleanLostInterface() {
return
}
klog.Infof("delete lost port %s", name)
output, err := exec.Command("ovs-vsctl", "--if-exists", "--with-iface", "del-port", "br-int", name).CombinedOutput()
output, err := Exec("--if-exists", "--with-iface", "del-port", "br-int", name)
if err != nil {
klog.Errorf("failed to delete ovs port %v, %s", err, output)
return
Expand All @@ -192,7 +195,7 @@ func CleanLostInterface() {
func CleanDuplicatePort(ifaceID string) {
uuids, _ := ovsFind("Interface", "_uuid", "external-ids:iface-id="+ifaceID)
for _, uuid := range uuids {
if out, err := ovsExec("remove", "Interface", uuid, "external-ids", "iface-id"); err != nil {
if out, err := Exec("remove", "Interface", uuid, "external-ids", "iface-id"); err != nil {
klog.Errorf("failed to clear stale OVS port %q iface-id %q: %v\n %q", uuid, ifaceID, err, out)
}
}
Expand Down

0 comments on commit bd97576

Please sign in to comment.