From 3af4cb6b0c656ec09c8d2f8e04658b6d7eb2adaf Mon Sep 17 00:00:00 2001 From: Kathryn Baldauf Date: Tue, 24 Aug 2021 14:38:37 -0700 Subject: [PATCH] Move containerID to compute agent cache from being a global and make it a sync map Signed-off-by: Kathryn Baldauf --- cmd/ncproxy/ncproxy.go | 66 +++++++++++++++++++++++++++++++++--------- cmd/ncproxy/run.go | 3 -- cmd/ncproxy/server.go | 8 +++-- 3 files changed, 58 insertions(+), 19 deletions(-) diff --git a/cmd/ncproxy/ncproxy.go b/cmd/ncproxy/ncproxy.go index 3ba1cac281..8ca1307801 100644 --- a/cmd/ncproxy/ncproxy.go +++ b/cmd/ncproxy/ncproxy.go @@ -24,8 +24,42 @@ import ( "google.golang.org/grpc/status" ) +type computeAgentCache struct { + // lock for synchronizing read/write access to `cache` + rw sync.RWMutex + // mapping of container ID to shim compute agent ttrpc service + cache map[string]computeagent.ComputeAgentService +} + +func newComputeAgentCache() *computeAgentCache { + return &computeAgentCache{ + cache: make(map[string]computeagent.ComputeAgentService), + } +} + +func (c *computeAgentCache) get(cid string) (computeagent.ComputeAgentService, bool) { + c.rw.RLock() + defer c.rw.RUnlock() + result, ok := c.cache[cid] + return result, ok +} + +func (c *computeAgentCache) put(cid string, agent computeagent.ComputeAgentService) { + c.rw.Lock() + defer c.rw.Unlock() + c.cache[cid] = agent +} + // GRPC service exposed for use by a Node Network Service. -type grpcService struct{} +type grpcService struct { + containerIDToComputeAgent *computeAgentCache +} + +func newGRPCService(agentCache *computeAgentCache) *grpcService { + return &grpcService{ + containerIDToComputeAgent: agentCache, + } +} var _ ncproxygrpc.NetworkConfigProxyServer = &grpcService{} @@ -42,13 +76,13 @@ func (s *grpcService) AddNIC(ctx context.Context, req *ncproxygrpc.AddNICRequest if req.ContainerID == "" || req.EndpointName == "" || req.NicID == "" { return nil, status.Errorf(codes.InvalidArgument, "received empty field in request: %+v", req) } - if client, ok := containerIDToShim[req.ContainerID]; ok { + if agent, ok := s.containerIDToComputeAgent.get(req.ContainerID); ok { caReq := &computeagent.AddNICInternalRequest{ ContainerID: req.ContainerID, NicID: req.NicID, EndpointName: req.EndpointName, } - if _, err := client.AddNIC(ctx, caReq); err != nil { + if _, err := agent.AddNIC(ctx, caReq); err != nil { return nil, err } return &ncproxygrpc.AddNICResponse{}, nil @@ -72,7 +106,7 @@ func (s *grpcService) ModifyNIC(ctx context.Context, req *ncproxygrpc.ModifyNICR return nil, status.Error(codes.InvalidArgument, "received empty field in request") } - if client, ok := containerIDToShim[req.ContainerID]; ok { + if agent, ok := s.containerIDToComputeAgent.get(req.ContainerID); ok { caReq := &computeagent.ModifyNICInternalRequest{ NicID: req.NicID, EndpointName: req.EndpointName, @@ -112,7 +146,7 @@ func (s *grpcService) ModifyNIC(ctx context.Context, req *ncproxygrpc.ModifyNICR // // To turn on iov offload, the reverse order is used. if req.IovPolicySettings.IovOffloadWeight == 0 { - if _, err := client.ModifyNIC(ctx, caReq); err != nil { + if _, err := agent.ModifyNIC(ctx, caReq); err != nil { return nil, err } if err := modifyEndpoint(ctx, ep.Id, policies, hcn.RequestTypeUpdate); err != nil { @@ -125,7 +159,7 @@ func (s *grpcService) ModifyNIC(ctx context.Context, req *ncproxygrpc.ModifyNICR if err := modifyEndpoint(ctx, ep.Id, policies, hcn.RequestTypeUpdate); err != nil { return nil, errors.Wrap(err, "failed to modify network adapter") } - if _, err := client.ModifyNIC(ctx, caReq); err != nil { + if _, err := agent.ModifyNIC(ctx, caReq); err != nil { return nil, err } } @@ -148,13 +182,13 @@ func (s *grpcService) DeleteNIC(ctx context.Context, req *ncproxygrpc.DeleteNICR if req.ContainerID == "" || req.EndpointName == "" || req.NicID == "" { return nil, status.Errorf(codes.InvalidArgument, "received empty field in request: %+v", req) } - if client, ok := containerIDToShim[req.ContainerID]; ok { + if agent, ok := s.containerIDToComputeAgent.get(req.ContainerID); ok { caReq := &computeagent.DeleteNICInternalRequest{ ContainerID: req.ContainerID, NicID: req.NicID, EndpointName: req.EndpointName, } - if _, err := client.DeleteNIC(ctx, caReq); err != nil { + if _, err := agent.DeleteNIC(ctx, caReq); err != nil { if err == uvm.ErrNICNotFound || err == uvm.ErrNetNSNotFound { return nil, status.Errorf(codes.NotFound, "failed to remove endpoint %q from namespace %q", req.EndpointName, req.NicID) } @@ -553,10 +587,15 @@ func (s *grpcService) GetNetworks(ctx context.Context, req *ncproxygrpc.GetNetwo }, nil } -// TTRPC service exposed for use by the shim. Holds a mutex for updating map of -// client connections. +// TTRPC service exposed for use by the shim. type ttrpcService struct { - m sync.Mutex + containerIDToComputeAgent *computeAgentCache +} + +func newTTRPCService(agentCache *computeAgentCache) *ttrpcService { + return &ttrpcService{ + containerIDToComputeAgent: agentCache, + } } func (s *ttrpcService) RegisterComputeAgent(ctx context.Context, req *ncproxyttrpc.RegisterComputeAgentRequest) (_ *ncproxyttrpc.RegisterComputeAgentResponse, err error) { @@ -579,9 +618,8 @@ func (s *ttrpcService) RegisterComputeAgent(ctx context.Context, req *ncproxyttr ) // Add to global client map if connection succeeds. Don't check if there's already a map entry // just overwrite as the client may have changed the address of the config agent. - s.m.Lock() - defer s.m.Unlock() - containerIDToShim[req.ContainerID] = computeagent.NewComputeAgentClient(client) + s.containerIDToComputeAgent.put(req.ContainerID, computeagent.NewComputeAgentClient(client)) + return &ncproxyttrpc.RegisterComputeAgentResponse{}, nil } diff --git a/cmd/ncproxy/run.go b/cmd/ncproxy/run.go index 3328b1f3fd..dbc714fb14 100644 --- a/cmd/ncproxy/run.go +++ b/cmd/ncproxy/run.go @@ -15,7 +15,6 @@ import ( "github.com/Microsoft/go-winio/pkg/etwlogrus" "github.com/Microsoft/go-winio/pkg/guid" "github.com/Microsoft/hcsshim/cmd/ncproxy/nodenetsvc" - "github.com/Microsoft/hcsshim/internal/computeagent" "github.com/Microsoft/hcsshim/internal/debug" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/oc" @@ -33,8 +32,6 @@ type nodeNetSvcConn struct { } var ( - // Global mapping of network namespace ID to shim compute agent ttrpc service. - containerIDToShim = make(map[string]computeagent.ComputeAgentService) // Global object representing the connection to the node network service that // ncproxy will be talking to. nodeNetSvcClient *nodeNetSvcConn diff --git a/cmd/ncproxy/server.go b/cmd/ncproxy/server.go index 10333a8b33..6bf02886c9 100644 --- a/cmd/ncproxy/server.go +++ b/cmd/ncproxy/server.go @@ -36,8 +36,12 @@ func newServer(ctx context.Context, conf *config) (*server, error) { } func (s *server) setup(ctx context.Context) (net.Listener, net.Listener, error) { - ncproxygrpc.RegisterNetworkConfigProxyServer(s.grpc, &grpcService{}) - ncproxyttrpc.RegisterNetworkConfigProxyService(s.ttrpc, &ttrpcService{}) + agentCache := newComputeAgentCache() + gService := newGRPCService(agentCache) + ncproxygrpc.RegisterNetworkConfigProxyServer(s.grpc, gService) + + tService := newTTRPCService(agentCache) + ncproxyttrpc.RegisterNetworkConfigProxyService(s.ttrpc, tService) ttrpcListener, err := winio.ListenPipe(s.conf.TTRPCAddr, nil) if err != nil {