Skip to content

Commit db93762

Browse files
committed
Reproduce the VM re-use issue
We are not deleting the files associated with a container correctly, as #65 suggested. The existing test didn't unveil the issue since the shim is deleting the files along with the files associated with the shim. Signed-off-by: Kazuyoshi Kato <[email protected]>
1 parent 03703f0 commit db93762

File tree

2 files changed

+20
-5
lines changed

2 files changed

+20
-5
lines changed

runtime/service.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,15 @@ func (s *service) Delete(requestCtx context.Context, req *taskAPI.DeleteRequest)
785785
return nil, err
786786
}
787787

788+
dir, err := s.shimDir.BundleLink(req.ID)
789+
if err != nil {
790+
return nil, errors.Wrapf(err, "failed to find the bundle directory of the container: %s", req.ID)
791+
}
792+
793+
if err = os.Remove(dir.RootPath()); err != nil {
794+
return nil, errors.Wrapf(err, "failed to remove the bundle directory of the container: %s", req.ID)
795+
}
796+
788797
return resp, nil
789798
}
790799

runtime/service_integ_test.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -562,10 +562,9 @@ func startAndWaitTask(ctx context.Context, t *testing.T, c containerd.Container)
562562
return stdout.String()
563563
}
564564

565-
func TestCreateContainerWithSameName_Isolated(t *testing.T) {
566-
internal.RequiresIsolation(t)
567-
565+
func testCreateContainerWithSameName(t *testing.T, opts ...oci.SpecOpts) {
568566
ctx := namespaces.WithNamespace(context.Background(), "default")
567+
withNewSpec := containerd.WithNewSpec(append([]oci.SpecOpts{oci.WithProcessArgs("echo", "hello")}, opts...)...)
569568

570569
client, err := containerd.New(containerdSockPath, containerd.WithDefaultRuntime(firecrackerRuntime))
571570
require.NoError(t, err, "unable to create client to containerd service at %s, is containerd running?", containerdSockPath)
@@ -583,7 +582,7 @@ func TestCreateContainerWithSameName_Isolated(t *testing.T) {
583582
containerName,
584583
containerd.WithSnapshotter(naiveSnapshotterName),
585584
containerd.WithNewSnapshot(snapshotName, image),
586-
containerd.WithNewSpec(oci.WithProcessArgs("echo", "hello")),
585+
withNewSpec,
587586
)
588587
require.NoError(t, err, "failed to create container %s", containerName)
589588
require.Equal(t, "hello\n", startAndWaitTask(ctx, t, c1))
@@ -600,7 +599,7 @@ func TestCreateContainerWithSameName_Isolated(t *testing.T) {
600599
containerName,
601600
containerd.WithSnapshotter(naiveSnapshotterName),
602601
containerd.WithNewSnapshot(snapshotName, image),
603-
containerd.WithNewSpec(oci.WithProcessArgs("echo", "hello")),
602+
withNewSpec,
604603
)
605604
require.NoError(t, err, "failed to create container %s", containerName)
606605
require.Equal(t, "hello\n", startAndWaitTask(ctx, t, c2))
@@ -611,3 +610,10 @@ func TestCreateContainerWithSameName_Isolated(t *testing.T) {
611610
_, err = os.Stat(containerPath)
612611
require.True(t, os.IsNotExist(err))
613612
}
613+
614+
func TestCreateContainerWithSameName_Isolated(t *testing.T) {
615+
internal.RequiresIsolation(t)
616+
617+
testCreateContainerWithSameName(t)
618+
testCreateContainerWithSameName(t, firecrackeroci.WithVMID("reuse-same-vm"))
619+
}

0 commit comments

Comments
 (0)