Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 37 additions & 17 deletions test/e2e/ovn-vpc-nat-gw/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,27 +301,48 @@ var _ = framework.Describe("[group:ovn-vpc-nat-gw]", func() {
bridgeName = linkMap[node.ID].IfName
}

links, err := node.ListLinks()
framework.ExpectNoError(err, "failed to list links on node %s: %v", node.Name(), err)
expectedAddrs := linkMap[node.ID].NonLinkLocalAddresses()

// Poll for bridge addresses to stabilize, as the daemon may
// re-process the provider network event (triggered by the
// controller status update) and briefly reconfigure the bridge.
var port, bridge *iproute.Link
for i, link := range links {
if link.IfIndex == linkMap[node.ID].IfIndex {
port = &links[i]
} else if link.IfName == bridgeName {
bridge = &links[i]
for _, addr := range bridge.NonLinkLocalAddresses() {
if util.CheckProtocol(addr) == kubeovnv1.ProtocolIPv4 {
ginkgo.By("get provider bridge v4 ip " + addr)
*bridgeIps = append(*bridgeIps, addr)
}
framework.WaitUntil(2*time.Second, 30*time.Second, func(_ context.Context) (bool, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The framework.WaitUntil function appears to have a hardcoded polling interval of 2 seconds and ignores its first argument. While the value you've passed (2*time.Second) coincidentally matches the hardcoded interval, this is misleading and could cause confusion or bugs during future maintenance. The function signature in test/e2e/framework/wait.go is func WaitUntil(_, timeout time.Duration, ...) where the first argument is explicitly ignored.

To avoid confusion, it would be better to pass a zero value for the ignored interval. Ideally, framework.WaitUntil would be updated to accept and use the interval parameter, but since that's outside the scope of this PR, making the call less ambiguous is a good step.

Suggested change
framework.WaitUntil(2*time.Second, 30*time.Second, func(_ context.Context) (bool, error) {
framework.WaitUntil(0, 30*time.Second, func(_ context.Context) (bool, error) {

links, err := node.ListLinks()
if err != nil {
return false, err
}

port = nil
bridge = nil
for i, link := range links {
if link.IfIndex == linkMap[node.ID].IfIndex {
port = &links[i]
} else if link.IfName == bridgeName {
bridge = &links[i]
}
if port != nil && bridge != nil {
break
}
}
if port == nil || bridge == nil {
return false, nil
}
if port != nil && bridge != nil {
break

bridgeAddrs := bridge.NonLinkLocalAddresses()
if len(bridgeAddrs) != len(expectedAddrs) {
return false, nil
}
return true, nil
}, fmt.Sprintf("waiting for bridge %s addresses on node %s", bridgeName, node.Name()))

for _, addr := range bridge.NonLinkLocalAddresses() {
if util.CheckProtocol(addr) == kubeovnv1.ProtocolIPv4 {
ginkgo.By("get provider bridge v4 ip " + addr)
*bridgeIps = append(*bridgeIps, addr)
}
}
framework.ExpectNotNil(port)

framework.ExpectEqual(port.Address, linkMap[node.ID].Address)
framework.ExpectEqual(port.Mtu, linkMap[node.ID].Mtu)
framework.ExpectEqual(port.Master, "ovs-system")
Expand All @@ -330,15 +351,14 @@ var _ = framework.Describe("[group:ovn-vpc-nat-gw]", func() {
framework.ExpectEqual(port.IfName, util.ExternalBridgeName(providerNetworkName))
}

framework.ExpectNotNil(bridge)
framework.ExpectEqual(bridge.LinkInfo.InfoKind, "openvswitch")
framework.ExpectEqual(bridge.Address, port.Address)
framework.ExpectEqual(bridge.Mtu, port.Mtu)
framework.ExpectEqual(bridge.OperState, "UNKNOWN")
framework.ExpectContainElement(bridge.Flags, "UP")

framework.ExpectEmpty(port.NonLinkLocalAddresses())
framework.ExpectConsistOf(bridge.NonLinkLocalAddresses(), linkMap[node.ID].NonLinkLocalAddresses())
framework.ExpectConsistOf(bridge.NonLinkLocalAddresses(), expectedAddrs)

linkNameMap[node.ID] = port.IfName
}
Expand Down
Loading