Skip to content

Commit bd10fc3

Browse files
kzyskzys
committed
Handle TTRPC client's error
The Client() method now returns (*ttrpc.Client, error). It wasn't returning the error before. Signed-off-by: Kazuyoshi Kato <[email protected]>
1 parent 886ba60 commit bd10fc3

File tree

7 files changed

+48
-60
lines changed

7 files changed

+48
-60
lines changed

‎firecracker-control/client/client.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ func New(ttrpcAddress string) (*Client, error) {
3434
return nil, errors.Wrap(err, "failed to create ttrpc client")
3535
}
3636

37-
fcClient := fccontrol.NewFirecrackerClient(ttrpcClient.Client())
37+
client, err := ttrpcClient.Client()
38+
if err != nil {
39+
return nil, err
40+
}
41+
fcClient := fccontrol.NewFirecrackerClient(client)
3842

3943
return &Client{
4044
FirecrackerService: fcClient,

‎runtime/benchmark_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020

2121
"github.com/containerd/containerd"
2222
"github.com/containerd/containerd/namespaces"
23-
"github.com/containerd/containerd/pkg/ttrpcutil"
2423
"github.com/firecracker-microvm/firecracker-containerd/proto"
2524
fccontrol "github.com/firecracker-microvm/firecracker-containerd/proto/service/fccontrol/ttrpc"
2625
"github.com/gofrs/uuid"
@@ -63,10 +62,9 @@ func benchmarkCreateAndStopVM(t *testing.T, vcpuCount uint32, kernelArgs string)
6362

6463
ctx := namespaces.WithNamespace(context.Background(), "default")
6564

66-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
67-
require.NoError(t, err, "failed to create ttrpc client")
65+
fcClient, err := newFCControlClient(containerdSockPath)
66+
require.NoError(t, err, "failed to create fccontrol client")
6867

69-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
7068
request := proto.CreateVMRequest{
7169
KernelArgs: kernelArgs,
7270
MachineCfg: &proto.FirecrackerMachineConfiguration{

‎runtime/cni_integ_test.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,13 @@ import (
2828
"github.com/containerd/containerd"
2929
"github.com/containerd/containerd/namespaces"
3030
"github.com/containerd/containerd/oci"
31-
"github.com/containerd/containerd/pkg/ttrpcutil"
3231
"github.com/shirou/gopsutil/cpu"
3332
"github.com/stretchr/testify/assert"
3433
"github.com/stretchr/testify/require"
3534

3635
"github.com/firecracker-microvm/firecracker-containerd/config"
3736
"github.com/firecracker-microvm/firecracker-containerd/internal"
3837
"github.com/firecracker-microvm/firecracker-containerd/proto"
39-
fccontrol "github.com/firecracker-microvm/firecracker-containerd/proto/service/fccontrol/ttrpc"
4038
"github.com/firecracker-microvm/firecracker-containerd/runtime/firecrackeroci"
4139
)
4240

@@ -50,8 +48,8 @@ func TestCNISupport_Isolated(t *testing.T) {
5048
require.NoError(t, err, "unable to create client to containerd service at %s, is containerd running?", containerdSockPath)
5149
defer client.Close()
5250

53-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
54-
require.NoError(t, err, "failed to create ttrpc client")
51+
fcClient, err := newFCControlClient(containerdSockPath)
52+
require.NoError(t, err, "failed to create fccontrol client")
5553

5654
image, err := alpineImage(ctx, client, defaultSnapshotterName)
5755
require.NoError(t, err, "failed to get alpine image")
@@ -89,7 +87,6 @@ func TestCNISupport_Isolated(t *testing.T) {
8987
go func(vmID string) {
9088
defer vmGroup.Done()
9189

92-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
9390
_, err := fcClient.CreateVM(ctx, &proto.CreateVMRequest{
9491
VMID: vmID,
9592
MachineCfg: &proto.FirecrackerMachineConfiguration{
@@ -232,11 +229,9 @@ func TestCNIPlugin_Performance(t *testing.T) {
232229
require.NoError(t, err, "unable to create client to containerd service at %s, is containerd running?", containerdSockPath)
233230
defer client.Close()
234231

235-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
232+
fcClient, err := newFCControlClient(containerdSockPath)
236233
require.NoError(t, err, "failed to create ttrpc client")
237234

238-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
239-
240235
image, err := iperf3Image(ctx, client, defaultSnapshotterName)
241236
require.NoError(t, err, "failed to get iperf3 image")
242237

‎runtime/integ_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/pkg/errors"
2626

2727
"github.com/firecracker-microvm/firecracker-containerd/config"
28+
"github.com/firecracker-microvm/firecracker-containerd/firecracker-control/client"
2829
"github.com/firecracker-microvm/firecracker-containerd/internal"
2930
)
3031

@@ -144,3 +145,7 @@ func runTask(ctx context.Context, c containerd.Container) (*commandResult, error
144145
return nil, errors.New("context cancelled")
145146
}
146147
}
148+
149+
func newFCControlClient(socket string) (*client.Client, error) {
150+
return client.New(socket + ".ttrpc")
151+
}

‎runtime/jailer_integ_test.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,13 @@ import (
2424
"github.com/containerd/containerd"
2525
"github.com/containerd/containerd/namespaces"
2626
"github.com/containerd/containerd/oci"
27-
"github.com/containerd/containerd/pkg/ttrpcutil"
2827
"github.com/opencontainers/runtime-spec/specs-go"
2928
"github.com/stretchr/testify/assert"
3029
"github.com/stretchr/testify/require"
3130

3231
_ "github.com/firecracker-microvm/firecracker-containerd/firecracker-control"
3332
"github.com/firecracker-microvm/firecracker-containerd/internal"
3433
"github.com/firecracker-microvm/firecracker-containerd/proto"
35-
fccontrol "github.com/firecracker-microvm/firecracker-containerd/proto/service/fccontrol/ttrpc"
3634
"github.com/firecracker-microvm/firecracker-containerd/runtime/cpuset"
3735
"github.com/firecracker-microvm/firecracker-containerd/runtime/firecrackeroci"
3836
)
@@ -76,9 +74,6 @@ func testJailer(t *testing.T, jailerConfig *proto.JailerConfig) {
7674
image, err := alpineImage(ctx, client, defaultSnapshotterName)
7775
require.NoError(err, "failed to get alpine image")
7876

79-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
80-
require.NoError(err, "failed to create ttrpc client")
81-
8277
vmID := testNameToVMID(t.Name())
8378

8479
additionalDrive := internal.CreateFSImg(ctx, t, "ext4", internal.FSImgFile{
@@ -116,7 +111,9 @@ func testJailer(t *testing.T, jailerConfig *proto.JailerConfig) {
116111
require.NoError(err, "failed to chown %q", additionalDrive)
117112
}
118113

119-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
114+
fcClient, err := newFCControlClient(containerdSockPath)
115+
require.NoError(err)
116+
120117
_, err = fcClient.CreateVM(ctx, &request)
121118
require.NoError(err)
122119

‎runtime/service.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,8 +367,12 @@ func (s *service) StartShim(shimCtx context.Context, containerID, containerdBina
367367
return "", err
368368
}
369369

370-
fcControlClient := fccontrolTtrpc.NewFirecrackerClient(client.Client())
370+
ttrpcClient, err := client.Client()
371+
if err != nil {
372+
return "", err
373+
}
371374

375+
fcControlClient := fccontrolTtrpc.NewFirecrackerClient(ttrpcClient)
372376
_, err = fcControlClient.CreateVM(shimCtx, &proto.CreateVMRequest{
373377
VMID: s.vmID,
374378
ExitAfterAllTasksDeleted: exitAfterAllTasksDeleted,

‎runtime/service_integ_test.go

Lines changed: 25 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import (
3636
"github.com/containerd/containerd/errdefs"
3737
"github.com/containerd/containerd/namespaces"
3838
"github.com/containerd/containerd/oci"
39-
"github.com/containerd/containerd/pkg/ttrpcutil"
4039
"github.com/containerd/containerd/runtime"
4140
"github.com/containerd/go-runc"
4241
"github.com/containerd/typeurl"
@@ -49,7 +48,6 @@ import (
4948

5049
"github.com/firecracker-microvm/firecracker-containerd/config"
5150
_ "github.com/firecracker-microvm/firecracker-containerd/firecracker-control"
52-
fcClient "github.com/firecracker-microvm/firecracker-containerd/firecracker-control/client"
5351
"github.com/firecracker-microvm/firecracker-containerd/internal"
5452
"github.com/firecracker-microvm/firecracker-containerd/internal/vm"
5553
"github.com/firecracker-microvm/firecracker-containerd/proto"
@@ -298,8 +296,8 @@ func TestMultipleVMs_Isolated(t *testing.T) {
298296
image, err := alpineImage(ctx, client, defaultSnapshotterName)
299297
require.NoError(t, err, "failed to get alpine image")
300298

301-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
302-
require.NoError(t, err, "failed to create ttrpc client")
299+
fcClient, err := newFCControlClient(containerdSockPath)
300+
require.NoError(t, err, "failed to create fccontrol client")
303301

304302
cfg, err := config.LoadConfig("")
305303
require.NoError(t, err, "failed to load config")
@@ -324,7 +322,6 @@ func TestMultipleVMs_Isolated(t *testing.T) {
324322
rootfsPath := cfg.RootDrive
325323

326324
vmIDStr := strconv.Itoa(vmID)
327-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
328325
req := &proto.CreateVMRequest{
329326
VMID: vmIDStr,
330327
RootDrive: &proto.FirecrackerRootDrive{
@@ -593,8 +590,8 @@ func TestLongUnixSocketPath_Isolated(t *testing.T) {
593590

594591
ctx := namespaces.WithNamespace(context.Background(), namespace)
595592

596-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
597-
require.NoError(t, err, "failed to create ttrpc client")
593+
fcClient, err := newFCControlClient(containerdSockPath)
594+
require.NoError(t, err, "failed to create fccontrol client")
598595

599596
subtests := []struct {
600597
name string
@@ -620,7 +617,6 @@ func TestLongUnixSocketPath_Isolated(t *testing.T) {
620617
},
621618
}
622619

623-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
624620
for _, subtest := range subtests {
625621
request := subtest.request
626622
vmID := request.VMID
@@ -700,10 +696,9 @@ func TestStubBlockDevices_Isolated(t *testing.T) {
700696
containerName := fmt.Sprintf("%s-%d", t.Name(), time.Now().UnixNano())
701697
snapshotName := fmt.Sprintf("%s-snapshot", containerName)
702698

703-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
704-
require.NoError(t, err, "failed to create ttrpc client")
699+
fcClient, err := newFCControlClient(containerdSockPath)
700+
require.NoError(t, err, "failed to create fccontrol client")
705701

706-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
707702
_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{
708703
VMID: strconv.Itoa(vmID),
709704
NetworkInterfaces: []*proto.FirecrackerNetworkInterface{
@@ -823,12 +818,11 @@ func startAndWaitTask(ctx context.Context, t *testing.T, c containerd.Container)
823818
func testCreateContainerWithSameName(t *testing.T, vmID string) {
824819
ctx := namespaces.WithNamespace(context.Background(), "default")
825820

826-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
827-
require.NoError(t, err, "failed to create ttrpc client")
828-
829821
// Explicitly specify Container Count = 2 to workaround #230
830822
if len(vmID) != 0 {
831-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
823+
fcClient, err := newFCControlClient(containerdSockPath)
824+
require.NoError(t, err, "failed to create fccontrol client")
825+
832826
_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{
833827
VMID: vmID,
834828
ContainerCount: 2,
@@ -967,7 +961,7 @@ func TestDriveMount_Isolated(t *testing.T) {
967961
ctrdClient, err := containerd.New(containerdSockPath, containerd.WithDefaultRuntime(firecrackerRuntime))
968962
require.NoError(t, err, "unable to create client to containerd service at %s, is containerd running?", containerdSockPath)
969963

970-
fcClient, err := fcClient.New(containerdSockPath + ".ttrpc")
964+
fcClient, err := newFCControlClient(containerdSockPath)
971965
require.NoError(t, err, "failed to create fccontrol client")
972966

973967
image, err := alpineImage(ctx, ctrdClient, defaultSnapshotterName)
@@ -1135,7 +1129,7 @@ func TestDriveMountFails_Isolated(t *testing.T) {
11351129
ctx, cancel := context.WithTimeout(namespaces.WithNamespace(context.Background(), defaultNamespace), testTimeout)
11361130
defer cancel()
11371131

1138-
fcClient, err := fcClient.New(containerdSockPath + ".ttrpc")
1132+
fcClient, err := newFCControlClient(containerdSockPath)
11391133
require.NoError(t, err, "failed to create fccontrol client")
11401134

11411135
testImgHostPath := internal.CreateFSImg(ctx, t, "ext4", internal.FSImgFile{
@@ -1197,9 +1191,8 @@ func TestUpdateVMMetadata_Isolated(t *testing.T) {
11971191
require.NoError(t, err, "unable to create client to containerd service at %s, is containerd running?", containerdSockPath)
11981192
defer client.Close()
11991193

1200-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
1201-
require.NoError(t, err, "failed to create ttrpc client")
1202-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
1194+
fcClient, err := newFCControlClient(containerdSockPath)
1195+
require.NoError(t, err, "failed to create fccontrol client")
12031196

12041197
cniNetworkName := "fcnet-test"
12051198
err = writeCNIConf("/etc/cni/conf.d/fcnet-test.conflist",
@@ -1450,9 +1443,6 @@ func TestStopVM_Isolated(t *testing.T) {
14501443
image, err := alpineImage(ctx, client, defaultSnapshotterName)
14511444
require.NoError(err, "failed to get alpine image")
14521445

1453-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
1454-
require.NoError(err, "failed to create ttrpc client")
1455-
14561446
tests := []struct {
14571447
name string
14581448
createVMRequest proto.CreateVMRequest
@@ -1550,6 +1540,9 @@ func TestStopVM_Isolated(t *testing.T) {
15501540
},
15511541
}
15521542

1543+
fcClient, err := newFCControlClient(containerdSockPath)
1544+
require.NoError(err)
1545+
15531546
for _, test := range tests {
15541547
test := test
15551548

@@ -1559,7 +1552,6 @@ func TestStopVM_Isolated(t *testing.T) {
15591552

15601553
vmID := createVMRequest.VMID
15611554

1562-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
15631555
_, err = fcClient.CreateVM(ctx, &createVMRequest)
15641556
require.NoError(err)
15651557

@@ -1637,12 +1629,11 @@ func TestExec_Isolated(t *testing.T) {
16371629
image, err := alpineImage(ctx, client, defaultSnapshotterName)
16381630
require.NoError(t, err, "failed to get alpine image")
16391631

1640-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
1632+
fcClient, err := newFCControlClient(containerdSockPath)
16411633
require.NoError(t, err, "failed to create ttrpc client")
16421634

16431635
vmID := testNameToVMID(t.Name())
16441636

1645-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
16461637
_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{VMID: vmID})
16471638
require.NoError(t, err)
16481639

@@ -1757,12 +1748,11 @@ func TestEvents_Isolated(t *testing.T) {
17571748
image, err := alpineImage(ctx, client, defaultSnapshotterName)
17581749
require.NoError(err, "failed to get alpine image")
17591750

1760-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
1761-
require.NoError(err, "failed to create ttrpc client")
1762-
17631751
vmID := testNameToVMID(t.Name())
17641752

1765-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
1753+
fcClient, err := newFCControlClient(containerdSockPath)
1754+
require.NoError(err, "failed to create ttrpc client")
1755+
17661756
_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{VMID: vmID})
17671757
require.NoError(err)
17681758

@@ -1835,12 +1825,11 @@ func TestOOM_Isolated(t *testing.T) {
18351825
image, err := alpineImage(ctx, client, defaultSnapshotterName)
18361826
require.NoError(t, err, "failed to get alpine image")
18371827

1838-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
1839-
require.NoError(t, err, "failed to create ttrpc client")
1840-
18411828
vmID := testNameToVMID(t.Name())
18421829

1843-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
1830+
fcClient, err := newFCControlClient(containerdSockPath)
1831+
require.NoError(t, err, "failed to create ttrpc client")
1832+
18441833
_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{VMID: vmID})
18451834
require.NoError(t, err)
18461835

@@ -1919,11 +1908,9 @@ func TestCreateVM_Isolated(t *testing.T) {
19191908

19201909
ctx := namespaces.WithNamespace(context.Background(), "default")
19211910

1922-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
1911+
fcClient, err := newFCControlClient(containerdSockPath)
19231912
require.NoError(t, err, "failed to create ttrpc client")
19241913

1925-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
1926-
19271914
type subtest struct {
19281915
name string
19291916
request proto.CreateVMRequest
@@ -2053,11 +2040,9 @@ func TestPauseResume_Isolated(t *testing.T) {
20532040

20542041
ctx := namespaces.WithNamespace(context.Background(), "default")
20552042

2056-
pluginClient, err := ttrpcutil.NewClient(containerdSockPath + ".ttrpc")
2043+
fcClient, err := newFCControlClient(containerdSockPath)
20572044
require.NoError(t, err, "failed to create ttrpc client")
20582045

2059-
fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
2060-
20612046
subtests := []struct {
20622047
name string
20632048
state func(ctx context.Context, resp *proto.CreateVMResponse)

0 commit comments

Comments
 (0)