Skip to content

Commit

Permalink
Merge pull request containerd#9210 from thaJeztah/1.6_backport_fix_de…
Browse files Browse the repository at this point in the history
…adlock
  • Loading branch information
fuweid authored Oct 12, 2023
2 parents 4908fef + 7b61862 commit 74b207c
Show file tree
Hide file tree
Showing 15 changed files with 353 additions and 49 deletions.
5 changes: 5 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,11 @@ bin/cni-bridge-fp: integration/failpoint/cmd/cni-bridge-fp FORCE
@echo "$(WHALE) $@"
@$(GO) build ${GO_BUILD_FLAGS} -o $@ ./integration/failpoint/cmd/cni-bridge-fp

# build runc-fp as runc wrapper to support failpoint, only used by integration test
bin/runc-fp: integration/failpoint/cmd/runc-fp FORCE
@echo "$(WHALE) $@"
@$(GO) build ${GO_BUILD_FLAGS} -o $@ ./integration/failpoint/cmd/runc-fp

benchmark: ## run benchmarks tests
@echo "$(WHALE) $@"
@$(GO) test ${TESTFLAGS} -bench . -run Benchmark -test.root
Expand Down
13 changes: 13 additions & 0 deletions Vagrantfile
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,19 @@ EOF
SHELL
end

config.vm.provision "install-failpoint-binaries", type: "shell", run: "once" do |sh|
sh.upload_path = "/tmp/vagrant-install-failpoint-binaries"
sh.inline = <<~SHELL
#!/usr/bin/env bash
source /etc/environment
source /etc/profile.d/sh.local
set -eux -o pipefail
${GOPATH}/src/github.com/containerd/containerd/script/setup/install-failpoint-binaries
chcon -v -t container_runtime_exec_t $(type -ap containerd-shim-runc-fp-v1)
containerd-shim-runc-fp-v1 -v
SHELL
end

# SELinux is Enforcing by default.
# To set SELinux as Disabled on a VM that has already been provisioned:
# SELINUX=Disabled vagrant up --provision-with=selinux
Expand Down
18 changes: 11 additions & 7 deletions cmd/containerd/command/oci-hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ import (
"syscall"
"text/template"

specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/containerd/containerd/oci"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/urfave/cli"
)

Expand All @@ -37,7 +38,8 @@ var ociHook = cli.Command{
if err != nil {
return err
}
spec, err := loadSpec(state.Bundle)
specFile := filepath.Join(state.Bundle, oci.ConfigFilename)
spec, err := loadSpec(specFile)
if err != nil {
return err
}
Expand All @@ -56,14 +58,16 @@ var ociHook = cli.Command{
},
}

// hookSpec is a shallow version of [oci.Spec] containing only the
// fields we need for the hook. We use a shallow struct to reduce
// the overhead of unmarshaling.
type hookSpec struct {
Root struct {
Path string `json:"path"`
} `json:"root"`
// Root configures the container's root filesystem.
Root *specs.Root `json:"root,omitempty"`
}

func loadSpec(bundle string) (*hookSpec, error) {
f, err := os.Open(filepath.Join(bundle, "config.json"))
func loadSpec(path string) (*hookSpec, error) {
f, err := os.Open(path)
if err != nil {
return nil, err
}
Expand Down
82 changes: 82 additions & 0 deletions integration/client/container_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ import (
"github.com/containerd/containerd/runtime/linux/runctypes"
"github.com/containerd/containerd/runtime/v2/runc/options"
"github.com/containerd/containerd/sys"

"github.com/opencontainers/runtime-spec/specs-go"
"github.com/stretchr/testify/require"
exec "golang.org/x/sys/execabs"
"golang.org/x/sys/unix"
)
Expand Down Expand Up @@ -1494,3 +1496,83 @@ func TestShimOOMScore(t *testing.T) {
case <-statusC:
}
}

// TestIssue9103 is used as regression case for issue 9103.
//
// The runc-fp will kill the init process so that the shim should return stopped
// status after container.NewTask. It's used to simulate that the runc-init
// might be killed by oom-kill.
func TestIssue9103(t *testing.T) {
if os.Getenv("RUNC_FLAVOR") == "crun" {
t.Skip("skip it when using crun")
}
if getRuntimeVersion() == "v1" {
t.Skip("skip it when using shim v1")
}

client, err := newClient(t, address)
require.NoError(t, err)
defer client.Close()

var (
image Image
ctx, cancel = testContext(t)
id = t.Name()
)
defer cancel()

image, err = client.GetImage(ctx, testImage)
require.NoError(t, err)

for idx, tc := range []struct {
desc string
cntrOpts []NewContainerOpts
expectedStatus ProcessStatus
}{
{
desc: "should be created status",
cntrOpts: []NewContainerOpts{
WithNewSpec(oci.WithImageConfig(image),
withProcessArgs("sleep", "30"),
),
},
expectedStatus: Created,
},
{
desc: "should be stopped status if init has been killed",
cntrOpts: []NewContainerOpts{
WithNewSpec(oci.WithImageConfig(image),
withProcessArgs("sleep", "30"),
oci.WithAnnotations(map[string]string{
"oci.runc.failpoint.profile": "issue9103",
}),
),
WithRuntime(client.Runtime(), &options.Options{
BinaryName: "runc-fp",
}),
},
expectedStatus: Stopped,
},
} {
tc := tc
tName := fmt.Sprintf("%s%d", id, idx)
t.Run(tc.desc, func(t *testing.T) {
container, err := client.NewContainer(ctx, tName,
append([]NewContainerOpts{WithNewSnapshot(tName, image)}, tc.cntrOpts...)...,
)
require.NoError(t, err)
defer container.Delete(ctx, WithSnapshotCleanup)

cctx, ccancel := context.WithTimeout(ctx, 30*time.Second)
task, err := container.NewTask(cctx, empty())
ccancel()
require.NoError(t, err)

defer task.Delete(ctx, WithProcessKill)

status, err := task.Status(ctx)
require.NoError(t, err)
require.Equal(t, status.Status, tc.expectedStatus)
})
}
}
4 changes: 4 additions & 0 deletions integration/client/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ require (
github.com/opencontainers/image-spec v1.1.0-rc2.0.20221005185240-3a7f492d3f1b
github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417
github.com/sirupsen/logrus v1.9.3
github.com/stretchr/testify v1.8.4
golang.org/x/sys v0.7.0
gotest.tools/v3 v3.5.0
)
Expand All @@ -28,6 +29,7 @@ require (
github.com/containerd/fifo v1.0.0 // indirect
github.com/containerd/log v0.1.0 // indirect
github.com/coreos/go-systemd/v22 v22.3.2 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect
github.com/docker/go-units v0.4.0 // indirect
github.com/godbus/dbus/v5 v5.0.6 // indirect
Expand All @@ -45,13 +47,15 @@ require (
github.com/opencontainers/selinux v1.10.1 // indirect
github.com/pelletier/go-toml v1.9.5 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
go.opencensus.io v0.23.0 // indirect
golang.org/x/net v0.8.0 // indirect
golang.org/x/sync v0.1.0 // indirect
golang.org/x/text v0.8.0 // indirect
google.golang.org/genproto v0.0.0-20220502173005-c8bf987b8c21 // indirect
google.golang.org/grpc v1.50.1 // indirect
google.golang.org/protobuf v1.28.1 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace (
Expand Down
1 change: 1 addition & 0 deletions integration/client/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,7 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=
gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys=
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package main

import (
"context"
"encoding/json"
"fmt"
"os"
"path/filepath"
Expand All @@ -35,8 +34,6 @@ import (
)

const (
ociConfigFilename = "config.json"

failpointPrefixKey = "io.containerd.runtime.v2.shim.failpoint."
)

Expand Down Expand Up @@ -113,15 +110,10 @@ func newFailpointFromOCIAnnotation() (map[string]*failpoint.Failpoint, error) {
return nil, fmt.Errorf("failed to get current working dir: %w", err)
}

configPath := filepath.Join(cwd, ociConfigFilename)
data, err := os.ReadFile(configPath)
configPath := filepath.Join(cwd, oci.ConfigFilename)
spec, err := oci.ReadSpec(configPath)
if err != nil {
return nil, fmt.Errorf("failed to read %v: %w", configPath, err)
}

var spec oci.Spec
if err := json.Unmarshal(data, &spec); err != nil {
return nil, fmt.Errorf("failed to parse oci.Spec(%v): %w", string(data), err)
return nil, err
}

res := make(map[string]*failpoint.Failpoint)
Expand Down
69 changes: 69 additions & 0 deletions integration/failpoint/cmd/runc-fp/issue9103.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//go:build linux

/*
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 main

import (
"context"
"fmt"
"os"
"strconv"
"strings"
"syscall"
"time"
)

// issue9103KillInitAfterCreate kills the runc.Init process after creating
// command returns successfully.
//
// REF: https://github.com/containerd/containerd/issues/9103
func issue9103KillInitAfterCreate(ctx context.Context, method invoker) error {
isCreated := strings.Contains(strings.Join(os.Args, ","), ",create,")

if err := method(ctx); err != nil {
return err
}

if !isCreated {
return nil
}

initPidPath := "init.pid"
data, err := os.ReadFile(initPidPath)
if err != nil {
return fmt.Errorf("failed to read %s: %w", initPidPath, err)
}

pid, err := strconv.Atoi(string(data))
if err != nil {
return fmt.Errorf("failed to get init pid from string %s: %w", string(data), err)
}

if pid <= 0 {
return fmt.Errorf("unexpected init pid %v", pid)
}

if err := syscall.Kill(pid, syscall.SIGKILL); err != nil {
return fmt.Errorf("failed to kill the init pid %v: %w", pid, err)
}

// Ensure that the containerd-shim has received the SIGCHLD and start
// to cleanup
time.Sleep(3 * time.Second)
return nil
}
Loading

0 comments on commit 74b207c

Please sign in to comment.