Skip to content

Commit

Permalink
Add Test for linksInContainer = false
Browse files Browse the repository at this point in the history
This commit introduces a new test case that tests the code flow when the linksInContainer parameter is set to false, indicating that links start in a different namespace instead of the pod namespace. This scenario requires a different setup than usual, involving the addition of dummy links to a different namespace rather than the pod namespace.

1. Created a  function (addLinksInNS) responsible for adding links during namespace setup for each test. This function is needed for the new test, where dummy links are added to the init namespace.

2. Added an option to modify the linksInContainer setting in the string config used across all tests. This change allows each test to set the linksInContainer option to true or false as needed.

3. Introduced a test case that verifies the correct addition and deletion of the plugin while links are active in the host network namespace. To avoid redundancy, new functions (validateBondIFConf, validateBondSlavesConf) were added.

4. Included a missing netNs.Close() statement in the bond code's setLinksinNetNs function to properly close the pod namespace.

5. Also changed the setLinksinNetNs function. Previous versions didn't include transfer of the namespace back to the current namespace after function execution. In this commit instead of using ns.netNs.Set function, ns.netNs.Do was used, including the transfer to the initial namespace. This change also included creation of a move links from namespace function for a more readable implementation. 

Signed-off-by: Alina Sudakov <asudakov@redhat.com>
  • Loading branch information
AlinaSecret committed Oct 23, 2023
1 parent 3864500 commit 111bd48
Show file tree
Hide file tree
Showing 3 changed files with 320 additions and 237 deletions.
1 change: 1 addition & 0 deletions bond-cni
65 changes: 32 additions & 33 deletions bond/bond.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ func deattachLinksFromBond(linkObjectsToDeattach []netlink.Link, netNsHandle *ne
return nil
}

func setLinksinNetNs(bondConf *bondingConfig, nspath string, releaseLinks bool) error {
var netNs, curnetNs ns.NetNS
func setLinksInNetNs(bondConf *bondingConfig, nspath string, releaseLinks bool) error {
var podNs, hostNS ns.NetNS
var err error

linkNames := []string{}
Expand All @@ -186,49 +186,48 @@ func setLinksinNetNs(bondConf *bondingConfig, nspath string, releaseLinks bool)
linkNames = append(linkNames, s)
}

if netNs, err = ns.GetNS(nspath); err != nil {
if podNs, err = ns.GetNS(nspath); err != nil {
return fmt.Errorf("failed to open netns %q: %v", nspath, err)
}
defer podNs.Close()

if curnetNs, err = ns.GetCurrentNS(); err != nil {
if hostNS, err = ns.GetCurrentNS(); err != nil {
return fmt.Errorf("failed to get init netns: %v", err)
}

if releaseLinks {
if err := netNs.Set(); err != nil {
return fmt.Errorf("failed to enter netns %q: %v", netNs, err)
}
err = moveLinksBetweenNs(linkNames, podNs, hostNS, "host")
} else {
err = moveLinksBetweenNs(linkNames, hostNS, podNs, "container")
}
return err
}

if len(linkNames) >= 2 { // currently supporting two or more links to one bond
for _, linkName := range linkNames {
// get interface link in the network namespace
link, err := netlink.LinkByName(linkName)
if err != nil {
return fmt.Errorf("failed to lookup link interface %q: %v", linkName, err)
}

// set link interface down
if err = netlink.LinkSetDown(link); err != nil {
return fmt.Errorf("failed to down link interface %q: %v", linkName, err)
}
func moveLinksBetweenNs(links []string, from ns.NetNS, to ns.NetNS, toNsName string) error {
return from.Do(func(ns.NetNS) error {
if len(links) < 2 { // currently supporting two or more links to one bond
return fmt.Errorf("Bonding requires at least two links, we have %+v", len(links))
} else {
for _, linkName := range links {
// get interface link in the network namespace
link, err := netlink.LinkByName(linkName)
if err != nil {
return fmt.Errorf("failed to lookup link interface %q: %v", linkName, err)
}

if releaseLinks { // move link inteface to network netns
if err = netlink.LinkSetNsFd(link, int(curnetNs.Fd())); err != nil {
return fmt.Errorf("failed to move link interface to host netns %q: %v", linkName, err)
// set link interface down
if err = netlink.LinkSetDown(link); err != nil {
return fmt.Errorf("failed to down link interface %q: %v", linkName, err)
}
} else {
if err = netlink.LinkSetNsFd(link, int(netNs.Fd())); err != nil {
return fmt.Errorf("failed to move link interface to container netns %q: %v", linkName, err)

// move link interface to network netns
if err = netlink.LinkSetNsFd(link, int(to.Fd())); err != nil {
return fmt.Errorf("failed to move link interface to %s netns %q: %v", toNsName, linkName, err)
}
}

}
} else {
return fmt.Errorf("Bonding requires at least two links, we have %+v", len(linkNames))
}

return nil
return nil
})
}

func createBond(bondName string, bondConf *bondingConfig, nspath string, ns ns.NetNS) (*current.Interface, error) {
Expand All @@ -249,7 +248,7 @@ func createBond(bondName string, bondConf *bondingConfig, nspath string, ns ns.N
defer netNsHandle.Close()

if !bondConf.LinksContNs {
if err := setLinksinNetNs(bondConf, nspath, false); err != nil {
if err := setLinksInNetNs(bondConf, nspath, false); err != nil {
return nil, fmt.Errorf("Failed to move the links (%+v) in container network namespace, error: %+v", bondConf.Links, err)
}
}
Expand Down Expand Up @@ -423,7 +422,7 @@ func cmdDel(args *skel.CmdArgs) error {
}

if !bondConf.LinksContNs {
if err := setLinksinNetNs(bondConf, args.Netns, true); err != nil {
if err := setLinksInNetNs(bondConf, args.Netns, true); err != nil {
return fmt.Errorf("Failed set links (%+v) in host network namespace, error: %+v", bondConf.Links, err)
}
}
Expand Down
Loading

0 comments on commit 111bd48

Please sign in to comment.