Add implementation of network configuration proxy#915
Add implementation of network configuration proxy#915dcantah merged 1 commit intomicrosoft:masterfrom
Conversation
|
Just remaking the PR as the comments for the old one were ancient and it was also complaining about merge conflicts that weren't showing up locally. |
dcantah
force-pushed
the
ncproxy-newdesign
branch
2 times, most recently
from
7e070c6 to
637f0fc
Compare
dcantah
force-pushed
the
ncproxy-newdesign
branch
2 times, most recently
from
6c55cd5 to
69ebb66
Compare
dcantah
force-pushed
the
ncproxy-newdesign
branch
from
69ebb66 to
4bb3a79
Compare
| @@ -1,221 +0,0 @@ | |||
| file { | |||
There was a problem hiding this comment.
@kevpar do we still need these files for something?
There was a problem hiding this comment.
@kevpar I'd also like to know. Everytime I make a change to the runhcs options this file gets deleted
There was a problem hiding this comment.
I don't think we need these files. It looks like they are used as part of documenting old API versions in containerd.
There was a problem hiding this comment.
If you're removing it, though, you should also remove the descriptor entry from Protobuild.toml.
dcantah
force-pushed
the
ncproxy-newdesign
branch
from
4bb3a79 to
d5b19d4
Compare
dcantah
force-pushed
the
ncproxy-newdesign
branch
2 times, most recently
from
fe2a067 to
1080429
Compare
| // | ||
| // `addr` is an optional parameter | ||
| func (uvm *UtilityVM) CreateAndAssignNetworkSetup(ctx context.Context, addr, containerID string) (err error) { | ||
| if uvm.NCProxyEnabled() { |
There was a problem hiding this comment.
should we also enforce that addr is not empty here and fall back to local if it is for some reason?
There was a problem hiding this comment.
We can enforce addr isn't empty to fail earlier, but I don't think we should fallback
| if err == uvm.ErrNoNetworkSetup { | ||
| // No network interface passed in, just set up locally. | ||
| coi.HostingSystem.NetworkSetup = uvm.NewInternalNetworkSetup(coi.HostingSystem) | ||
| err := coi.HostingSystem.ConfigureNetworking(ctx, coi.actualNetworkNamespace) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
do we want to use CreateAndAssignNetworkSetup here?
There was a problem hiding this comment.
Sure, makes more sense if we're using everywhere else
dcantah
force-pushed
the
ncproxy-newdesign
branch
from
1080429 to
54704d9
Compare
|
@katiewasnothere Remedied last round |
| // No network setup type was specified for this UVM. Create and assign one here unless | ||
| // we received a different error. | ||
| if err == uvm.ErrNoNetworkSetup { | ||
| if err := coi.HostingSystem.CreateAndAssignNetworkSetup(ctx, "", ""); err != nil { |
There was a problem hiding this comment.
do we know for sure at this point that ncproxy is not enabled for the VM?
There was a problem hiding this comment.
The only time in our use cases we'd end up here is for a standalone task (launched through ctr) and we don't have any code to setup ncproxy for that so I think this is a fine assumption for now.
|
I think the plan was to check this in after @katiewasnothere's review, as this was just seen as "adding on" to what we have and not changing anything else, but as can be seen this is not the case 😄. A lot of the UVM networking code was reworked or moved around to make a cleaner abstraction between setting up networking via our normal path vs. ncproxy. @kevpar should we get another review on this? |
If this has risk to existing code paths then I think a 2nd reviewer would be a good idea. |
* Ncproxy (abbreviation of network configuration proxy) is a proxy used to facilitate external configuration of a pods network through a set of TTRPC and GRPC services. Ncproxy relies on other TTRPC/GRPC services to get the information it needs to perform its actions. The full set of services are as follows: ------------------------------------------------------------------------------------------------------ NetworkConfigProxy (TTRPC) - This service is exposed by Ncproxy and is used by the shim. NodeNetworkService (GRPC) - This service is exposed by any application implementing the interface to the service (/cmd/ncproxy/sdn_nodenetsvc/nodenetsvc.proto). NetworkConfigProxy (GRPC) - This service is exposed by Ncproxy and is used by a service implementing the NodeNetworkService GRPC interface. ComputeAgent (TTRPC) - This service is exposed by the shim and is called by ncproxy. --------------------------------------------------------------------------------------------------------- This is an optional feature that can be enabled by setting the annotation "io.microsoft.network.ncproxy" and providing an address to a TTRPC service that implements the NetworkConfigProxy TTRPC service defined in /internal/ncproxyttrpc. Signed-off-by: Daniel Canter <dcanter@microsoft.com>
dcantah
force-pushed
the
ncproxy-newdesign
branch
from
54704d9 to
88e182c
Compare
anmaxvl
left a comment
anmaxvl
left a comment
There was a problem hiding this comment.
skimmed through old code paths and the changes look ok. had one comment though
| Spec: s, | ||
| HostingSystem: parent, | ||
| NetworkNamespace: netNS, | ||
| ScaleCPULimitsToSandbox: shimOpts.ScaleCpuLimitsToSandbox, |
There was a problem hiding this comment.
was this affecting something? or was it a rebase?
Add implementation of network configuration proxy
5 participants
external configuration of a pods network through a set of TTRPC and GRPC services.
Ncproxy relies on other TTRPC/GRPC services to get the information it needs to perform
its actions.
The full set of services are as follows:
NetworkConfigProxy (TTRPC) - This service is exposed by Ncproxy and is used by the shim.
NodeNetworkService (GRPC) - This service is exposed by any application implementing the interface
to the service (/cmd/ncproxy/sdn_nodenetsvc/nodenetsvc.proto).
NetworkConfigProxy (GRPC) - This service is exposed by Ncproxy and is used by a service implementing
the NodeNetworkService GRPC interface.
ComputeAgent (TTRPC) - This service is exposed by the shim and is called by ncproxy.
This is an optional feature that can be enabled by setting the annotation "io.microsoft.network.ncproxy"
and providing an address to a TTRPC service that implements the NetworkConfigProxy TTRPC service defined in
/internal/ncproxyttrpc.
Signed-off-by: Daniel Canter dcanter@microsoft.com