diff --git a/integration/addition_gids_test.go b/integration/addition_gids_test.go index 832c26ab637b2..3edd62279458a 100644 --- a/integration/addition_gids_test.go +++ b/integration/addition_gids_test.go @@ -114,7 +114,7 @@ func TestAdditionalGids(t *testing.T) { if err != nil { return false, err } - if s.GetState() == runtime.ContainerState_CONTAINER_EXITED { + if s.Status.GetState() == runtime.ContainerState_CONTAINER_EXITED { return true, nil } return false, nil diff --git a/integration/container_cgroup_writable_linux_test.go b/integration/container_cgroup_writable_linux_test.go index 2aee05054147a..3572ec4fc7380 100644 --- a/integration/container_cgroup_writable_linux_test.go +++ b/integration/container_cgroup_writable_linux_test.go @@ -123,7 +123,7 @@ func TestContainerCgroupWritable(t *testing.T) { status, err := runtimeService.ContainerStatus(cn) require.NoError(t, err) - assert.Equal(t, status.GetState(), runtime.ContainerState_CONTAINER_RUNNING) + assert.Equal(t, status.Status.GetState(), runtime.ContainerState_CONTAINER_RUNNING) // Execute a command to verify if cgroup is writable _, stderr, err := runtimeService.ExecSync(cn, []string{"mkdir", "sys/fs/cgroup/dummy-group"}, 2) diff --git a/integration/container_log_test.go b/integration/container_log_test.go index 983b4b380e6c7..08749e096a5e9 100644 --- a/integration/container_log_test.go +++ b/integration/container_log_test.go @@ -65,7 +65,7 @@ func TestContainerLogWithoutTailingNewLine(t *testing.T) { if err != nil { return false, err } - if s.GetState() == runtime.ContainerState_CONTAINER_EXITED { + if s.Status.GetState() == runtime.ContainerState_CONTAINER_EXITED { return true, nil } return false, nil @@ -120,7 +120,7 @@ func TestLongContainerLog(t *testing.T) { if err != nil { return false, err } - if s.GetState() == runtime.ContainerState_CONTAINER_EXITED { + if s.Status.GetState() == runtime.ContainerState_CONTAINER_EXITED { return true, nil } return false, nil diff --git a/integration/container_stop_test.go b/integration/container_stop_test.go index 616a940d6a754..8ef8060d19072 100644 --- a/integration/container_stop_test.go +++ b/integration/container_stop_test.go @@ -67,7 +67,7 @@ func TestSharedPidMultiProcessContainerStop(t *testing.T) { t.Log("The container state should be exited") s, err := runtimeService.ContainerStatus(cn) require.NoError(t, err) - assert.Equal(t, runtime.ContainerState_CONTAINER_EXITED, s.GetState()) + assert.Equal(t, runtime.ContainerState_CONTAINER_EXITED, s.Status.GetState()) }) } } @@ -117,7 +117,7 @@ func TestContainerStopCancellation(t *testing.T) { if err != nil { return false, err } - return s.GetState() == runtime.ContainerState_CONTAINER_RUNNING, nil + return s.Status.GetState() == runtime.ContainerState_CONTAINER_RUNNING, nil }, 100*time.Millisecond, 5*time.Second)) t.Log("Stop the container with 1s timeout, without shorter context timeout") @@ -126,5 +126,5 @@ func TestContainerStopCancellation(t *testing.T) { t.Log("The container state should be exited") s, err := runtimeService.ContainerStatus(cn) require.NoError(t, err) - assert.Equal(t, runtime.ContainerState_CONTAINER_EXITED, s.GetState()) + assert.Equal(t, runtime.ContainerState_CONTAINER_EXITED, s.Status.GetState()) } diff --git a/integration/container_update_resources_test.go b/integration/container_update_resources_test.go index a489c48ff645a..3c4d2741b8ea5 100644 --- a/integration/container_update_resources_test.go +++ b/integration/container_update_resources_test.go @@ -325,7 +325,7 @@ func TestUpdateContainerResources_StatusUpdated(t *testing.T) { t.Log("Check memory limit in container status") status, err := runtimeService.ContainerStatus(cn) - checkMemoryLimitInContainerStatus(t, status, 200*1024*1024) + checkMemoryLimitInContainerStatus(t, status.Status, 200*1024*1024) require.NoError(t, err) t.Log("Update container memory limit after created") @@ -336,7 +336,7 @@ func TestUpdateContainerResources_StatusUpdated(t *testing.T) { t.Log("Check memory limit in container status") status, err = runtimeService.ContainerStatus(cn) - checkMemoryLimitInContainerStatus(t, status, 400*1024*1024) + checkMemoryLimitInContainerStatus(t, status.Status, 400*1024*1024) require.NoError(t, err) t.Log("Start the container") @@ -350,6 +350,6 @@ func TestUpdateContainerResources_StatusUpdated(t *testing.T) { t.Log("Check memory limit in container status") status, err = runtimeService.ContainerStatus(cn) - checkMemoryLimitInContainerStatus(t, status, 800*1024*1024) + checkMemoryLimitInContainerStatus(t, status.Status, 800*1024*1024) require.NoError(t, err) } diff --git a/integration/container_volume_test.go b/integration/container_volume_test.go index 07a6e27c903ab..4d5be98563e6d 100644 --- a/integration/container_volume_test.go +++ b/integration/container_volume_test.go @@ -131,7 +131,7 @@ func TestContainerSymlinkVolumes(t *testing.T) { if err != nil { return false, err } - if s.GetState() == runtime.ContainerState_CONTAINER_EXITED { + if s.Status.GetState() == runtime.ContainerState_CONTAINER_EXITED { return true, nil } return false, nil diff --git a/integration/container_without_image_ref_test.go b/integration/container_without_image_ref_test.go index 1e613d27e8865..34e025ecda013 100644 --- a/integration/container_without_image_ref_test.go +++ b/integration/container_without_image_ref_test.go @@ -53,7 +53,7 @@ func TestContainerLifecycleWithoutImageRef(t *testing.T) { t.Log("Container status should be running") status, err := runtimeService.ContainerStatus(cn) require.NoError(t, err) - assert.Equal(t, runtime.ContainerState_CONTAINER_RUNNING, status.GetState()) + assert.Equal(t, runtime.ContainerState_CONTAINER_RUNNING, status.Status.GetState()) t.Logf("Stop container") err = runtimeService.StopContainer(cn, 1) @@ -62,5 +62,5 @@ func TestContainerLifecycleWithoutImageRef(t *testing.T) { t.Log("Container status should be exited") status, err = runtimeService.ContainerStatus(cn) require.NoError(t, err) - assert.Equal(t, runtime.ContainerState_CONTAINER_EXITED, status.GetState()) + assert.Equal(t, runtime.ContainerState_CONTAINER_EXITED, status.Status.GetState()) } diff --git a/integration/containerd_image_test.go b/integration/containerd_image_test.go index af16e602d60ea..21e9aebe9e13a 100644 --- a/integration/containerd_image_test.go +++ b/integration/containerd_image_test.go @@ -155,10 +155,10 @@ func TestContainerdImage(t *testing.T) { if err != nil { return false, err } - if s.Resources == nil || (s.Resources.Linux == nil && s.Resources.Windows == nil) { + if s.Status.Resources == nil || (s.Status.Resources.Linux == nil && s.Status.Resources.Windows == nil) { return false, fmt.Errorf("No Resource field in container status: %+v", s) } - return s.GetState() == runtime.ContainerState_CONTAINER_RUNNING, nil + return s.Status.GetState() == runtime.ContainerState_CONTAINER_RUNNING, nil } require.NoError(t, Eventually(checkContainer, 100*time.Millisecond, 10*time.Second)) require.NoError(t, Consistently(checkContainer, 100*time.Millisecond, time.Second)) diff --git a/integration/cri-api/pkg/apis/services.go b/integration/cri-api/pkg/apis/services.go index a60ffc5276205..afe0b7fdb0b1d 100644 --- a/integration/cri-api/pkg/apis/services.go +++ b/integration/cri-api/pkg/apis/services.go @@ -62,7 +62,7 @@ type ContainerManager interface { // ListContainers lists all containers by filters. ListContainers(filter *runtimeapi.ContainerFilter, opts ...grpc.CallOption) ([]*runtimeapi.Container, error) // ContainerStatus returns the status of the container. - ContainerStatus(containerID string, opts ...grpc.CallOption) (*runtimeapi.ContainerStatus, error) + ContainerStatus(containerID string, opts ...grpc.CallOption) (*runtimeapi.ContainerStatusResponse, error) // UpdateContainerResources updates the cgroup resources for the container. UpdateContainerResources(containerID string, resources *runtimeapi.LinuxContainerResources, windowsResources *runtimeapi.WindowsContainerResources, opts ...grpc.CallOption) error // ExecSync executes a command in the container, and returns the stdout output. diff --git a/integration/duplicate_name_linux_test.go b/integration/duplicate_name_linux_test.go new file mode 100644 index 0000000000000..d15f12f4adae7 --- /dev/null +++ b/integration/duplicate_name_linux_test.go @@ -0,0 +1,112 @@ +/* + 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 ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strconv" + "syscall" + "testing" + + "github.com/containerd/containerd/v2/integration/images" + "github.com/containerd/containerd/v2/internal/cri/store/container" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1" +) + +type versionedStatus struct { + // Version indicates the version of the versioned container status. + Version string + container.Status +} + +// https://github.com/containerd/containerd/issues/7247 +func TestDuplicateNameIssue7247(t *testing.T) { + t.Logf("Create a sandbox") + sb, sbConfig := PodSandboxConfigWithCleanup(t, "sandbox", "duplicate-name7247") + + t.Logf("Create the sandbox again should fail") + _, err := runtimeService.RunPodSandbox(sbConfig, *runtimeHandler) + require.Error(t, err) + + pauseImage := images.Get(images.Pause) + EnsureImageExists(t, pauseImage) + + t.Logf("Create a container") + + var ( + testImage = images.Get(images.BusyBox) + containerName = "test-container" + ) + + EnsureImageExists(t, testImage) + + cnConfig := ContainerConfig( + containerName, + testImage, + WithCommand("sh", "-c", "sleep 1000"), + ) + + cID, err := runtimeService.CreateContainer(sb, cnConfig, sbConfig) + require.NoError(t, err) + err = runtimeService.StartContainer(cID) + require.NoError(t, err) + + status, err := runtimeService.ContainerStatus(cID) + require.NoError(t, err) + assert.Equal(t, status.Status.GetState(), runtime.ContainerState_CONTAINER_RUNNING) + + cfg := criRuntimeInfo(t, runtimeService) + rootDir := cfg["rootDir"].(string) + containerDir := filepath.Join(rootDir, "containers", cID) + statusDir := filepath.Join(containerDir, "status") + + versioned := &versionedStatus{} + data, err := os.ReadFile(statusDir) + require.NoError(t, err) + err = json.Unmarshal(data, versioned) + require.NoError(t, err) + pid := strconv.Itoa(int(versioned.Pid)) + + t.Logf("containerid is %s", cID) + t.Logf("pid is %s", pid) + _, err = exec.Command("chattr", "+i", statusDir).CombinedOutput() + require.NoError(t, err) + StopContainerd(t, syscall.SIGTERM) + status.GetInfo() + t.Logf("status is %v", status) + + //kill pid make sure container status changed + info := status.GetInfo() + t.Logf("info is %v", info) + _, err = exec.Command("kill", "-9", pid).CombinedOutput() + require.NoError(t, err) + // if can't update status info status file, containerd will exited. + StartContainerd(t) + //let status file can be writed + _, err = exec.Command("chattr", "-i", statusDir).CombinedOutput() + require.NoError(t, err) + StartContainerd(t) + + t.Logf("Create the container again should fail") + _, err = runtimeService.CreateContainer(sb, cnConfig, sbConfig) + require.Error(t, err) +} diff --git a/integration/image_load_test.go b/integration/image_load_test.go index 31e6d279db9f4..8e9ef807bc1e0 100644 --- a/integration/image_load_test.go +++ b/integration/image_load_test.go @@ -88,5 +88,5 @@ func TestImageLoad(t *testing.T) { t.Logf("make sure container is running") status, err := runtimeService.ContainerStatus(cn) require.NoError(t, err) - require.Equal(t, runtime.ContainerState_CONTAINER_RUNNING, status.State) + require.Equal(t, runtime.ContainerState_CONTAINER_RUNNING, status.Status.State) } diff --git a/integration/image_mount_test.go b/integration/image_mount_test.go index e1cc2a439eada..c10f993e8fa18 100644 --- a/integration/image_mount_test.go +++ b/integration/image_mount_test.go @@ -102,7 +102,7 @@ func testImageMountSELinux(t *testing.T, testImage, testMountImage, mountPath st if err != nil { return false, err } - if s.GetState() == criruntime.ContainerState_CONTAINER_EXITED { + if s.Status.GetState() == criruntime.ContainerState_CONTAINER_EXITED { return true, nil } return false, nil @@ -149,7 +149,7 @@ func testImageMount(t *testing.T, testImage, testMountImage, mountPath string, c if err != nil { return false, err } - if s.GetState() == criruntime.ContainerState_CONTAINER_EXITED { + if s.Status.GetState() == criruntime.ContainerState_CONTAINER_EXITED { return true, nil } return false, nil diff --git a/integration/main_test.go b/integration/main_test.go index adfac9c25f378..f0a36e7d9d6e5 100644 --- a/integration/main_test.go +++ b/integration/main_test.go @@ -803,6 +803,32 @@ func RestartContainerd(t *testing.T, signal syscall.Signal) { }, time.Second, 30*time.Second), "wait for containerd to be restarted") } +func StopContainerd(t *testing.T, signal syscall.Signal) { + require.NoError(t, KillProcess(*containerdBin, signal)) + + // Use assert so that the 3rd wait always runs, this makes sure + // containerd is running before this function returns. + assert.NoError(t, Eventually(func() (bool, error) { + pid, err := PidOf(*containerdBin) + if err != nil { + return false, err + } + return pid == 0, nil + }, time.Second, 30*time.Second), "wait for containerd to be killed") +} + +func StartContainerd(t *testing.T) { + require.NoError(t, Eventually(func() (bool, error) { + return ConnectDaemons() == nil, nil + }, time.Second, 30*time.Second), "wait for containerd to be restarted") +} + +func StartContainerdFailed(t *testing.T) { + require.Error(t, Eventually(func() (bool, error) { + return ConnectDaemons() == nil, nil + }, time.Second, 30*time.Second), "wait for containerd to be restarted") +} + // EnsureImageExists pulls the given image, ensures that no error was encountered // while pulling it. func EnsureImageExists(t *testing.T, imageName string) string { diff --git a/integration/pod_dualstack_test.go b/integration/pod_dualstack_test.go index b30a84611bb67..7f492ad1fb05c 100644 --- a/integration/pod_dualstack_test.go +++ b/integration/pod_dualstack_test.go @@ -69,7 +69,7 @@ func TestPodDualStack(t *testing.T) { if err != nil { return false, err } - if s.GetState() == runtime.ContainerState_CONTAINER_EXITED { + if s.Status.GetState() == runtime.ContainerState_CONTAINER_EXITED { return true, nil } return false, nil diff --git a/integration/pod_hostname_test.go b/integration/pod_hostname_test.go index 661362b026d01..5026236dd08bd 100644 --- a/integration/pod_hostname_test.go +++ b/integration/pod_hostname_test.go @@ -113,7 +113,7 @@ func TestPodHostname(t *testing.T) { if err != nil { return false, err } - if s.GetState() == runtime.ContainerState_CONTAINER_EXITED { + if s.Status.GetState() == runtime.ContainerState_CONTAINER_EXITED { return true, nil } return false, nil diff --git a/integration/pod_userns_linux_test.go b/integration/pod_userns_linux_test.go index 3615051ff5bd5..b702f05a386b1 100644 --- a/integration/pod_userns_linux_test.go +++ b/integration/pod_userns_linux_test.go @@ -289,7 +289,7 @@ func TestPodUserNS(t *testing.T) { if err != nil { return false, err } - if s.GetState() == runtime.ContainerState_CONTAINER_EXITED { + if s.Status.GetState() == runtime.ContainerState_CONTAINER_EXITED { return true, nil } return false, nil @@ -410,7 +410,7 @@ func TestIssue10598(t *testing.T) { return false, err } - if state := s.GetState(); state != runtime.ContainerState_CONTAINER_RUNNING { + if state := s.Status.GetState(); state != runtime.ContainerState_CONTAINER_RUNNING { return false, fmt.Errorf("%s is not running\nstate: %s\nlog: %s", containerName, state, string(content)) } diff --git a/integration/release_upgrade_linux_test.go b/integration/release_upgrade_linux_test.go index 7938e6a2f07a1..e07f8ba860898 100644 --- a/integration/release_upgrade_linux_test.go +++ b/integration/release_upgrade_linux_test.go @@ -533,7 +533,7 @@ func (pCtx *podTCtx) containerDataDir(cntrID string) string { cfg := criRuntimeInfo(t, pCtx.rSvc) rootDir := cfg["rootDir"].(string) - return filepath.Join(rootDir, "containers", status.Id) + return filepath.Join(rootDir, "containers", status.Status.Id) } // shimPid returns shim's pid. @@ -589,7 +589,7 @@ func checkContainerState(t *testing.T, svc cri.RuntimeService, name string, expe t.Logf("Checking container %s state", name) status, err := svc.ContainerStatus(name) require.NoError(t, err) - assert.Equal(t, expected, status.State) + assert.Equal(t, expected, status.Status.State) } // pullImagesByCRI pulls images by CRI. diff --git a/integration/remote/remote_runtime.go b/integration/remote/remote_runtime.go index 956ca54880f5f..a23dedc09e223 100644 --- a/integration/remote/remote_runtime.go +++ b/integration/remote/remote_runtime.go @@ -334,7 +334,7 @@ func (r *RuntimeService) ListContainers(filter *runtimeapi.ContainerFilter, opts } // ContainerStatus returns the container status. -func (r *RuntimeService) ContainerStatus(containerID string, opts ...grpc.CallOption) (*runtimeapi.ContainerStatus, error) { +func (r *RuntimeService) ContainerStatus(containerID string, opts ...grpc.CallOption) (*runtimeapi.ContainerStatusResponse, error) { klog.V(10).Infof("[RuntimeService] ContainerStatus (containerID=%v, timeout=%v)", containerID, r.timeout) ctx, cancel := getContextWithTimeout(r.timeout) defer cancel() @@ -359,7 +359,7 @@ func (r *RuntimeService) ContainerStatus(containerID string, opts ...grpc.CallOp } } - return resp.Status, nil + return resp, nil } // UpdateContainerResources updates a containers resource config diff --git a/integration/truncindex_test.go b/integration/truncindex_test.go index a3264471cd3f2..ebe86310b51ab 100644 --- a/integration/truncindex_test.go +++ b/integration/truncindex_test.go @@ -95,7 +95,7 @@ func TestTruncIndex(t *testing.T) { t.Logf("Get container status by truncindex") cStatus, err := runtimeService.ContainerStatus(cnTruncIndex) require.NoError(t, err) - assert.Equal(t, cn, cStatus.Id) + assert.Equal(t, cn, cStatus.Status.Id) t.Logf("Start the container") require.NoError(t, runtimeService.StartContainer(cnTruncIndex)) diff --git a/integration/windows_device_test.go b/integration/windows_device_test.go index 7825432869a2c..2827c0732f475 100644 --- a/integration/windows_device_test.go +++ b/integration/windows_device_test.go @@ -75,7 +75,7 @@ func TestWindowsDevice(t *testing.T) { if err != nil { return false, err } - if s.GetState() == runtime.ContainerState_CONTAINER_EXITED { + if s.Status.GetState() == runtime.ContainerState_CONTAINER_EXITED { return true, nil } return false, nil diff --git a/integration/windows_rootfs_size_test.go b/integration/windows_rootfs_size_test.go index dd50659904ac6..d6a828d20502e 100644 --- a/integration/windows_rootfs_size_test.go +++ b/integration/windows_rootfs_size_test.go @@ -74,7 +74,7 @@ func TestWindowsRootfsSize(t *testing.T) { if err != nil { return false, err } - if s.GetState() == runtime.ContainerState_CONTAINER_EXITED { + if s.Status.GetState() == runtime.ContainerState_CONTAINER_EXITED { return true, nil } return false, nil diff --git a/internal/cri/server/restart.go b/internal/cri/server/restart.go index 54aae010b6dfa..548d6fc1be3ec 100644 --- a/internal/cri/server/restart.go +++ b/internal/cri/server/restart.go @@ -174,7 +174,10 @@ func (c *criService) recover(ctx context.Context) error { WithError(err). WithField("container", container.ID()). Error("Failed to load container") - + if _, ok := err.(containerstore.StoreStatusError); ok { + return err + } + // should not happen return nil } log.G(ctx2).Debugf("Loaded container %+v", cntr) @@ -248,7 +251,7 @@ func (c *criService) recover(ctx context.Context) error { const loadContainerTimeout = 10 * time.Second // loadContainer loads container from containerd and status checkpoint. -func (c *criService) loadContainer(ctx context.Context, cntr containerd.Container) (containerstore.Container, error) { +func (c *criService) loadContainer(ctx context.Context, cntr containerd.Container) (cstore containerstore.Container, retErr error) { ctx, cancel := context.WithTimeout(ctx, loadContainerTimeout) defer cancel() id := cntr.ID() @@ -413,6 +416,11 @@ func (c *criService) loadContainer(ctx context.Context, cntr containerd.Containe if containerIO != nil { opts = append(opts, containerstore.WithContainerIO(containerIO)) } + defer func() { + if retErr != nil && containerIO != nil { + containerIO.Close() + } + }() return containerstore.NewContainer(*meta, opts...) } diff --git a/internal/cri/store/container/container.go b/internal/cri/store/container/container.go index 6054a157629af..961254ce18c6e 100644 --- a/internal/cri/store/container/container.go +++ b/internal/cri/store/container/container.go @@ -17,6 +17,7 @@ package container import ( + "fmt" "sync" containerd "github.com/containerd/containerd/v2/client" @@ -70,12 +71,20 @@ func WithContainerIO(io *cio.ContainerIO) Opts { } } +type StoreStatusError struct { + msg string +} + +func (e StoreStatusError) Error() string { + return e.msg +} + // WithStatus adds status to the container. func WithStatus(status Status, root string) Opts { return func(c *Container) error { s, err := StoreStatus(root, c.ID, status) if err != nil { - return err + return StoreStatusError{msg: fmt.Sprintf("store status:%v", err)} } c.Status = s if s.Get().State() == runtime.ContainerState_CONTAINER_EXITED {