From 9d05ae03b7b6a6c993a8dfd4775764819ea1bede Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Fri, 14 Mar 2025 11:24:38 -0700 Subject: [PATCH 1/3] Revert "Remove test for issue 10467" This reverts commit 12762891d6c4e0e91384c01650c102d911f9a915. Signed-off-by: Derek McGowan --- integration/issue10467_linux_test.go | 117 +++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 integration/issue10467_linux_test.go diff --git a/integration/issue10467_linux_test.go b/integration/issue10467_linux_test.go new file mode 100644 index 0000000000..96a8d4ed51 --- /dev/null +++ b/integration/issue10467_linux_test.go @@ -0,0 +1,117 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package integration + +import ( + "fmt" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/containerd/continuity/fs" + "github.com/stretchr/testify/require" + "go.etcd.io/bbolt" +) + +func TestIssue10467(t *testing.T) { + latestVersion := "v1.7.20" + + releaseBinDir := t.TempDir() + + downloadReleaseBinary(t, releaseBinDir, latestVersion) + + t.Logf("Install config for release %s", latestVersion) + workDir := t.TempDir() + previousReleaseCtrdConfig(t, releaseBinDir, workDir) + + t.Log("Starting the previous release's containerd") + previousCtrdBinPath := filepath.Join(releaseBinDir, "bin", "containerd") + previousProc := newCtrdProc(t, previousCtrdBinPath, workDir, []string{"ENABLE_CRI_SANDBOXES=yes"}) + + boltdbPath := filepath.Join(workDir, "root", "io.containerd.metadata.v1.bolt", "meta.db") + + ctrdLogPath := previousProc.logPath() + t.Cleanup(func() { + if t.Failed() { + dumpFileContent(t, ctrdLogPath) + } + }) + + require.NoError(t, previousProc.isReady()) + + needToCleanup := true + t.Cleanup(func() { + if t.Failed() && needToCleanup { + t.Logf("Try to cleanup leaky pods") + cleanupPods(t, previousProc.criRuntimeService(t)) + } + }) + + t.Log("Prepare pods for current release") + upgradeCaseFunc, hookFunc := shouldManipulateContainersInPodAfterUpgrade(t, previousProc.criRuntimeService(t), previousProc.criImageService(t)) + needToCleanup = false + require.Nil(t, hookFunc) + + t.Log("Gracefully stop previous release's containerd process") + require.NoError(t, previousProc.kill(syscall.SIGTERM)) + require.NoError(t, previousProc.wait(5*time.Minute)) + + t.Logf("%s should have bucket k8s.io in root", boltdbPath) + db, err := bbolt.Open(boltdbPath, 0600, &bbolt.Options{ReadOnly: true}) + require.NoError(t, err) + require.NoError(t, db.View(func(tx *bbolt.Tx) error { + if tx.Bucket([]byte("k8s.io")) == nil { + return fmt.Errorf("expected k8s.io bucket") + } + return nil + })) + require.NoError(t, db.Close()) + + t.Log("Install default config for current release") + currentReleaseCtrdDefaultConfig(t, workDir) + + t.Log("Starting the current release's containerd") + currentProc := newCtrdProc(t, "containerd", workDir, nil) + require.NoError(t, currentProc.isReady()) + + t.Cleanup(func() { + t.Log("Cleanup all the pods") + cleanupPods(t, currentProc.criRuntimeService(t)) + + t.Log("Stopping current release's containerd process") + require.NoError(t, currentProc.kill(syscall.SIGTERM)) + require.NoError(t, currentProc.wait(5*time.Minute)) + }) + + t.Logf("%s should not have bucket k8s.io in root after restart", boltdbPath) + copiedBoltdbPath := filepath.Join(t.TempDir(), "meta.db.new") + require.NoError(t, fs.CopyFile(copiedBoltdbPath, boltdbPath)) + + db, err = bbolt.Open(copiedBoltdbPath, 0600, &bbolt.Options{ReadOnly: true}) + require.NoError(t, err) + require.NoError(t, db.View(func(tx *bbolt.Tx) error { + if tx.Bucket([]byte("k8s.io")) != nil { + return fmt.Errorf("unexpected k8s.io bucket") + } + return nil + })) + require.NoError(t, db.Close()) + + t.Log("Verifing") + upgradeCaseFunc(t, currentProc.criRuntimeService(t), currentProc.criImageService(t)) +} From 713f753e5df9bab231fb2f85de6ef05ea63e2dea Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Sat, 15 Mar 2025 00:09:01 -0700 Subject: [PATCH 2/3] Update release upgrade tests to test 1.7 and 2.0 Fix 1.7 config to match previous version which allowed upgrade to 2.x. Signed-off-by: Derek McGowan --- integration/issue10467_linux_test.go | 4 +- integration/release_upgrade_linux_test.go | 69 ++++++++++++++----- .../release_upgrade_utils_linux_test.go | 31 ++------- 3 files changed, 56 insertions(+), 48 deletions(-) diff --git a/integration/issue10467_linux_test.go b/integration/issue10467_linux_test.go index 96a8d4ed51..28b77d7eec 100644 --- a/integration/issue10467_linux_test.go +++ b/integration/issue10467_linux_test.go @@ -37,7 +37,7 @@ func TestIssue10467(t *testing.T) { t.Logf("Install config for release %s", latestVersion) workDir := t.TempDir() - previousReleaseCtrdConfig(t, releaseBinDir, workDir) + oneSevenCtrdConfig(t, releaseBinDir, workDir) t.Log("Starting the previous release's containerd") previousCtrdBinPath := filepath.Join(releaseBinDir, "bin", "containerd") @@ -63,7 +63,7 @@ func TestIssue10467(t *testing.T) { }) t.Log("Prepare pods for current release") - upgradeCaseFunc, hookFunc := shouldManipulateContainersInPodAfterUpgrade(t, previousProc.criRuntimeService(t), previousProc.criImageService(t)) + upgradeCaseFunc, hookFunc := shouldManipulateContainersInPodAfterUpgrade(t, 2, previousProc.criRuntimeService(t), previousProc.criImageService(t)) needToCleanup = false require.Nil(t, hookFunc) diff --git a/integration/release_upgrade_linux_test.go b/integration/release_upgrade_linux_test.go index 879a860438..c1504cf8e4 100644 --- a/integration/release_upgrade_linux_test.go +++ b/integration/release_upgrade_linux_test.go @@ -49,19 +49,23 @@ type beforeUpgradeHookFunc func(*testing.T) // TODO: Support Windows func TestUpgrade(t *testing.T) { - previousReleaseBinDir := t.TempDir() - downloadPreviousLatestReleaseBinary(t, previousReleaseBinDir) - - t.Run("recover", runUpgradeTestCase(previousReleaseBinDir, shouldRecoverAllThePodsAfterUpgrade)) - t.Run("exec", runUpgradeTestCase(previousReleaseBinDir, execToExistingContainer)) - t.Run("manipulate", runUpgradeTestCase(previousReleaseBinDir, shouldManipulateContainersInPodAfterUpgrade)) - t.Run("recover-images", runUpgradeTestCase(previousReleaseBinDir, shouldRecoverExistingImages)) - t.Run("metrics", runUpgradeTestCase(previousReleaseBinDir, shouldParseMetricDataCorrectly)) + for _, version := range []string{"1.7", "2.0"} { + t.Run(version, func(t *testing.T) { + previousReleaseBinDir := t.TempDir() + downloadPreviousLatestReleaseBinary(t, version, previousReleaseBinDir) + t.Run("recover", runUpgradeTestCase(version, previousReleaseBinDir, shouldRecoverAllThePodsAfterUpgrade)) + t.Run("exec", runUpgradeTestCase(version, previousReleaseBinDir, execToExistingContainer)) + t.Run("manipulate", runUpgradeTestCase(version, previousReleaseBinDir, shouldManipulateContainersInPodAfterUpgrade)) + t.Run("recover-images", runUpgradeTestCase(version, previousReleaseBinDir, shouldRecoverExistingImages)) + t.Run("metrics", runUpgradeTestCase(version, previousReleaseBinDir, shouldParseMetricDataCorrectly)) + }) + } } func runUpgradeTestCase( + previousVersion string, previousReleaseBinDir string, - setupUpgradeVerifyCase func(*testing.T, cri.RuntimeService, cri.ImageManagerService) (upgradeVerifyCaseFunc, beforeUpgradeHookFunc), + setupUpgradeVerifyCase func(*testing.T, int, cri.RuntimeService, cri.ImageManagerService) (upgradeVerifyCaseFunc, beforeUpgradeHookFunc), ) func(t *testing.T) { return func(t *testing.T) { // NOTE: Using t.TempDir() here is to ensure there are no leaky @@ -69,7 +73,14 @@ func runUpgradeTestCase( workDir := t.TempDir() t.Log("Install config for previous release") - previousReleaseCtrdConfig(t, previousReleaseBinDir, workDir) + var taskVersion int + if previousVersion == "1.7" { + oneSevenCtrdConfig(t, previousReleaseBinDir, workDir) + taskVersion = 2 + } else { + previousReleaseCtrdConfig(t, previousReleaseBinDir, workDir) + taskVersion = 3 + } t.Log("Starting the previous release's containerd") previousCtrdBinPath := filepath.Join(previousReleaseBinDir, "bin", "containerd") @@ -91,7 +102,7 @@ func runUpgradeTestCase( }) t.Log("Prepare pods for current release") - upgradeCaseFunc, hookFunc := setupUpgradeVerifyCase(t, previousProc.criRuntimeService(t), previousProc.criImageService(t)) + upgradeCaseFunc, hookFunc := setupUpgradeVerifyCase(t, taskVersion, previousProc.criRuntimeService(t), previousProc.criImageService(t)) needToCleanup = false t.Log("Gracefully stop previous release's containerd process") @@ -123,7 +134,7 @@ func runUpgradeTestCase( } } -func shouldRecoverAllThePodsAfterUpgrade(t *testing.T, +func shouldRecoverAllThePodsAfterUpgrade(t *testing.T, taskVersion int, rSvc cri.RuntimeService, iSvc cri.ImageManagerService) (upgradeVerifyCaseFunc, beforeUpgradeHookFunc) { var busyboxImage = images.Get(images.BusyBox) @@ -152,7 +163,8 @@ func shouldRecoverAllThePodsAfterUpgrade(t *testing.T, criruntime.ContainerState_CONTAINER_RUNNING, WithCommand("sleep", "3d")) - thirdPodShimPid := int(thirdPodCtx.shimPid()) + // TODO: Need to pass in task version + thirdPodShimPid := int(thirdPodCtx.shimPid(taskVersion)) hookFunc := func(t *testing.T) { // Kill the shim after stop previous containerd process @@ -211,7 +223,7 @@ func shouldRecoverAllThePodsAfterUpgrade(t *testing.T, }, hookFunc } -func execToExistingContainer(t *testing.T, +func execToExistingContainer(t *testing.T, _ int, rSvc cri.RuntimeService, iSvc cri.ImageManagerService) (upgradeVerifyCaseFunc, beforeUpgradeHookFunc) { var busyboxImage = images.Get(images.BusyBox) @@ -276,7 +288,7 @@ func getFileSize(t *testing.T, filePath string) int64 { return st.Size() } -func shouldManipulateContainersInPodAfterUpgrade(t *testing.T, +func shouldManipulateContainersInPodAfterUpgrade(t *testing.T, _ int, rSvc cri.RuntimeService, iSvc cri.ImageManagerService) (upgradeVerifyCaseFunc, beforeUpgradeHookFunc) { var busyboxImage = images.Get(images.BusyBox) @@ -375,7 +387,7 @@ func shouldManipulateContainersInPodAfterUpgrade(t *testing.T, }, nil } -func shouldRecoverExistingImages(t *testing.T, +func shouldRecoverExistingImages(t *testing.T, _ int, _ cri.RuntimeService, iSvc cri.ImageManagerService) (upgradeVerifyCaseFunc, beforeUpgradeHookFunc) { images := []string{images.Get(images.BusyBox), images.Get(images.Alpine)} @@ -398,7 +410,7 @@ func shouldRecoverExistingImages(t *testing.T, // shouldParseMetricDataCorrectly is to check new release containerd can parse // metric data from existing shim created by previous release. -func shouldParseMetricDataCorrectly(t *testing.T, +func shouldParseMetricDataCorrectly(t *testing.T, _ int, rSvc cri.RuntimeService, iSvc cri.ImageManagerService) (upgradeVerifyCaseFunc, beforeUpgradeHookFunc) { imageName := images.Get(images.BusyBox) @@ -537,7 +549,7 @@ func (pCtx *podTCtx) containerDataDir(cntrID string) string { } // shimPid returns shim's pid. -func (pCtx *podTCtx) shimPid() uint32 { +func (pCtx *podTCtx) shimPid(version int) uint32 { t := pCtx.t cfg := criRuntimeInfo(t, pCtx.rSvc) @@ -546,7 +558,7 @@ func (pCtx *podTCtx) shimPid() uint32 { ctx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() - shimCli := connectToShim(ctx, t, cfg["containerdEndpoint"].(string), 3, pCtx.id) + shimCli := connectToShim(ctx, t, cfg["containerdEndpoint"].(string), version, pCtx.id) return shimPid(ctx, t, shimCli) } @@ -652,6 +664,25 @@ version = 2 require.NoError(t, err, "failed to create config for previous release") } +// previousReleaseCtrdConfig generates containerd config with previous release +// shim binary. +func oneSevenCtrdConfig(t *testing.T, previousReleaseBinDir, targetDir string) { + // TODO(fuweid): + // + // We should choose correct config version based on previous release. + // Currently, we're focusing on v1.x -> v2.0 so we use version = 2 here. + rawCfg := fmt.Sprintf(` +version = 2 + +[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc] + runtime_type = "%s/bin/containerd-shim-runc-v2" +`, previousReleaseBinDir) + + fileName := filepath.Join(targetDir, "config.toml") + err := os.WriteFile(fileName, []byte(rawCfg), 0600) + require.NoError(t, err, "failed to create config for previous release") +} + // criRuntimeService returns cri.RuntimeService based on the grpc address. func (p *ctrdProc) criRuntimeService(t *testing.T) cri.RuntimeService { service, err := remote.NewRuntimeService(p.grpcAddress(), 1*time.Minute) diff --git a/integration/release_upgrade_utils_linux_test.go b/integration/release_upgrade_utils_linux_test.go index 430298fd55..530e32b076 100644 --- a/integration/release_upgrade_utils_linux_test.go +++ b/integration/release_upgrade_utils_linux_test.go @@ -30,13 +30,12 @@ import ( "golang.org/x/mod/semver" "github.com/containerd/containerd/v2/pkg/archive" - "github.com/containerd/containerd/v2/version" ) // downloadPreviousLatestReleaseBinary downloads the latest version of previous // release into the target dir. -func downloadPreviousLatestReleaseBinary(t *testing.T, targetDir string) { - ver := previousReleaseVersion(t) +func downloadPreviousLatestReleaseBinary(t *testing.T, version, targetDir string) { + ver := previousReleaseVersion(t, version) downloadReleaseBinary(t, targetDir, ver) } @@ -62,10 +61,8 @@ func downloadReleaseBinary(t *testing.T, targetDir string, ver string) { } // previousReleaseVersion returns the latest version of previous release. -func previousReleaseVersion(t *testing.T) string { - majorMinor := ctrdPreviousMajorMinor(t) - - tags := gitLsRemoteCtrdTags(t, fmt.Sprintf("refs/tags/%s.*", majorMinor)) +func previousReleaseVersion(t *testing.T, version string) string { + tags := gitLsRemoteCtrdTags(t, fmt.Sprintf("refs/tags/v%s.*", version)) require.True(t, len(tags) >= 1) // sort them and get the latest version @@ -73,26 +70,6 @@ func previousReleaseVersion(t *testing.T) string { return tags[len(tags)-1] } -// ctrdPreviousMajorMinor gets the current version of running containerd. -// -// TODO(fuweid): We should parse containerd --version to get the result. -func ctrdPreviousMajorMinor(t *testing.T) string { - currentVer := "v" + version.Version - - version := semver.MajorMinor(currentVer) - switch version { - case "v2.1": - return "v2.0" - case "v2.0": - return "v1.7" - case "v1.7": - return "v1.6" - default: - t.Fatalf("unexpected containerd version: %s", currentVer) - panic("unreachable") - } -} - // gitLsRemoteTags lists containerd tags based on pattern. func gitLsRemoteCtrdTags(t *testing.T, pattern string) (_tags []string) { cmd := exec.Command("git", "ls-remote", "--tags", "--exit-code", From bca39a6f4656c4f2e3a13def7accaafcf9c0c59a Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Mon, 31 Mar 2025 17:32:05 -0700 Subject: [PATCH 3/3] Add documentation for test for issue 10467 Signed-off-by: Derek McGowan --- integration/issue10467_linux_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/integration/issue10467_linux_test.go b/integration/issue10467_linux_test.go index 28b77d7eec..fd4fe886d8 100644 --- a/integration/issue10467_linux_test.go +++ b/integration/issue10467_linux_test.go @@ -28,6 +28,9 @@ import ( "go.etcd.io/bbolt" ) +// TestIssue10467 tests the migration of sandboxes into the proper bucket. Prior to v1.7.21, the +// sandboxes were stored incorrectly in the root bucket. In order to verify the migration, a v1.7.20 +// must run and create a sandbox, then check the migration after upgrading to a newer version. func TestIssue10467(t *testing.T) { latestVersion := "v1.7.20"