Skip to content

Commit

Permalink
fix issues in underlay networking
Browse files Browse the repository at this point in the history 
When nic used is changed, addresses and routes on the OVS bridge should be returned before adding the new nic to the OVS bridge.

When nodes are added to a provider network's `spec.excludeNodes`:
1. The nodes should be removed from the provider network's `status.readyNodes`;
2. If the provider network is being used by subnets, patch ports should be removed first by setting `ovn-bridge-mappings`;
3. Node labels should be updated first.
  • Loading branch information
@zhangzujian
zhangzujian committed Aug 19, 2021
1 parent de1d096 commit 8596ddc
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 49 deletions.
45 changes: 37 additions & 8 deletions pkg/daemon/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,12 +350,33 @@ func (c *Controller) initProviderNetwork(pn *kubeovnv1.ProviderNetwork, node *v1
return nil
}

func (c *Controller) cleanProviderNetwork(pn *kubeovnv1.ProviderNetwork, node *v1.Node) error {
var err error
if pn.Status.RemoveNodeConditions(node.Name) {
func (c *Controller) updateProviderNetworkStatusForNodeDeletion(pn *kubeovnv1.ProviderNetwork, node string) error {
if util.ContainsString(pn.Status.ReadyNodes, node) {
pn.Status.ReadyNodes = util.RemoveString(pn.Status.ReadyNodes, node)
if len(pn.Status.ReadyNodes) == 0 {
bytes := []byte(`[{ "op": "remove", "path": "/status/readyNodes"}]`)
_, err := c.config.KubeOvnClient.KubeovnV1().ProviderNetworks().Patch(context.Background(), pn.Name, types.JSONPatchType, bytes, metav1.PatchOptions{})
if err != nil {
klog.Errorf("failed to patch provider network %s: %v", pn.Name, err)
return err
}
} else {
bytes, err := pn.Status.Bytes()
if err != nil {
klog.Error(err)
return err
}
_, err = c.config.KubeOvnClient.KubeovnV1().ProviderNetworks().Patch(context.Background(), pn.Name, types.MergePatchType, bytes, metav1.PatchOptions{})
if err != nil {
klog.Errorf("failed to patch provider network %s: %v", pn.Name, err)
return err
}
}
}
if pn.Status.RemoveNodeConditions(node) {
if len(pn.Status.Conditions) == 0 {
bytes := []byte(`[{ "op": "remove", "path": "/status/conditions"}]`)
pn, err = c.config.KubeOvnClient.KubeovnV1().ProviderNetworks().Patch(context.Background(), pn.Name, types.JSONPatchType, bytes, metav1.PatchOptions{})
_, err := c.config.KubeOvnClient.KubeovnV1().ProviderNetworks().Patch(context.Background(), pn.Name, types.JSONPatchType, bytes, metav1.PatchOptions{})
if err != nil {
klog.Errorf("failed to patch provider network %s: %v", pn.Name, err)
return err
Expand All @@ -366,24 +387,25 @@ func (c *Controller) cleanProviderNetwork(pn *kubeovnv1.ProviderNetwork, node *v
klog.Error(err)
return err
}
pn, err = c.config.KubeOvnClient.KubeovnV1().ProviderNetworks().Patch(context.Background(), pn.Name, types.MergePatchType, bytes, metav1.PatchOptions{})
_, err = c.config.KubeOvnClient.KubeovnV1().ProviderNetworks().Patch(context.Background(), pn.Name, types.MergePatchType, bytes, metav1.PatchOptions{})
if err != nil {
klog.Errorf("failed to patch provider network %s: %v", pn.Name, err)
return err
}
}
}

if err = ovsCleanProviderNetwork(pn.Name); err != nil {
return err
}
return nil
}

func (c *Controller) cleanProviderNetwork(pn *kubeovnv1.ProviderNetwork, node *v1.Node) error {
patchPayloadTemplate := `[{ "op": "%s", "path": "/metadata/labels", "value": %s }]`
op := "replace"
if len(node.Labels) == 0 {
op = "add"
}

var err error
if pn.Status.RemoveNodeConditions(node.Name) {
if len(pn.Status.Conditions) == 0 {
bytes := []byte(`[{ "op": "remove", "path": "/status/conditions"}]`)
Expand Down Expand Up @@ -418,6 +440,13 @@ func (c *Controller) cleanProviderNetwork(pn *kubeovnv1.ProviderNetwork, node *v
return err
}

if err = c.updateProviderNetworkStatusForNodeDeletion(pn, node.Name); err != nil {
return err
}
if err = ovsCleanProviderNetwork(pn.Name); err != nil {
return err
}

return nil
}

Expand Down
33 changes: 28 additions & 5 deletions pkg/daemon/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,31 @@ func ovsCleanProviderNetwork(provider string) error {
return nil
}

if output, err = ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:ovn-bridge-mappings"); err != nil {
return fmt.Errorf("failed to get ovn-bridge-mappings, %v: %q", err, output)
}

mappings := strings.Split(output, ",")
brMap := fmt.Sprintf("%s:%s", provider, brName)

var idx int
for idx = range mappings {
if mappings[idx] == brMap {
break
}
}
if idx != len(mappings) {
mappings = append(mappings[:idx], mappings[idx+1:]...)
if len(mappings) == 0 {
output, err = ovs.Exec(ovs.IfExists, "remove", "open", ".", "external-ids", "ovn-bridge-mappings")
} else {
output, err = ovs.Exec("set", "open", ".", "external-ids:ovn-bridge-mappings="+strings.Join(mappings, ","))
}
if err != nil {
return fmt.Errorf("failed to set ovn-bridge-mappings, %v: %q", err, output)
}
}

// get host nic
if output, err = ovs.Exec("list-ports", brName); err != nil {
return fmt.Errorf("failed to list ports of OVS birdge %s, %v: %q", brName, err, output)
Expand All @@ -145,11 +170,9 @@ func ovsCleanProviderNetwork(provider string) error {
}
}

// remove external bridge
if err = removeExternalBridge(provider, brName); err != nil {
errMsg := fmt.Errorf("failed to remove external bridge %s: %v", brName, err)
klog.Error(errMsg)
return errMsg
// remove OVS bridge
if output, err = ovs.Exec(ovs.IfExists, "del-br", brName); err != nil {
return fmt.Errorf("failed to remove OVS bridge %s, %v: %q", brName, err, output)
}

return nil
Expand Down
38 changes: 2 additions & 36 deletions pkg/daemon/ovs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -511,8 +511,8 @@ func configExternalBridge(provider, bridge, nic string) error {
return fmt.Errorf("failed to check vendor of port %s: %v", port, err)
}
if ok {
if _, err = ovs.Exec(ovs.IfExists, "del-port", bridge, port); err != nil {
return fmt.Errorf("failed to remove %s from OVS birdge %s: %v", port, bridge, err)
if err = removeProviderNic(port, bridge); err != nil {
return fmt.Errorf("failed to remove port %s from OVS birdge %s: %v", port, bridge, err)
}
}
}
Expand All @@ -537,40 +537,6 @@ func configExternalBridge(provider, bridge, nic string) error {
return nil
}

func removeExternalBridge(provider, bridge string) error {
output, err := ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:ovn-bridge-mappings")
if err != nil {
return fmt.Errorf("failed to get ovn-bridge-mappings, %v: %q", err, output)
}

mappings := strings.Split(output, ",")
brMap := fmt.Sprintf("%s:%s", provider, bridge)

var idx int
for idx = range mappings {
if mappings[idx] == brMap {
break
}
}
if idx != len(mappings) {
mappings = append(mappings[:idx], mappings[idx+1:]...)
if len(mappings) == 0 {
output, err = ovs.Exec(ovs.IfExists, "remove", "open", ".", "external-ids", "ovn-bridge-mappings")
} else {
output, err = ovs.Exec("set", "open", ".", "external-ids:ovn-bridge-mappings="+strings.Join(mappings, ","))
}
if err != nil {
return fmt.Errorf("failed to set ovn-bridge-mappings, %v: %q", err, output)
}
}

if output, err = ovs.Exec(ovs.IfExists, "del-br", bridge); err != nil {
return fmt.Errorf("failed to remove OVS bridge %s, %v: %q", bridge, err, output)
}

return nil
}

// Add host nic to external bridge
// Mac address, MTU, IP addresses & routes will be copied/transferred to the external bridge
func configProviderNic(nicName, brName string) (int, error) {
Expand Down

0 comments on commit 8596ddc

Please sign in to comment.