Skip to content

Commit

Permalink
Create Test Case Where Bond MTU is Greater than Links MTU
Browse files Browse the repository at this point in the history 
This commit introduces a new test case in which the bond's MTU is larger than the MTU of its links. The test verifies whether an error is raised when the bond's MTU exceeds the MTU of its links.

- Added an option to modify the MTU setting in the string config used across all tests. This change allows each test to set the MTU option to the desired value.

- Created a constant for the values used in the MTU validation function, thereby improving code readability by avoiding the use of "magic numbers."

Signed-off-by: Alina Sudakov <[email protected]>
  • Loading branch information
@AlinaSecret
AlinaSecret committed Oct 17, 2023
1 parent e25c54f commit 4ec9da2
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 26 deletions.
84 changes: 60 additions & 24 deletions bond/bond_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,30 +42,33 @@ const (
"failOverMac": 1,
"linksInContainer": %s,
"miimon": "100",
"mtu": 1400,
"mtu": %s,
"links": [
{"name": "net1"},
{"name": "net2"}
]
}`
ActiveBackupMode = "active-backup"
BalanceTlbMode = "balance-tlb"
DefaultMTU = 1400
)

var Slaves = []string{Slave1, Slave2}

var _ = Describe("bond plugin", func() {
var podNS ns.NetNS
var initNS ns.NetNS
var linkAttrs []netlink.LinkAttrs
var linksInContainer bool
var linkAttrs = []netlink.LinkAttrs{
{Name: Slave1},
{Name: Slave2},
}

When("links are in container`s network namespace at initial state (meaning linksInContainer is true)", func() {
BeforeEach(func() {
var err error
linksInContainer = true
linkAttrs = []netlink.LinkAttrs{
{Name: Slave1},
{Name: Slave2},
}
podNS, err = testutils.NewNS()
Expect(err).NotTo(HaveOccurred())
addLinksInNS(podNS, linkAttrs)
Expand All @@ -80,7 +83,7 @@ var _ = Describe("bond plugin", func() {
ContainerID: "dummy",
Netns: podNS.Path(),
IfName: IfName,
StdinData: []byte(fmt.Sprintf(Config, "0.3.1", ActiveBackupMode, strconv.FormatBool(linksInContainer))),
StdinData: []byte(fmt.Sprintf(Config, "0.3.1", ActiveBackupMode, strconv.FormatBool(linksInContainer), strconv.Itoa(DefaultMTU))),
}

By("creating the plugin")
Expand All @@ -89,26 +92,18 @@ var _ = Describe("bond plugin", func() {
})
Expect(err).NotTo(HaveOccurred())

By("validationg the returned result is correct")
By("validating the returned result is correct")
checkAddReturnResult(&r, IfName)

err = podNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()
By("validating the bond interface is configured correctly")
link, err := netlink.LinkByName(IfName)
Expect(err).NotTo(HaveOccurred())
bond := link.(*netlink.Bond)
Expect(bond.Attrs().MTU).To(Equal(1400))
Expect(bond.Mode.String()).To(Equal(ActiveBackupMode))
Expect(bond.Miimon).To(Equal(100))
validateBondIFConf(link, DefaultMTU, ActiveBackupMode, 100)

By("validating the bond slaves are configured correctly")
for _, slaveName := range Slaves {
slave, err := netlink.LinkByName(slaveName)
Expect(err).NotTo(HaveOccurred())
Expect(slave.Attrs().Slave).NotTo(BeNil())
Expect(slave.Attrs().MasterIndex).To(Equal(bond.Attrs().Index))
}
validateBondSlavesConf(link, Slaves)
return nil
})
Expect(err).NotTo(HaveOccurred())
Expand All @@ -132,7 +127,7 @@ var _ = Describe("bond plugin", func() {
ContainerID: "dummy",
Netns: podNS.Path(),
IfName: IfName,
StdinData: []byte(fmt.Sprintf(Config, "1.0.0", ActiveBackupMode, strconv.FormatBool(linksInContainer))),
StdinData: []byte(fmt.Sprintf(Config, "1.0.0", ActiveBackupMode, strconv.FormatBool(linksInContainer), strconv.Itoa(DefaultMTU))),
}

By("adding a bond interface")
Expand All @@ -158,7 +153,7 @@ var _ = Describe("bond plugin", func() {
ContainerID: "dummy",
Netns: podNS.Path(),
IfName: IfName,
StdinData: []byte(fmt.Sprintf(Config, "1.0.0", ActiveBackupMode, strconv.FormatBool(linksInContainer))),
StdinData: []byte(fmt.Sprintf(Config, "1.0.0", ActiveBackupMode, strconv.FormatBool(linksInContainer), strconv.Itoa(DefaultMTU))),
}

By("adding a bond interface")
Expand Down Expand Up @@ -192,7 +187,7 @@ var _ = Describe("bond plugin", func() {
ContainerID: "dummy",
Netns: podNS.Path(),
IfName: IfName,
StdinData: []byte(fmt.Sprintf(Config, version, ActiveBackupMode, strconv.FormatBool(linksInContainer))),
StdinData: []byte(fmt.Sprintf(Config, version, ActiveBackupMode, strconv.FormatBool(linksInContainer), strconv.Itoa(DefaultMTU))),
}
By(fmt.Sprintf("creating the plugin with config in version %s", version))
r, _, err := testutils.CmdAddWithArgs(args, func() error {
Expand Down Expand Up @@ -221,7 +216,7 @@ var _ = Describe("bond plugin", func() {
ContainerID: "dummy",
Netns: podNS.Path(),
IfName: IfName,
StdinData: []byte(fmt.Sprintf(Config, "0.3.1", BalanceTlbMode, strconv.FormatBool(linksInContainer))),
StdinData: []byte(fmt.Sprintf(Config, "0.3.1", BalanceTlbMode, strconv.FormatBool(linksInContainer), strconv.Itoa(DefaultMTU))),
}

err := podNS.Do(func(ns.NetNS) error {
Expand Down Expand Up @@ -258,7 +253,7 @@ var _ = Describe("bond plugin", func() {
ContainerID: "dummy",
Netns: podNS.Path(),
IfName: IfName,
StdinData: []byte(fmt.Sprintf(Config, "0.3.1", ActiveBackupMode, strconv.FormatBool(linksInContainer))),
StdinData: []byte(fmt.Sprintf(Config, "0.3.1", ActiveBackupMode, strconv.FormatBool(linksInContainer), strconv.Itoa(DefaultMTU))),
}

err = podNS.Do(func(ns.NetNS) error {
Expand Down Expand Up @@ -308,10 +303,51 @@ var _ = Describe("bond plugin", func() {
Expect(err).NotTo(HaveOccurred())
})
})
When("Links Have Custom MTU", func() {
const Slave1Mtu = 1000
const Slave2Mtu = 800

BeforeEach(func() {
var err error
linksInContainer = true
linkAttrs = []netlink.LinkAttrs{
{MTU: Slave1Mtu, Name: Slave1},
{MTU: Slave2Mtu, Name: Slave2},
}
podNS, err = testutils.NewNS()
Expect(err).NotTo(HaveOccurred())
addLinksInNS(podNS, linkAttrs)
})

AfterEach(func() {
Expect(podNS.Close()).To(Succeed())
Expect(testutils.UnmountNS(podNS)).To(Succeed())
})
DescribeTable("Verify plugin raises error when Bond MTU is bigger then links MTU", func(bondMTU string) {
args := &skel.CmdArgs{
ContainerID: "dummy",
Netns: podNS.Path(),
IfName: IfName,
StdinData: []byte(fmt.Sprintf(Config, "0.3.1", ActiveBackupMode, strconv.FormatBool(linksInContainer), bondMTU)),
}
By("creating the plugin")
_, _, err := testutils.CmdAddWithArgs(args, func() error {
return cmdAdd(args)
})
Expect(err).To(HaveOccurred())
},
Entry("Bond MTU is bigger then one of links MTU", "1200"),
Entry("Bond MTU is bigger then all of links MTU", "900"),
)
})
When("links are in host`s network namespace at initial state (meaning linksInContainer is false)", func() {
BeforeEach(func() {
var err error
linksInContainer = false
linkAttrs = []netlink.LinkAttrs{
{Name: Slave1},
{Name: Slave2},
}
podNS, err = testutils.NewNS()
Expect(err).NotTo(HaveOccurred())
initNS, err = testutils.NewNS()
Expand All @@ -330,7 +366,7 @@ var _ = Describe("bond plugin", func() {
ContainerID: "dummy",
Netns: podNS.Path(),
IfName: IfName,
StdinData: []byte(fmt.Sprintf(Config, "0.3.1", ActiveBackupMode, strconv.FormatBool(linksInContainer))),
StdinData: []byte(fmt.Sprintf(Config, "0.3.1", ActiveBackupMode, strconv.FormatBool(linksInContainer), strconv.Itoa(DefaultMTU))),
}
err := initNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()
Expand All @@ -351,7 +387,7 @@ var _ = Describe("bond plugin", func() {
By("validating the bond interface is configured correctly")
link, err := netlink.LinkByName(IfName)
Expect(err).NotTo(HaveOccurred())
validateBondIFConf(link, 1400, ActiveBackupMode, 100)
validateBondIFConf(link, DefaultMTU, ActiveBackupMode, 100)

By("validating the bond slaves are configured correctly")
validateBondSlavesConf(link, Slaves)
Expand Down
10 changes: 8 additions & 2 deletions bond/util/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,19 @@ import (
"github.com/vishvananda/netlink"
)

const (
standardEthernetMTU = 1500
defaultMTU = standardEthernetMTU
minMtuIpv4Packet = 68
)

func ValidateMTU(slaveLinks []netlink.Link, mtu int) error {
// if not specified set MTU to default
if mtu == 0 {
mtu = 1500
mtu = defaultMTU
}

if mtu < 68 {
if mtu < minMtuIpv4Packet {
return fmt.Errorf("Invalid bond MTU value (%+v), should be 68 or bigger", mtu)
}
netHandle, err := netlink.NewHandle()
Expand Down

0 comments on commit 4ec9da2

Please sign in to comment.