Skip to content

Commit

Permalink
Revert "cri: make read-only mounts recursively read-only"
Browse files Browse the repository at this point in the history 
Revert PR 9713, as it appeared to break the compatibility too much
kubernetes/enhancements#3858 (comment)

This reverts commit b2f254f.

> Conflicts:
>	internal/cri/opts/spec_linux_opts.go

Signed-off-by: Akihiro Suda <[email protected]>
  • Loading branch information
@AkihiroSuda
AkihiroSuda committed Feb 7, 2024
1 parent 805ed8e commit a395618
Show file tree
Hide file tree
Showing 11 changed files with 8 additions and 387 deletions.
19 changes: 0 additions & 19 deletions RELEASES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,25 +461,6 @@ version = 2

</p></details>

## Other breaking changes
### containerd v2.0
#### CRI plugin treats read-only mounts recursively read-only
Starting with containerd v2.0, the CRI plugin treats read-only mounts
as recursively read-only mounts when running on Linux kernel v5.12 or later.

To rollback to the legacy behavior that corresponds to containerd v1.x,
set the following config:
```toml
version = 2
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
# treat_ro_mounts_as_rro ("Enabled"|"IfPossible"|"Disabled")
# treats read-only mounts as recursive read-only mounts.
# An empty string means "IfPossible".
# "Enabled" requires Linux kernel v5.12 or later.
# This configuration does not apply to non-volume mounts such as "/sys/fs/cgroup".
treat_ro_mounts_as_rro = "Disabled"
```

## Experimental features

Experimental features are new features added to containerd which do not have the
Expand Down
8 changes: 0 additions & 8 deletions docs/cri/config.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,14 +369,6 @@ version = 2
# See https://github.com/containerd/containerd/issues/6657 for context.
snapshotter = ""

# treat_ro_mounts_as_rro ("Enabled"|"IfPossible"|"Disabled")
# treats read-only mounts as recursive read-only mounts.
# An empty string means "IfPossible".
# "Enabled" requires Linux kernel v5.12 or later.
# Introduced in containerd v2.0.
# This configuration does not apply to non-volume mounts such as "/sys/fs/cgroup".
treat_ro_mounts_as_rro = ""

# 'plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options' is options specific to
# "io.containerd.runc.v1" and "io.containerd.runc.v2". Its corresponding options type is:
# https://github.com/containerd/containerd/blob/v1.3.2/runtime/v2/runc/options/oci.pb.go#L26 .
Expand Down
149 changes: 0 additions & 149 deletions integration/container_volume_linux_test.go
View file

This file was deleted.

157 changes: 2 additions & 155 deletions internal/cri/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,9 @@ import (
"errors"
"fmt"
"net/url"
goruntime "runtime"
"strconv"
"time"

introspectionapi "github.com/containerd/containerd/v2/api/services/introspection/v1"
apitypes "github.com/containerd/containerd/v2/api/types"
"github.com/containerd/containerd/v2/protobuf"
"github.com/containerd/log"
"github.com/containerd/typeurl/v2"
"github.com/pelletier/go-toml/v2"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
"k8s.io/kubelet/pkg/cri/streaming"
Expand All @@ -40,16 +34,8 @@ import (
"github.com/containerd/containerd/v2/pkg/deprecation"
runtimeoptions "github.com/containerd/containerd/v2/pkg/runtimeoptions/v1"
"github.com/containerd/containerd/v2/plugins"
"github.com/opencontainers/image-spec/specs-go"
"github.com/opencontainers/runtime-spec/specs-go/features"
)

func init() {
const prefix = "types.containerd.io"
major := strconv.Itoa(specs.VersionMajor)
typeurl.Register(&features.Features{}, prefix, "opencontainers/runtime-spec", major, "features", "Features")
}

const (
// defaultImagePullProgressTimeoutDuration is the default value of imagePullProgressTimeout.
//
Expand Down Expand Up @@ -87,17 +73,6 @@ const (
DefaultSandboxImage = "registry.k8s.io/pause:3.9"
)

// Ternary represents a ternary value.
// Ternary is needed because TOML does not accept "null" for boolean values.
type Ternary = string

const (
TernaryEmpty Ternary = "" // alias for IfPossible
TernaryEnabled Ternary = "Enabled"
TernaryIfPossible Ternary = "IfPossible"
TernaryDisabled Ternary = "Disabled"
)

// Runtime struct to contain the type(ID), engine, and root variables for a default runtime
// and a runtime for untrusted workload.
type Runtime struct {
Expand Down Expand Up @@ -141,15 +116,6 @@ type Runtime struct {
// shim - means use whatever Controller implementation provided by shim (e.g. use RemoteController).
// podsandbox - means use Controller implementation from sbserver podsandbox package.
Sandboxer string `toml:"sandboxer" json:"sandboxer"`

// TreatRoMountsAsRro ("Enabled"|"IfPossible"|"Disabled")
// treats read-only mounts as recursive read-only mounts.
// An empty string means "IfPossible".
// "Enabled" requires Linux kernel v5.12 or later.
// Introduced in containerd v2.0.
// This configuration does not apply to non-volume mounts such as "/sys/fs/cgroup".
TreatRoMountsAsRro Ternary `toml:"treat_ro_mount_as_rro" json:"treatRoMountsAsRro"`
TreatRoMountsAsRroResolved bool `toml:"-" json:"-"` // Do not set manually
}

// ContainerdConfig contains toml config related to containerd
Expand Down Expand Up @@ -533,120 +499,8 @@ func ValidateImageConfig(ctx context.Context, c *ImageConfig) ([]deprecation.War
return warnings, nil
}

func introspectRuntimeFeatures(ctx context.Context, introspectionClient introspectionapi.IntrospectionClient, r Runtime) (*features.Features, error) {
if introspectionClient == nil { // happens for unit tests
return nil, errors.New("introspectionClient is nil")
}
infoReq := &introspectionapi.PluginInfoRequest{
Type: string(plugins.RuntimePluginV2),
ID: "task",
}
rr := &apitypes.RuntimeRequest{
RuntimePath: r.Type,
}
if r.Path != "" {
rr.RuntimePath = r.Path
}
options, err := GenerateRuntimeOptions(r)
if err != nil {
return nil, err
}
rr.Options, err = protobuf.MarshalAnyToProto(options)
if err != nil {
return nil, fmt.Errorf("failed to marshal %T: %w", options, err)
}
infoReq.Options, err = protobuf.MarshalAnyToProto(rr)
if err != nil {
return nil, fmt.Errorf("failed to marshal %T: %w", rr, err)
}
infoResp, err := introspectionClient.PluginInfo(ctx, infoReq)
if err != nil {
return nil, fmt.Errorf("failed to call PluginInfo: %w", err)
}
var info apitypes.RuntimeInfo
if err := typeurl.UnmarshalTo(infoResp.Extra, &info); err != nil {
return nil, fmt.Errorf("failed to get runtime info from plugin info: %w", err)
}
featuresX, err := typeurl.UnmarshalAny(info.Features)
if err != nil {
return nil, fmt.Errorf("failed to unmarshal Features (%T): %w", info.Features, err)
}
features, ok := featuresX.(*features.Features)
if !ok {
return nil, fmt.Errorf("unknown features type %T", featuresX)
}
return features, nil
}

// resolveTreatRoMountsAsRro resolves r.TreatRoMountsAsRro string into a boolean.
func resolveTreatRoMountsAsRro(ctx context.Context, introspectionClient introspectionapi.IntrospectionClient, r Runtime) (bool, error) {
debugPrefix := "treat_ro_mounts_as_rro"
if r.Type != "" {
debugPrefix += fmt.Sprintf("[%s]", r.Type)
}
if binaryName := r.Options["BinaryName"]; binaryName != "" {
debugPrefix += fmt.Sprintf("[%v]", binaryName)
}
debugPrefix += ": "

var runtimeSupportsRro bool
if r.Type == plugins.RuntimeRuncV2 {
features, err := introspectRuntimeFeatures(ctx, introspectionClient, r)
if err != nil {
log.G(ctx).WithError(err).Warnf(debugPrefix + "failed to introspect runtime features (binary is not compatible with runc v1.1?)")
} else {
log.G(ctx).Debugf(debugPrefix+"Features: %+v", features)
for _, s := range features.MountOptions {
if s == "rro" {
runtimeSupportsRro = true
break
}
}
}
}

switch r.TreatRoMountsAsRro {
case TernaryDisabled:
log.G(ctx).Debug(debugPrefix + "rro mounts are explicitly disabled")
return false, nil
case TernaryEnabled:
log.G(ctx).Debug(debugPrefix + "rro mounts are explicitly enabled")
if !kernelSupportsRro {
return true, fmt.Errorf("invalid `treat_ro_mounts_as_rro`: %q: needs Linux kernel v5.12 or later", TernaryEnabled)
}
if !runtimeSupportsRro {
return true, fmt.Errorf("invalid `treat_ro_mounts_as_rro`: %q: needs a runtime that is compatible with runc v1.1", TernaryEnabled)
}
return true, nil
case TernaryEmpty, TernaryIfPossible:
if r.Type != plugins.RuntimeRuncV2 {
log.G(ctx).Debugf(debugPrefix+"rro mounts are not supported by runtime %q, disabling rro mounts", r.Type)
return false, nil
}
if !kernelSupportsRro {
msg := debugPrefix + "rro mounts are not supported by kernel, disabling rro mounts"
if goruntime.GOOS == "linux" {
msg += " (Hint: upgrade the kernel to v5.12 or later)"
log.G(ctx).Warn(msg)
} else {
log.G(ctx).Debug(msg)
}
return false, nil
}
if !runtimeSupportsRro {
log.G(ctx).Warn(debugPrefix + "rro mounts are not supported by runtime, disabling rro mounts (Hint: use a runtime that is compatible with runc v1.1)")
return false, nil
}
log.G(ctx).Debug(debugPrefix + "rro mounts are implicitly enabled")
return true, nil
default:
return false, fmt.Errorf("invalid `treat_ro_mounts_as_rro`: %q (must be %q, %q, or %q)",
r.TreatRoMountsAsRro, TernaryDisabled, TernaryEnabled, TernaryIfPossible)
}
}

// ValidateRuntimeConfig validates the given runtime configuration.
func ValidateRuntimeConfig(ctx context.Context, c *RuntimeConfig, introspectionClient introspectionapi.IntrospectionClient) ([]deprecation.Warning, error) {
func ValidateRuntimeConfig(ctx context.Context, c *RuntimeConfig) ([]deprecation.Warning, error) {
var warnings []deprecation.Warning
if c.ContainerdConfig.Runtimes == nil {
c.ContainerdConfig.Runtimes = make(map[string]Runtime)
Expand All @@ -667,15 +521,8 @@ func ValidateRuntimeConfig(ctx context.Context, c *RuntimeConfig, introspectionC
// If empty, use default podSandbox mode
if len(r.Sandboxer) == 0 {
r.Sandboxer = string(ModePodSandbox)
c.ContainerdConfig.Runtimes[k] = r
}

// Resolve r.TreatRoMountsAsRro (string; empty value must not be ignored) into r.TreatRoMountsAsRroResolved (bool)
var err error
r.TreatRoMountsAsRroResolved, err = resolveTreatRoMountsAsRro(ctx, introspectionClient, r)
if err != nil {
return warnings, err
}
c.ContainerdConfig.Runtimes[k] = r
}

// Validation for drain_exec_sync_io_timeout
Expand Down
Loading

0 comments on commit a395618

Please sign in to comment.