From e699298d58fdefa30a0c9713571a9f4eca4dfa75 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Thu, 6 Mar 2025 15:09:40 -0800 Subject: [PATCH] Add Container.RestoreInTest() to handle known Docker bugs. This can be used by all test users. Avoids duplicated code. We can handle all known issues in one place. There is a Docker bug which causes restore to fail sporadically. See https://github.com/moby/moby/issues/42900. This has been broken at least since Docker v19.03.12 (when the issue was reported) and was fixed in v25.0.4. Added the handling for this issue. Also got rid of the testutil.Poll() around restore. That can hide gVisor restore flakiness issues. That was added in 0990ef751723 ("Make checkpoint/restore e2e test less flaky"). The original sleep has been restored. PiperOrigin-RevId: 734303878 --- pkg/test/dockerutil/container.go | 15 +++++++++++++++ test/e2e/integration_runtime_test.go | 8 +++----- test/e2e/integration_test.go | 13 ++++--------- test/gpu/sr_test.go | 8 +++----- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/pkg/test/dockerutil/container.go b/pkg/test/dockerutil/container.go index d763f45d4..410af3d61 100644 --- a/pkg/test/dockerutil/container.go +++ b/pkg/test/dockerutil/container.go @@ -28,6 +28,7 @@ import ( "sort" "strconv" "strings" + "testing" "time" "github.com/docker/docker/api/types" @@ -410,6 +411,20 @@ func (c *Container) Restore(ctx context.Context, name string) error { return c.client.ContainerStart(ctx, c.id, container.StartOptions{CheckpointID: name}) } +// RestoreInTest is the same as Restore, except that it handles known issues +// while testing. +func (c *Container) RestoreInTest(ctx context.Context, t *testing.T, name string) { + // TODO(b/143498576): Remove sleep after github.com/moby/moby/issues/38963 is fixed. + time.Sleep(2 * time.Second) + if err := c.Restore(ctx, name); err != nil { + if regexp.MustCompile("failed to upload checkpoint to containerd: commit failed: content sha256:.*: already exists").MatchString(err.Error()) || + regexp.MustCompile("failed to create task for container: content digest .*: not found: unknown").MatchString(err.Error()) { + t.Skip("Skipping restore due to known issue: https://github.com/moby/moby/issues/42900") + } + t.Fatalf("docker restore failed: %v", err) + } +} + // CheckpointResume is analogous to 'docker checkpoint'. func (c *Container) CheckpointResume(ctx context.Context, name string) error { return c.client.CheckpointCreate(ctx, c.Name, checkpoint.CreateOptions{CheckpointID: name, Exit: false}) diff --git a/test/e2e/integration_runtime_test.go b/test/e2e/integration_runtime_test.go index b17693648..26751f9ad 100644 --- a/test/e2e/integration_runtime_test.go +++ b/test/e2e/integration_runtime_test.go @@ -287,7 +287,8 @@ func TestOverlayCheckpointRestore(t *testing.T) { } // Create a snapshot. - if err := d.Checkpoint(ctx, "test"); err != nil { + const ckptName = "test" + if err := d.Checkpoint(ctx, ckptName); err != nil { t.Fatalf("docker checkpoint failed: %v", err) } if err := d.WaitTimeout(ctx, defaultWait); err != nil { @@ -295,10 +296,7 @@ func TestOverlayCheckpointRestore(t *testing.T) { } // Restore the snapshot. - // TODO(b/143498576): Remove Poll after github.com/moby/moby/issues/38963 is fixed. - if err := testutil.Poll(func() error { return d.Restore(ctx, "test") }, defaultWait); err != nil { - t.Fatalf("docker restore failed: %v", err) - } + d.RestoreInTest(ctx, t, ckptName) // Make sure the files are restored in the overlay. if got, err := d.Exec(ctx, dockerutil.ExecOpts{}, "cat", "/file"); err != nil || got != "rootfs\n" { diff --git a/test/e2e/integration_test.go b/test/e2e/integration_test.go index 9d755f380..2243d3674 100644 --- a/test/e2e/integration_test.go +++ b/test/e2e/integration_test.go @@ -196,17 +196,15 @@ func TestCheckpointRestore(t *testing.T) { } // Create a snapshot. - if err := d.Checkpoint(ctx, "test"); err != nil { + const ckptName = "test" + if err := d.Checkpoint(ctx, ckptName); err != nil { t.Fatalf("docker checkpoint failed: %v", err) } if err := d.WaitTimeout(ctx, defaultWait); err != nil { t.Fatalf("wait failed: %v", err) } - // TODO(b/143498576): Remove Poll after github.com/moby/moby/issues/38963 is fixed. - if err := testutil.Poll(func() error { return d.Restore(ctx, "test") }, defaultWait); err != nil { - t.Fatalf("docker restore failed: %v", err) - } + d.RestoreInTest(ctx, t, ckptName) // Find container IP address. ip, err := d.FindIP(ctx, false) @@ -1253,10 +1251,7 @@ func testCheckpointRestoreListeningConnection(ctx context.Context, t *testing.T, if err := d.WaitTimeout(ctx, defaultWait); err != nil { t.Fatalf("wait failed: %v", err) } - // TODO(b/143498576): Remove Poll after github.com/moby/moby/issues/38963 is fixed. - if err := testutil.Poll(func() error { return d.Restore(ctx, checkpointFile) }, defaultWait); err != nil { - t.Fatalf("docker restore failed: %v", err) - } + d.RestoreInTest(ctx, t, checkpointFile) var ( newIP net.IP diff --git a/test/gpu/sr_test.go b/test/gpu/sr_test.go index b08498a61..95fcb2b7d 100644 --- a/test/gpu/sr_test.go +++ b/test/gpu/sr_test.go @@ -60,7 +60,8 @@ func TestGPUCheckpointRestore(t *testing.T) { } // Create a snapshot. - if err := c.Checkpoint(ctx, "test"); err != nil { + const ckptName = "test" + if err := c.Checkpoint(ctx, ckptName); err != nil { t.Fatalf("docker checkpoint failed: %v", err) } if err := c.WaitTimeout(ctx, time.Minute); err != nil { @@ -68,10 +69,7 @@ func TestGPUCheckpointRestore(t *testing.T) { } // Restore the snapshot. - // TODO(b/143498576): Remove Poll after github.com/moby/moby/issues/38963 is fixed. - if err := testutil.Poll(func() error { return c.Restore(ctx, "test") }, time.Minute); err != nil { - t.Fatalf("docker restore failed: %v", err) - } + c.RestoreInTest(ctx, t, ckptName) // Run the vector add program again to ensure GPUs are functional. if _, err := c.Exec(ctx, dockerutil.ExecOpts{}, vectorAddCmd...); err != nil {