Skip to content

Commit

Permalink
Merge pull request #18049 from danwinship/egress-ip-setup-fix
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue (batch tested with PRs 18117, 18049).

Make sure oc.tunMAC gets set even if AlreadySetUp()

Noticed while trying to fix rhbz 1527642: oc.tunMAC currently only gets set from SetupOVS(), so if you restart OpenShift and SDN setup gets skipped, then tunMAC will be unset, and so new auto-egress-ip rules will fail.

The switch from using netlink to use ovs-vsctl to fetch the MAC is because an earlier version of the patch broke the hack in ovscontroller_test.go that manually sets tunMAC, and made it so that SetupOVS would always have to read tunMAC from tun0. But calling netlink wouldn't work from ovscontroller_test, so I rewrote it to use ovs-vsctl to get the MAC instead, since that was mockable. But then I ended up rewriting things so that it was possible for ovscontroller_test to still just manually override it anyway. But I liked the ovs-vsctl-rather-than-netlink approach because it makes ovsController more self-contained and mock-able so I kept it.
  • Loading branch information
openshift-merge-robot committed Jan 17, 2018
2 parents 0201b09 + f40ccbc commit d5175c1
Showing 1 changed file with 18 additions and 9 deletions.
27 changes: 18 additions & 9 deletions pkg/network/node/ovscontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import (

"k8s.io/apimachinery/pkg/util/sets"
kapi "k8s.io/kubernetes/pkg/apis/core"

"github.com/vishvananda/netlink"
)

type ovsController struct {
Expand Down Expand Up @@ -87,13 +85,6 @@ func (oc *ovsController) SetupOVS(clusterNetworkCIDR []string, serviceNetworkCID
if err != nil {
return err
}
if oc.tunMAC == "" {
link, err := netlink.LinkByName(Tun0)
if err != nil {
return err
}
oc.tunMAC = link.Attrs().HardwareAddr.String()
}

otx := oc.ovs.NewTransaction()

Expand Down Expand Up @@ -675,6 +666,21 @@ func (oc *ovsController) FindUnusedVNIDs() []int {
return policyVNIDs.Difference(inUseVNIDs).UnsortedList()
}

func (oc *ovsController) ensureTunMAC() error {
if oc.tunMAC != "" {
return nil
}

val, err := oc.ovs.Get("Interface", Tun0, "mac_in_use")
if err != nil {
return fmt.Errorf("could not get %s MAC address: %v", Tun0, err)
} else if len(val) != 19 || val[0] != '"' || val[18] != '"' {
return fmt.Errorf("bad MAC address for %s: %q", Tun0, val)
}
oc.tunMAC = val[1:18]
return nil
}

func (oc *ovsController) UpdateNamespaceEgressRules(vnid uint32, nodeIP, egressHex string) error {
otx := oc.ovs.NewTransaction()
otx.DeleteFlows("table=100, reg0=%d", vnid)
Expand All @@ -686,6 +692,9 @@ func (oc *ovsController) UpdateNamespaceEgressRules(vnid uint32, nodeIP, egressH
otx.AddFlow("table=100, priority=100, reg0=%d, actions=drop", vnid)
} else if nodeIP == oc.localIP {
// Local Egress IP
if err := oc.ensureTunMAC(); err != nil {
return err
}
otx.AddFlow("table=100, priority=100, reg0=%d, ip, actions=set_field:%s->eth_dst,set_field:%s->pkt_mark,goto_table:101", vnid, oc.tunMAC, egressHex)
} else {
// Remote Egress IP; send via VXLAN
Expand Down

0 comments on commit d5175c1

Please sign in to comment.