Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support v0 and v1 nodenetsvc api for ncproxy #1806

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/ncproxy/ncproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions cmd/ncproxy/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
helsaawy marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down Expand Up @@ -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,
}
}
Expand Down
98 changes: 95 additions & 3 deletions cmd/ncproxy/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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()

Expand Down
3 changes: 3 additions & 0 deletions pkg/ncproxy/nodenetsvc/v0/doc.go
Original file line number Diff line number Diff line change
@@ -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
Loading