diff --git a/cmd/containerd/command/oci-hook.go b/cmd/containerd/command/oci-hook.go index 2592b3d5d..361ecacfc 100644 --- a/cmd/containerd/command/oci-hook.go +++ b/cmd/containerd/command/oci-hook.go @@ -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" ) @@ -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 } @@ -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 } diff --git a/integration/failpoint/cmd/containerd-shim-runc-fp-v1/plugin_linux.go b/integration/failpoint/cmd/containerd-shim-runc-fp-v1/plugin_linux.go index 1e70c3c2a..ff5fa6d59 100644 --- a/integration/failpoint/cmd/containerd-shim-runc-fp-v1/plugin_linux.go +++ b/integration/failpoint/cmd/containerd-shim-runc-fp-v1/plugin_linux.go @@ -18,7 +18,6 @@ package main import ( "context" - "encoding/json" "fmt" "os" "path/filepath" @@ -35,8 +34,6 @@ import ( ) const ( - ociConfigFilename = "config.json" - failpointPrefixKey = "io.containerd.runtime.v2.shim.failpoint." ) @@ -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) diff --git a/oci/spec.go b/oci/spec.go index 23961b75d..a28abaf91 100644 --- a/oci/spec.go +++ b/oci/spec.go @@ -18,6 +18,8 @@ package oci import ( "context" + "encoding/json" + "os" "path/filepath" "runtime" @@ -46,6 +48,22 @@ var ( // to be created without the "issues" with go vendoring and package imports type Spec = specs.Spec +const ConfigFilename = "config.json" + +// ReadSpec deserializes JSON into an OCI runtime Spec from a given path. +func ReadSpec(path string) (*Spec, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + var s Spec + if err := json.NewDecoder(f).Decode(&s); err != nil { + return nil, err + } + return &s, nil +} + // GenerateSpec will generate a default spec from the provided image // for use as a containerd container func GenerateSpec(ctx context.Context, client Client, c *containers.Container, opts ...SpecOpts) (*Spec, error) { diff --git a/runtime/v2/bundle.go b/runtime/v2/bundle.go index 0e3c73d70..e7b5800c8 100644 --- a/runtime/v2/bundle.go +++ b/runtime/v2/bundle.go @@ -26,12 +26,11 @@ import ( "github.com/containerd/containerd/identifiers" "github.com/containerd/containerd/mount" "github.com/containerd/containerd/namespaces" + "github.com/containerd/containerd/oci" "github.com/containerd/typeurl/v2" "github.com/opencontainers/runtime-spec/specs-go" ) -const configFilename = "config.json" - // LoadBundle loads an existing bundle from disk func LoadBundle(ctx context.Context, root, id string) (*Bundle, error) { ns, err := namespaces.NamespaceRequired(ctx) @@ -107,9 +106,10 @@ func NewBundle(ctx context.Context, root, state, id string, spec typeurl.Any) (b } if spec := spec.GetValue(); spec != nil { // write the spec to the bundle - err = os.WriteFile(filepath.Join(b.Path, configFilename), spec, 0666) + specPath := filepath.Join(b.Path, oci.ConfigFilename) + err = os.WriteFile(specPath, spec, 0666) if err != nil { - return nil, fmt.Errorf("failed to write %s", configFilename) + return nil, fmt.Errorf("failed to write bundle spec: %w", err) } } return b, nil diff --git a/runtime/v2/runc/manager/manager_linux.go b/runtime/v2/runc/manager/manager_linux.go index ea4d4aa0f..8d20d89e1 100644 --- a/runtime/v2/runc/manager/manager_linux.go +++ b/runtime/v2/runc/manager/manager_linux.go @@ -32,6 +32,7 @@ import ( "github.com/containerd/containerd/log" "github.com/containerd/containerd/mount" "github.com/containerd/containerd/namespaces" + "github.com/containerd/containerd/oci" "github.com/containerd/containerd/pkg/process" "github.com/containerd/containerd/pkg/schedcore" "github.com/containerd/containerd/runtime/v2/runc" @@ -58,7 +59,11 @@ var groupLabels = []string{ "io.kubernetes.cri.sandbox-id", } +// spec 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 spec struct { + // Annotations contains arbitrary metadata for the container. Annotations map[string]string `json:"annotations,omitempty"` } @@ -97,7 +102,7 @@ func newCommand(ctx context.Context, id, containerdAddress, containerdTTRPCAddre } func readSpec() (*spec, error) { - f, err := os.Open("config.json") + f, err := os.Open(oci.ConfigFilename) if err != nil { return nil, err } diff --git a/runtime/v2/runc/util.go b/runtime/v2/runc/util.go index 10c13617a..611d93155 100644 --- a/runtime/v2/runc/util.go +++ b/runtime/v2/runc/util.go @@ -20,29 +20,24 @@ package runc import ( "context" - "encoding/json" - "os" "path/filepath" "github.com/containerd/containerd/log" - specs "github.com/opencontainers/runtime-spec/specs-go" + "github.com/containerd/containerd/oci" + "github.com/opencontainers/runtime-spec/specs-go" ) // ShouldKillAllOnExit reads the bundle's OCI spec and returns true if // there is an error reading the spec or if the container has a private PID namespace func ShouldKillAllOnExit(ctx context.Context, bundlePath string) bool { - var bundleSpec specs.Spec - bundleConfigContents, err := os.ReadFile(filepath.Join(bundlePath, "config.json")) + spec, err := oci.ReadSpec(filepath.Join(bundlePath, oci.ConfigFilename)) if err != nil { log.G(ctx).WithError(err).Error("shouldKillAllOnExit: failed to read config.json") return true } - if err := json.Unmarshal(bundleConfigContents, &bundleSpec); err != nil { - log.G(ctx).WithError(err).Error("shouldKillAllOnExit: failed to unmarshal bundle json") - return true - } - if bundleSpec.Linux != nil { - for _, ns := range bundleSpec.Linux.Namespaces { + + if spec.Linux != nil { + for _, ns := range spec.Linux.Namespaces { if ns.Type == specs.PIDNamespace && ns.Path == "" { return false }