Support using multiple different network plugins#1151
Support using multiple different network plugins#1151katiewasnothere wants to merge 2 commits intoapple:mainfrom
Conversation
katiewasnothere
force-pushed
the
multi_network_allocate_in_service
branch
2 times, most recently
from
0c71dd3 to
2f0d9a1
Compare
katiewasnothere
force-pushed
the
multi_network_allocate_in_service
branch
from
2f0d9a1 to
71f97cb
Compare
jglogan
left a comment
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
There was a problem hiding this comment.
Definitely open to other naming options. I change AllocatedNetwork to AllocatedAttachment, but I'm not tied to that name.
| } | ||
| } | ||
|
|
||
| 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.
There was a problem hiding this comment.
Agreed. I'd prefer to do that in another PR though if you're okay with that.
| 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
There was a problem hiding this comment.
Updated the description
Sources/Services/ContainerSandboxService/Server/SandboxService.swift
Outdated
Show resolved
Hide resolved
katiewasnothere
force-pushed
the
multi_network_allocate_in_service
branch
2 times, most recently
from
fa18f84 to
042ab0b
Compare
* Update interface strategies to depend on network plugin information
katiewasnothere
force-pushed
the
multi_network_allocate_in_service
branch
from
042ab0b to
30b7722
Compare
katiewasnothere
force-pushed
the
multi_network_allocate_in_service
branch
from
30b7722 to
94e62d1
Compare
| var ipv6Subnet: CIDRv6? = nil | ||
|
|
||
| @Option(name: .long, help: "Set the plugin to use to create this network.") | ||
| var plugin: String = "container-network-vmnet" |
There was a problem hiding this comment.
Not sure if this is relevant for plugins in general or this specific pr, but is the "container-network" portion of this identifier something we can omit? It feels like the only relevant information (and just much easier to type and remember for a user) is the end. If I made a new network plugin and called it foobar, I'd hope I could just provide --plugin foobar
| private var containers: [String: ContainerState] | ||
|
|
||
| // FIXME: Find a better mechanism for services running on the APIServer to work with each other | ||
| private weak var networksService: NetworksService? |
4 participants
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