Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of cni: ensure to setup CNI addresses in deterministic order into release/1.5.x #17827

Merged
Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions .changelog/17766.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
cni: Ensure to setup CNI addresses in deterministic order
```
48 changes: 27 additions & 21 deletions client/allocrunner/networking_cni.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,46 +127,52 @@ func (c *cniNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Alloc
// cniToAllocNet converts a CNIResult to an AllocNetworkStatus or returns an
// error. The first interface and IP with a sandbox and address set are
// preferred. Failing that the first interface with an IP is selected.
//
// Unfortunately the go-cni library returns interfaces in an unordered map so
// the results may be nondeterministic depending on CNI plugin output.
func (c *cniNetworkConfigurator) cniToAllocNet(res *cni.CNIResult) (*structs.AllocNetworkStatus, error) {
if len(res.Interfaces) == 0 {
return nil, fmt.Errorf("failed to configure network: no interfaces found")
}

netStatus := new(structs.AllocNetworkStatus)

// Unfortunately the go-cni library returns interfaces in an unordered map meaning
// the results may be nondeterministic depending on CNI plugin output so make
// sure we sort them by interface name.
names := make([]string, 0, len(res.Interfaces))
for k := range res.Interfaces {
names = append(names, k)
}
sort.Strings(names)

// Use the first sandbox interface with an IP address
if len(res.Interfaces) > 0 {
for name, iface := range res.Interfaces {
if iface == nil {
// this should never happen but this value is coming from external
// plugins so we should guard against it
delete(res.Interfaces, name)
}
for _, name := range names {
iface := res.Interfaces[name]
if iface == nil {
// this should never happen but this value is coming from external
// plugins so we should guard against it
delete(res.Interfaces, name)
continue
}

if iface.Sandbox != "" && len(iface.IPConfigs) > 0 {
netStatus.Address = iface.IPConfigs[0].IP.String()
netStatus.InterfaceName = name
break
}
if iface.Sandbox != "" && len(iface.IPConfigs) > 0 {
netStatus.Address = iface.IPConfigs[0].IP.String()
netStatus.InterfaceName = name
break
}
}

// If no IP address was found, use the first interface with an address
// found as a fallback
if netStatus.Address == "" {
var found bool
for name, iface := range res.Interfaces {
for _, name := range names {
iface := res.Interfaces[name]
if len(iface.IPConfigs) > 0 {
ip := iface.IPConfigs[0].IP.String()
c.logger.Debug("no sandbox interface with an address found CNI result, using first available", "interface", name, "ip", ip)
netStatus.Address = ip
netStatus.InterfaceName = name
found = true
break
}
}
if !found {
c.logger.Warn("no address could be found from CNI result")
}
}

// If no IP address could be found, return an error
Expand Down
16 changes: 16 additions & 0 deletions client/allocrunner/networking_cni_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,22 @@ func TestCNI_forceCleanup(t *testing.T) {
})
}

// TestCNI_cniToAllocNet_NoInterfaces asserts an error is returned if CNIResult
// contains no interfaces.
func TestCNI_cniToAllocNet_NoInterfaces(t *testing.T) {
ci.Parallel(t)

cniResult := &cni.CNIResult{}

// Only need a logger
c := &cniNetworkConfigurator{
logger: testlog.HCLogger(t),
}
allocNet, err := c.cniToAllocNet(cniResult)
require.Error(t, err)
require.Nil(t, allocNet)
}

// TestCNI_cniToAllocNet_Fallback asserts if a CNI plugin result lacks an IP on
// its sandbox interface, the first IP found is used.
func TestCNI_cniToAllocNet_Fallback(t *testing.T) {
Expand Down