Skip to content

Support using multiple different network plugins#1151

Open
katiewasnothere wants to merge 1 commit intoapple:mainfrom
katiewasnothere:multi_network_allocate_in_service
Open

Support using multiple different network plugins#1151
katiewasnothere wants to merge 1 commit intoapple:mainfrom
katiewasnothere:multi_network_allocate_in_service

Conversation

@katiewasnothere
Copy link
Contributor

@katiewasnothere katiewasnothere commented Feb 4, 2026

Type of Change

  • New feature
  • Breaking change

Motivation and Context

We want to be able to support using multiple network plugins during container's lifetime. This additionally means needing to pick an interface strategy to interpret a network attachment based on what network plugin was used to create that attachment. This PR will potentially replace #1081.

Followups:

  • doc updates to include the ability to specify plugin in the network creation cli

Testing

  • Tested locally
  • Added/updated tests

@katiewasnothere katiewasnothere force-pushed the multi_network_allocate_in_service branch 2 times, most recently from 0c71dd3 to 2f0d9a1 Compare February 4, 2026 18:44
ipv6Subnet = try container.decodeIfPresent(String.self, forKey: .ipv6Subnet)
.map { try CIDRv6($0) }
labels = try container.decodeIfPresent([String: String].self, forKey: .labels) ?? [:]
pluginInfo = try container.decode(NetworkPluginInfo.self, forKey: .pluginInfo)
Copy link
Contributor

Choose a reason for hiding this comment

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

decodeIfPresent would be a better choice here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We talked about this already but for others: I want the plugin information to be mandatory.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps Raj is suggesting is that decode will break if presented with an on-disk network config that was created before adding this field.

* Update interface strategies to depend on network plugin information
@katiewasnothere katiewasnothere force-pushed the multi_network_allocate_in_service branch from 2f0d9a1 to 71f97cb Compare February 5, 2026 00:30
Copy link
Contributor

@jglogan jglogan left a comment

Choose a reason for hiding this comment

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

Early nitpicks regarding the type name AllocatedNetwork

/// AllocatedNetwork represents an allocated network attachment and additional
/// relevant data needed for a sandbox to properly configure a network interface
/// on bootstrap.
public struct AllocatedNetwork: Sendable {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpicking on the meaning...is sounds like networks are being allocated, is that really what's happening?

gonna throw a palette of terms/type out there as maybe it'll help

  • Network is a prototype that's used at the network helper level for something that can start up a network, can return its state, and can provide some additional data (such as the XPC serialization of the vmnet_network_ref)
  • Attachment is used at the apiserver client level to describe an attachment from a container to a network, identifying the network to which to attach, and providing additional options (MAC, perhaps later MTU and IPAM info, such dhcp or a static IP suffix)
  • Interface is used at the sandbox as the abstraction of the virtual device that realizes the attachment between the container and the network, resulting in a Linux network interface in the running container
  • "allocation" occurs as a request from the sandbox helper to the network helper for a given attachment, so that the sandbox service has the information it needs to configure the Interface, and to bring up the corresponding Linux network interface once the container is running

ipv6Subnet = try container.decodeIfPresent(String.self, forKey: .ipv6Subnet)
.map { try CIDRv6($0) }
labels = try container.decodeIfPresent([String: String].self, forKey: .labels) ?? [:]
pluginInfo = try container.decode(NetworkPluginInfo.self, forKey: .pluginInfo)
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps Raj is suggesting is that decode will break if presented with an on-disk network config that was created before adding this field.

}
}

public var pluginInfo: NetworkPluginInfo {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should see if there's a better way to deal with this proliferation of wrappers we work on a common managed resource paradigm.

Networks and containers both have configuration and runtime aspects so both would benefit from an approach that cleanly integrates them into a resource where state and config are easily accessible.

@katiewasnothere katiewasnothere marked this pull request as ready for review February 5, 2026 19:35
public init(
root: URL,
interfaceStrategy: InterfaceStrategy,
interfaceStrategies: [NetworkPluginInfo: InterfaceStrategy],
Copy link
Contributor

Choose a reason for hiding this comment

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

docc needs updating, we'll want a decent description of what interfaceStrategies is for, let's chat about this today

for attachmentConfiguration in attachmentConfigurations {
let client = NetworkClient(id: attachmentConfiguration.network)
let state = try await client.state()
let state = try await ClientNetwork.get(id: attachmentConfiguration.network)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we calling from the sandbox to the API server here, instead of directly to the network helper?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants