Merge pull request #1806 from katiewasnothere/kabaldau/ncproxy_nodene…

…tsvc

Support v0 and v1 nodenetsvc api for ncproxy

cmd/ncproxy/ncproxy.go

@@ -855,7 +855,7 @@ func (s *ttrpcService) ConfigureNetworking(ctx context.Context, req *ncproxyttrp
 
 	ctx, cancel := context.WithTimeout(ctx, 5*time.Minute)
 	defer cancel()
-	if _, err := nodeNetSvcClient.client.ConfigureNetworking(ctx, netsvcReq); err != nil {
+	if _, err := nodeNetSvcClient.ConfigureNetworking(ctx, netsvcReq); err != nil {
 		return nil, err
 	}
 	return &ncproxyttrpc.ConfigureNetworkingInternalResponse{}, nil

cmd/ncproxy/run.go

@@ -19,6 +19,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/debug"
 	"github.com/Microsoft/hcsshim/internal/log"
 	"github.com/Microsoft/hcsshim/internal/oc"
+	nodenetsvcV0 "github.com/Microsoft/hcsshim/pkg/ncproxy/nodenetsvc/v0"
 	nodenetsvc "github.com/Microsoft/hcsshim/pkg/ncproxy/nodenetsvc/v1"
 	"github.com/containerd/ttrpc"
 	"github.com/pkg/errors"
@@ -27,14 +28,42 @@ import (
 	"go.opencensus.io/plugin/ocgrpc"
 	"go.opencensus.io/trace"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 type nodeNetSvcConn struct {
 	client   nodenetsvc.NodeNetworkServiceClient
+	v0Client nodenetsvcV0.NodeNetworkServiceClient
 	addr     string
 	grpcConn *grpc.ClientConn
 }
 
+func (n *nodeNetSvcConn) ConfigureNetworking(ctx context.Context, req *nodenetsvc.ConfigureNetworkingRequest) (*nodenetsvc.ConfigureNetworkingResponse, error) {
+	// try to use v1 client
+	_, err := n.client.ConfigureNetworking(ctx, req)
+	if err != nil {
+		errCode := status.Code(err)
+		if errCode == codes.Unimplemented {
+			// v1 api call for ConfigureNetworking returned "unimplemented",
+			// try the v0 client instead
+			log.G(ctx).Info("falling back to v0 nodenetsvc api")
+			v0Req := &nodenetsvcV0.ConfigureNetworkingRequest{
+				ContainerID: req.ContainerID,
+				RequestType: nodenetsvcV0.RequestType(req.RequestType),
+			}
+			_, err = n.v0Client.ConfigureNetworking(ctx, v0Req)
+			if err != nil {
+				return nil, err
+			}
+
+			return &nodenetsvc.ConfigureNetworkingResponse{}, nil
+		}
+		return nil, err
+	}
+	return &nodenetsvc.ConfigureNetworkingResponse{}, nil
+}
+
 type computeAgentClient struct {
 	raw *ttrpc.Client
 	computeagent.ComputeAgentService
@@ -207,10 +236,13 @@ func run(clicontext *cli.Context) error {
 
 		log.G(ctx).Infof("Successfully connected to NodeNetworkService at address %s", conf.NodeNetSvcAddr)
 
+		// create a client for both api versions
 		netSvcClient := nodenetsvc.NewNodeNetworkServiceClient(client)
+		v0NetSvcClient := nodenetsvcV0.NewNodeNetworkServiceClient(client)
 		nodeNetSvcClient = &nodeNetSvcConn{
 			addr:     conf.NodeNetSvcAddr,
 			client:   netSvcClient,
+			v0Client: v0NetSvcClient,
 			grpcConn: client,
 		}
 	}

cmd/ncproxy/server_test.go

@@ -12,12 +12,16 @@ import (
 	computeagentMock "github.com/Microsoft/hcsshim/internal/computeagent/mock"
 	ncproxystore "github.com/Microsoft/hcsshim/internal/ncproxy/store"
 	"github.com/Microsoft/hcsshim/internal/ncproxyttrpc"
+	nodenetsvcV0 "github.com/Microsoft/hcsshim/pkg/ncproxy/nodenetsvc/v0"
+	nodenetsvcMockV0 "github.com/Microsoft/hcsshim/pkg/ncproxy/nodenetsvc/v0/mock"
 	nodenetsvc "github.com/Microsoft/hcsshim/pkg/ncproxy/nodenetsvc/v1"
 	nodenetsvcMock "github.com/Microsoft/hcsshim/pkg/ncproxy/nodenetsvc/v1/mock"
 	"github.com/containerd/ttrpc"
 	"github.com/golang/mock/gomock"
 	"github.com/pkg/errors"
 	bolt "go.etcd.io/bbolt"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func TestRegisterComputeAgent(t *testing.T) {
@@ -65,7 +69,7 @@ func TestRegisterComputeAgent(t *testing.T) {
 	}
 }
 
-func TestConfigureNetworking(t *testing.T) {
+func TestConfigureNetworking_V1(t *testing.T) {
 	ctx := context.Background()
 
 	// setup test database
@@ -86,10 +90,14 @@ func TestConfigureNetworking(t *testing.T) {
 	nodeNetCtrl := gomock.NewController(t)
 	defer nodeNetCtrl.Finish()
 	mockedClient := nodenetsvcMock.NewMockNodeNetworkServiceClient(nodeNetCtrl)
+	mockedClientV0 := nodenetsvcMockV0.NewMockNodeNetworkServiceClient(nodeNetCtrl)
 	nodeNetSvcClient = &nodeNetSvcConn{
-		addr:   "",
-		client: mockedClient,
+		addr:     "",
+		client:   mockedClient,
+		v0Client: mockedClientV0,
 	}
+
+	// allow calls to v1 mock api
 	mockedClient.EXPECT().ConfigureNetworking(gomock.Any(), gomock.Any()).Return(&nodenetsvc.ConfigureNetworkingResponse{}, nil).AnyTimes()
 
 	type config struct {
@@ -143,6 +151,90 @@ func TestConfigureNetworking(t *testing.T) {
 	}
 }
 
+func TestConfigureNetworking_V0(t *testing.T) {
+	ctx := context.Background()
+
+	// setup test database
+	tempDir := t.TempDir()
+
+	db, err := bolt.Open(filepath.Join(tempDir, "networkproxy.db.test"), 0600, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer db.Close()
+
+	// create test TTRPC service
+	store := ncproxystore.NewComputeAgentStore(db)
+	agentCache := newComputeAgentCache()
+	tService := newTTRPCService(ctx, agentCache, store)
+
+	// setup mocked client and mocked calls for nodenetsvc
+	nodeNetCtrl := gomock.NewController(t)
+	defer nodeNetCtrl.Finish()
+	mockedClient := nodenetsvcMock.NewMockNodeNetworkServiceClient(nodeNetCtrl)
+	mockedClientV0 := nodenetsvcMockV0.NewMockNodeNetworkServiceClient(nodeNetCtrl)
+	nodeNetSvcClient = &nodeNetSvcConn{
+		addr:     "",
+		client:   mockedClient,
+		v0Client: mockedClientV0,
+	}
+
+	// v1 api calls should return "Unimplemented" so that we will try the v0 code path
+	// allow succcessful calls to v0 api
+	mockedClientV0.EXPECT().ConfigureNetworking(gomock.Any(), gomock.Any()).Return(&nodenetsvcV0.ConfigureNetworkingResponse{}, nil).AnyTimes()
+	mockedClient.EXPECT().ConfigureNetworking(gomock.Any(), gomock.Any()).Return(nil, status.Error(codes.Unimplemented, "mock the v1 api not implemented")).AnyTimes()
+
+	type config struct {
+		name          string
+		containerID   string
+		requestType   ncproxyttrpc.RequestTypeInternal
+		errorExpected bool
+	}
+	containerID := t.Name() + "-containerID"
+	tests := []config{
+		{
+			name:          "Configure Networking setup returns no error",
+			containerID:   containerID,
+			requestType:   ncproxyttrpc.RequestTypeInternal_Setup,
+			errorExpected: false,
+		},
+		{
+			name:          "Configure Networking teardown returns no error",
+			containerID:   containerID,
+			requestType:   ncproxyttrpc.RequestTypeInternal_Teardown,
+			errorExpected: false,
+		},
+		{
+			name:          "Configure Networking setup returns error when container ID is empty",
+			containerID:   "",
+			requestType:   ncproxyttrpc.RequestTypeInternal_Setup,
+			errorExpected: true,
+		},
+		{
+			name:          "Configure Networking setup returns error when request type is not supported",
+			containerID:   containerID,
+			requestType:   3, // unsupported request type
+			errorExpected: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(_ *testing.T) {
+			req := &ncproxyttrpc.ConfigureNetworkingInternalRequest{
+				ContainerID: test.containerID,
+				RequestType: test.requestType,
+			}
+			_, err := tService.ConfigureNetworking(ctx, req)
+			if test.errorExpected && err == nil {
+				t.Fatalf("expected ConfigureNetworking to return an error")
+			}
+			if !test.errorExpected && err != nil {
+				t.Fatalf("expected ConfigureNetworking to return no error, instead got %v", err)
+			}
+		})
+	}
+}
+
 func TestReconnectComputeAgents_Success(t *testing.T) {
 	ctx := context.Background()
 

pkg/ncproxy/nodenetsvc/v0/doc.go

@@ -0,0 +1,3 @@
+package v0
+
+//go:generate go run github.com/golang/mock/mockgen -source=nodenetsvc.pb.go -package=nodenetsvc_v0_mock -destination=mock\nodenetsvc_mock.pb.go