Skip to content

Setup pod network after creating the sandbox container #5904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dmcgowan merged 3 commits into from
Sep 30, 2022
Merged

Setup pod network after creating the sandbox container #5904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a66f70
cri-integration: pass ENABLE_CRI_SANDBOXES to test
samuelkarp Sep 23, 2022
4f4aad0
Persist container and sandbox if resource cleanup fails, like teardow…
qiutongs Sep 16, 2022
4196fad
Add integration tests with failpoint
qiutongs Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions integration/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,15 @@ func WithPodHostname(hostname string) PodSandboxOpts {
}
}

// Add pod labels.
func WithPodLabels(kvs map[string]string) PodSandboxOpts {
return func(p *runtime.PodSandboxConfig) {
for k, v := range kvs {
p.Labels[k] = v
}
}
}

// PodSandboxConfig generates a pod sandbox config for test.
func PodSandboxConfig(name, ns string, opts ...PodSandboxOpts) *runtime.PodSandboxConfig {
config := &runtime.PodSandboxConfig{
Expand All @@ -176,6 +185,7 @@ func PodSandboxConfig(name, ns string, opts ...PodSandboxOpts) *runtime.PodSandb
},
Linux: &runtime.LinuxPodSandboxConfig{},
Annotations: make(map[string]string),
Labels: make(map[string]string),
}
for _, opt := range opts {
opt(config)
Expand Down
144 changes: 142 additions & 2 deletions integration/sandbox_run_rollback_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ import (
"strings"
"testing"

"github.com/stretchr/testify/require"
criapiv1 "k8s.io/cri-api/pkg/apis/runtime/v1"

"github.com/containerd/containerd/pkg/failpoint"
"github.com/stretchr/testify/require"
)

const (
Expand Down Expand Up @@ -89,6 +89,146 @@ func TestRunPodSandboxWithShimStartFailure(t *testing.T) {
require.Equal(t, true, strings.Contains(err.Error(), "no hard feelings"))
}

// TestRunPodSandboxWithShimDeleteFailure should keep the sandbox record if
// failed to rollback shim by shim.Delete API.
func TestRunPodSandboxWithShimDeleteFailure(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip()
}
if os.Getenv("ENABLE_CRI_SANDBOXES") != "" {
t.Skip()
}

testCase := func(restart bool) func(*testing.T) {
return func(t *testing.T) {
t.Log("Init PodSandboxConfig with specific label")
labels := map[string]string{
t.Name(): "true",
}
sbConfig := PodSandboxConfig(t.Name(), "failpoint", WithPodLabels(labels))

t.Log("Inject Shim failpoint")
injectShimFailpoint(t, sbConfig, map[string]string{
"Start": "1*error(failed to start shim)",
"Delete": "1*error(please retry)", // inject failpoint during rollback shim
})

t.Log("Create a sandbox")
_, err := runtimeService.RunPodSandbox(sbConfig, failpointRuntimeHandler)
require.Error(t, err)
require.ErrorContains(t, err, "failed to start shim")

t.Log("ListPodSandbox with the specific label")
l, err := runtimeService.ListPodSandbox(&criapiv1.PodSandboxFilter{LabelSelector: labels})
require.NoError(t, err)
require.Len(t, l, 1)

sb := l[0]
require.Equal(t, sb.State, criapiv1.PodSandboxState_SANDBOX_NOTREADY)
require.Equal(t, sb.Metadata.Name, sbConfig.Metadata.Name)
require.Equal(t, sb.Metadata.Namespace, sbConfig.Metadata.Namespace)
require.Equal(t, sb.Metadata.Uid, sbConfig.Metadata.Uid)
require.Equal(t, sb.Metadata.Attempt, sbConfig.Metadata.Attempt)

t.Log("Check PodSandboxStatus")
sbStatus, err := runtimeService.PodSandboxStatus(sb.Id)
require.NoError(t, err)
require.Equal(t, sbStatus.State, criapiv1.PodSandboxState_SANDBOX_NOTREADY)
require.Greater(t, len(sbStatus.Network.Ip), 0)

if restart {
t.Log("Restart containerd")
RestartContainerd(t)

t.Log("ListPodSandbox with the specific label")
l, err = runtimeService.ListPodSandbox(&criapiv1.PodSandboxFilter{Id: sb.Id})
require.NoError(t, err)
require.Len(t, l, 1)
require.Equal(t, l[0].State, criapiv1.PodSandboxState_SANDBOX_NOTREADY)

t.Log("Check PodSandboxStatus")
sbStatus, err := runtimeService.PodSandboxStatus(sb.Id)
require.NoError(t, err)
t.Log(sbStatus.Network)
require.Equal(t, sbStatus.State, criapiv1.PodSandboxState_SANDBOX_NOTREADY)
}

t.Log("Cleanup leaky sandbox")
err = runtimeService.RemovePodSandbox(sb.Id)
require.NoError(t, err)
}
}

t.Run("CleanupAfterRestart", testCase(true))
t.Run("JustCleanup", testCase(false))
}

// TestRunPodSandboxWithShimStartAndTeardownCNIFailure should keep the sandbox
// record if failed to rollback CNI API.
func TestRunPodSandboxWithShimStartAndTeardownCNIFailure(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip()
}
if os.Getenv("ENABLE_CRI_SANDBOXES") != "" {
t.Skip()
}

testCase := func(restart bool) func(*testing.T) {
return func(t *testing.T) {
t.Log("Init PodSandboxConfig with specific key")
labels := map[string]string{
t.Name(): "true",
}
sbConfig := PodSandboxConfig(t.Name(), "failpoint", WithPodLabels(labels))

t.Log("Inject Shim failpoint")
injectShimFailpoint(t, sbConfig, map[string]string{
"Start": "1*error(failed to start shim)",
})

t.Log("Inject CNI failpoint")
conf := &failpointConf{
Del: "1*error(please retry)",
}
injectCNIFailpoint(t, sbConfig, conf)

t.Log("Create a sandbox")
_, err := runtimeService.RunPodSandbox(sbConfig, failpointRuntimeHandler)
require.Error(t, err)
require.ErrorContains(t, err, "failed to start shim")

t.Log("ListPodSandbox with the specific label")
l, err := runtimeService.ListPodSandbox(&criapiv1.PodSandboxFilter{LabelSelector: labels})
require.NoError(t, err)
require.Len(t, l, 1)

sb := l[0]
require.Equal(t, sb.State, criapiv1.PodSandboxState_SANDBOX_NOTREADY)
require.Equal(t, sb.Metadata.Name, sbConfig.Metadata.Name)
require.Equal(t, sb.Metadata.Namespace, sbConfig.Metadata.Namespace)
require.Equal(t, sb.Metadata.Uid, sbConfig.Metadata.Uid)
require.Equal(t, sb.Metadata.Attempt, sbConfig.Metadata.Attempt)

if restart {
t.Log("Restart containerd")
RestartContainerd(t)

t.Log("ListPodSandbox with the specific label")
l, err = runtimeService.ListPodSandbox(&criapiv1.PodSandboxFilter{Id: sb.Id})
require.NoError(t, err)
require.Len(t, l, 1)
require.Equal(t, l[0].State, criapiv1.PodSandboxState_SANDBOX_NOTREADY)
}

t.Log("Cleanup leaky sandbox")
err = runtimeService.RemovePodSandbox(sb.Id)
require.NoError(t, err)
}
}
t.Run("CleanupAfterRestart", testCase(true))
t.Run("JustCleanup", testCase(false))
}

// failpointConf is used to describe cmdAdd/cmdDel/cmdCheck command's failpoint.
type failpointConf struct {
Add string `json:"cmdAdd"`
Expand All @@ -101,7 +241,7 @@ func injectCNIFailpoint(t *testing.T, sbConfig *criapiv1.PodSandboxConfig, conf

metadata := sbConfig.Metadata
fpFilename := filepath.Join(stateDir,
fmt.Sprintf("%s-%s.json", metadata.Namespace, metadata.Name))
fmt.Sprintf("%s-%s.json", metadata.Namespace, strings.Replace(metadata.Name, "/", "-", -1)))

data, err := json.Marshal(conf)
require.NoError(t, err)
Expand Down
20 changes: 20 additions & 0 deletions pkg/cri/server/container_update_resources_other.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import (
"context"
"fmt"

"github.com/containerd/containerd"
"github.com/containerd/containerd/containers"
"github.com/containerd/typeurl"
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"

containerstore "github.com/containerd/containerd/pkg/cri/store/container"
Expand All @@ -44,3 +48,19 @@ func (c *criService) UpdateContainerResources(ctx context.Context, r *runtime.Up
}
return &runtime.UpdateContainerResourcesResponse{}, nil
}

// TODO: Copied from container_update_resources.go because that file is not built for darwin.
Copy link
Copy Markdown
Member

@samuelkarp samuelkarp Sep 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(non-blocker) Do you need this on darwin since you're running unit tests there? If so, we could remove the !darwin from container_update_resources.go instead of copying it here.

(This is really out of scope for your change, but container_update_resource.go should probably be platform-nonspecific and delegate out to anything that needs platform-specific changes.)

Copy link
Copy Markdown
Contributor Author

@qiutongs qiutongs Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

container_update_resources.go shouldn't be platform-dependent so I assume !darwin has a good reason?

I can try remove, if anything breaks, I will have to go this way.

// updateContainerSpec updates container spec.
func updateContainerSpec(ctx context.Context, cntr containerd.Container, spec *runtimespec.Spec) error {
any, err := typeurl.MarshalAny(spec)
if err != nil {
return fmt.Errorf("failed to marshal spec %+v: %w", spec, err)
}
if err := cntr.Update(ctx, func(ctx context.Context, client *containerd.Client, c *containers.Container) error {
c.Spec = any
return nil
}); err != nil {
return fmt.Errorf("failed to update container spec: %w", err)
}
return nil
}
Loading