Support using multiple different network plugins#1151
Support using multiple different network plugins#1151katiewasnothere wants to merge 1 commit intoapple:mainfrom
Conversation
0c71dd3 to
2f0d9a1
Compare
| 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) |
There was a problem hiding this comment.
decodeIfPresent would be a better choice here
There was a problem hiding this comment.
We talked about this already but for others: I want the plugin information to be mandatory.
There was a problem hiding this comment.
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
2f0d9a1 to
71f97cb
Compare
jglogan
left a comment
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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
Networkis 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 thevmnet_network_ref)Attachmentis 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, suchdhcpor a static IP suffix)Interfaceis 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) |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
| public init( | ||
| root: URL, | ||
| interfaceStrategy: InterfaceStrategy, | ||
| interfaceStrategies: [NetworkPluginInfo: InterfaceStrategy], |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
Are we calling from the sandbox to the API server here, instead of directly to the network helper?
Type of 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:
Testing