From f42ab2ff7fa0daf356b9fab7dd0ea0657cc65576 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Sun, 31 Mar 2024 18:23:27 +0900 Subject: [PATCH 1/2] Implement `nerdctl run --annotation` (introduced in Docker v24) An OCI runtime (as well as `nerdctl internal oci-hook`) may consume an annotation and behave differently. e.g., https://github.com/opencontainers/runc/blob/v1.1.12/docs/systemd.md#auxiliary-properties nerdctl v1: - `nerdctl run --annotation` was not implemented. - `nerdctl run --label` is set as a containerd label and an OCI annotation. nerdctl v2: - `nerdctl run --annotation` is only set as an OCI annotation. - `nerdctl run --label` is only set as a containerd label. A label with the `nerdctl/` prefix can no longer be set manually, with an exception for `nerdctl/bypass4netns`. The `nerdctl/bypass4netns` label is still allowed and is propagated to an OCI annotation, for sake of compatibility. Docker v23: - `docker run --annotation` was not implemented. - `docker run --label` is only set as a Docker label. Docker v24 (implemented in docker/cli PR 4156, moby/moby PR 45025): - `docker run --annotation` is only set as an OCI annotation. - `docker run --label` is only set as a Docker label. (In a nutshell, `--annotation` may change the behavior, while `--label` should not.) Signed-off-by: Akihiro Suda --- README.md | 2 +- cmd/nerdctl/compose_up_linux_test.go | 4 +-- cmd/nerdctl/container_create.go | 4 +++ cmd/nerdctl/container_run.go | 7 ++-- docs/command-reference.md | 3 +- docs/rootless.md | 8 +++-- .../rootless/containerd-rootless-setuptool.sh | 2 +- pkg/annotations/annotations.go | 34 +++++++++++++++++++ pkg/api/types/container_types.go | 3 ++ pkg/bypass4netnsutil/bypass4netnsutil.go | 10 +++--- pkg/cmd/container/create.go | 22 ++++++++++-- pkg/cmd/container/run_linux.go | 7 ++-- pkg/composer/serviceparser/serviceparser.go | 9 +++++ pkg/labels/labels.go | 13 +------ 14 files changed, 94 insertions(+), 34 deletions(-) create mode 100644 pkg/annotations/annotations.go diff --git a/README.md b/README.md index 4b5a02aba72..36d9bbe1897 100644 --- a/README.md +++ b/README.md @@ -189,7 +189,7 @@ Major: - [P2P image distribution using IPFS](./docs/ipfs.md): `nerdctl run ipfs://CID` . P2P image distribution (IPFS) is completely optional. Your host is NOT connected to any P2P network, unless you opt in to [install and run IPFS daemon](https://docs.ipfs.io/install/). - [Cosign integration](./docs/cosign.md): `nerdctl pull --verify=cosign` and `nerdctl push --sign=cosign`, and [in Compose](./docs/cosign.md#cosign-in-compose) -- [Accelerated rootless containers using bypass4netns](./docs/rootless.md): `nerdctl run --label nerdctl/bypass4netns=true` +- [Accelerated rootless containers using bypass4netns](./docs/rootless.md): `nerdctl run --annotation nerdctl/bypass4netns=true` Minor: diff --git a/cmd/nerdctl/compose_up_linux_test.go b/cmd/nerdctl/compose_up_linux_test.go index ddbea597ef5..95dc761c54b 100644 --- a/cmd/nerdctl/compose_up_linux_test.go +++ b/cmd/nerdctl/compose_up_linux_test.go @@ -523,7 +523,7 @@ services: WORDPRESS_DB_NAME: exampledb volumes: - wordpress:/var/www/html - labels: + annotations: - nerdctl/bypass4netns=1 db: @@ -536,7 +536,7 @@ services: MYSQL_RANDOM_ROOT_PASSWORD: '1' volumes: - db:/var/lib/mysql - labels: + annotations: - nerdctl/bypass4netns=1 volumes: diff --git a/cmd/nerdctl/container_create.go b/cmd/nerdctl/container_create.go index de99a782671..9d94f068d9e 100644 --- a/cmd/nerdctl/container_create.go +++ b/cmd/nerdctl/container_create.go @@ -337,6 +337,10 @@ func processContainerCreateOptions(cmd *cobra.Command) (opt types.ContainerCreat if err != nil { return } + opt.Annotations, err = cmd.Flags().GetStringArray("annotation") + if err != nil { + return + } opt.CidFile, err = cmd.Flags().GetString("cidfile") if err != nil { return diff --git a/cmd/nerdctl/container_run.go b/cmd/nerdctl/container_run.go index 077332d447c..036b1f57da6 100644 --- a/cmd/nerdctl/container_run.go +++ b/cmd/nerdctl/container_run.go @@ -23,6 +23,7 @@ import ( "github.com/containerd/console" "github.com/containerd/log" + "github.com/containerd/nerdctl/v2/pkg/annotations" "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/clientutil" "github.com/containerd/nerdctl/v2/pkg/cmd/container" @@ -230,8 +231,10 @@ func setCreateFlags(cmd *cobra.Command) { cmd.Flags().String("name", "", "Assign a name to the container") // label needs to be StringArray, not StringSlice, to prevent "foo=foo1,foo2" from being split to {"foo=foo1", "foo2"} cmd.Flags().StringArrayP("label", "l", nil, "Set metadata on container") - cmd.RegisterFlagCompletionFunc("label", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - return labels.ShellCompletions, cobra.ShellCompDirectiveNoFileComp + // annotation needs to be StringArray, not StringSlice, to prevent "foo=foo1,foo2" from being split to {"foo=foo1", "foo2"} + cmd.Flags().StringArray("annotation", nil, "Add an annotation to the container (passed through to the OCI runtime)") + cmd.RegisterFlagCompletionFunc("annotation", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return annotations.ShellCompletions, cobra.ShellCompDirectiveNoFileComp }) // label-file is defined as StringSlice, not StringArray, to allow specifying "--env-file=FILE1,FILE2" (compatible with Podman) diff --git a/docs/command-reference.md b/docs/command-reference.md index dcdde45ec83..3c3371c787c 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -298,8 +298,9 @@ Env flags: Metadata flags: - :whale: :blue_square: `--name`: Assign a name to the container -- :whale: :blue_square: `-l, --label`: Set meta data on a container +- :whale: :blue_square: `-l, --label`: Set meta data on a container (Not passed through the OCI runtime since nerdctl v2.0, with an exception for `nerdctl/bypass4netns`) - :whale: :blue_square: `--label-file`: Read in a line delimited file of labels +- :whale: :blue_square: `--annotation`: Add an annotation to the container (passed through to the OCI runtime) - :whale: :blue_square: `--cidfile`: Write the container ID to the file - :nerd_face: `--pidfile`: file path to write the task's pid. The CLI syntax conforms to Podman convention. diff --git a/docs/rootless.md b/docs/rootless.md index c8c738df06c..5b9333ebabc 100644 --- a/docs/rootless.md +++ b/docs/rootless.md @@ -121,11 +121,15 @@ The performance benchmark with iperf3 on Ubuntu 21.10 on Hyper-V VM is shown bel This benchmark can be reproduced with [https://github.com/rootless-containers/bypass4netns/blob/f009d96139e9e38ce69a2ea8a9a746349bad273c/Vagrantfile](https://github.com/rootless-containers/bypass4netns/blob/f009d96139e9e38ce69a2ea8a9a746349bad273c/Vagrantfile) -Acceleration with bypass4netns is available with `--label nerdctl/bypass4netns=true`. You also need to have `bypass4netnsd` (bypass4netns daemon) to be running. +Acceleration with bypass4netns is available with: +- `--annotation nerdctl/bypass4netns=true` (for nerdctl v2.0 and later) +- `--label nerdctl/bypass4netns=true` (deprecated form, used in nerdctl prior to v2.0). + +You also need to have `bypass4netnsd` (bypass4netns daemon) to be running. Example ```console $ containerd-rootless-setuptool.sh install-bypass4netnsd -$ nerdctl run -it --rm -p 8080:80 --label nerdctl/bypass4netns=true alpine +$ nerdctl run -it --rm -p 8080:80 --annotation nerdctl/bypass4netns=true alpine ``` More detail is available at [https://github.com/rootless-containers/bypass4netns/blob/master/README.md](https://github.com/rootless-containers/bypass4netns/blob/master/README.md) diff --git a/extras/rootless/containerd-rootless-setuptool.sh b/extras/rootless/containerd-rootless-setuptool.sh index 9b31d5efe2c..9b005c7cce0 100755 --- a/extras/rootless/containerd-rootless-setuptool.sh +++ b/extras/rootless/containerd-rootless-setuptool.sh @@ -365,7 +365,7 @@ cmd_entrypoint_install_bypass4netnsd() { [Install] WantedBy=default.target EOT - INFO "To use bypass4netnsd, set the \"nerdctl/bypass4netns=true\" label on containers, e.g., \`nerdctl run --label nerdctl/bypass4netns=true\`" + INFO "To use bypass4netnsd, set the \"nerdctl/bypass4netns=true\" annotation on containers, e.g., \`nerdctl run --annotation nerdctl/bypass4netns=true\`" } # CLI subcommand: "install-fuse-overlayfs" diff --git a/pkg/annotations/annotations.go b/pkg/annotations/annotations.go new file mode 100644 index 00000000000..751d231541f --- /dev/null +++ b/pkg/annotations/annotations.go @@ -0,0 +1,34 @@ +/* + 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 annotations defines OCI annotations +package annotations + +const ( + // Prefix is the common prefix of nerdctl annotations + Prefix = "nerdctl/" + + // Bypass4netns is the flag for acceleration with bypass4netns + // Boolean value which can be parsed with strconv.ParseBool() is required. + // (like "nerdctl/bypass4netns=true" or "nerdctl/bypass4netns=false") + Bypass4netns = Prefix + "bypass4netns" +) + +var ShellCompletions = []string{ + Bypass4netns + "=true", + Bypass4netns + "=false", + // Other annotations should not be set via CLI +} diff --git a/pkg/api/types/container_types.go b/pkg/api/types/container_types.go index f4c4f6f1b16..60e5da4c373 100644 --- a/pkg/api/types/container_types.go +++ b/pkg/api/types/container_types.go @@ -217,9 +217,12 @@ type ContainerCreateOptions struct { // Name assign a name to the container Name string // Label set meta data on a container + // (not passed through to the OCI runtime since nerdctl v2.0, with an exception for "nerdctl/bypass4netns") Label []string // LabelFile read in a line delimited file of labels LabelFile []string + // Annotations set meta data on a container (passed through to the OCI runtime) + Annotations []string // CidFile write the container ID to the file CidFile string // PidFile specifies the file path to write the task's pid. The CLI syntax conforms to Podman convention. diff --git a/pkg/bypass4netnsutil/bypass4netnsutil.go b/pkg/bypass4netnsutil/bypass4netnsutil.go index 133102ae98f..2dd0093a35a 100644 --- a/pkg/bypass4netnsutil/bypass4netnsutil.go +++ b/pkg/bypass4netnsutil/bypass4netnsutil.go @@ -25,7 +25,7 @@ import ( "github.com/containerd/containerd/containers" "github.com/containerd/containerd/oci" - "github.com/containerd/nerdctl/v2/pkg/labels" + "github.com/containerd/nerdctl/v2/pkg/annotations" "github.com/opencontainers/runtime-spec/specs-go" b4nnoci "github.com/rootless-containers/bypass4netns/pkg/oci" ) @@ -46,8 +46,8 @@ func generateSecurityOpt(listenerPath string) (oci.SpecOpts, error) { return opt, nil } -func GenerateBypass4netnsOpts(securityOptsMaps map[string]string, labelMaps map[string]string, id string) ([]oci.SpecOpts, error) { - b4nn, ok := labelMaps[labels.Bypass4netns] +func GenerateBypass4netnsOpts(securityOptsMaps map[string]string, annotationsMap map[string]string, id string) ([]oci.SpecOpts, error) { + b4nn, ok := annotationsMap[annotations.Bypass4netns] if !ok { return nil, nil } @@ -133,8 +133,8 @@ func GetPidFilePathByID(id string) (string, error) { return socketPath, nil } -func IsBypass4netnsEnabled(annotations map[string]string) (bool, error) { - if b4nn, ok := annotations[labels.Bypass4netns]; ok { +func IsBypass4netnsEnabled(annotationsMap map[string]string) (bool, error) { + if b4nn, ok := annotationsMap[annotations.Bypass4netns]; ok { b4nnEnable, err := strconv.ParseBool(b4nn) if err != nil { return false, err diff --git a/pkg/cmd/container/create.go b/pkg/cmd/container/create.go index 2045b820c44..eb680fd0a6b 100644 --- a/pkg/cmd/container/create.go +++ b/pkg/cmd/container/create.go @@ -35,6 +35,7 @@ import ( "github.com/containerd/containerd/oci" gocni "github.com/containerd/go-cni" "github.com/containerd/log" + "github.com/containerd/nerdctl/v2/pkg/annotations" "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/clientutil" "github.com/containerd/nerdctl/v2/pkg/cmd/image" @@ -276,13 +277,15 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa } } + // TODO: abolish internal labels and only use annotations ilOpt, err := withInternalLabels(internalLabels) if err != nil { return nil, nil, err } cOpts = append(cOpts, ilOpt) - opts = append(opts, propagateContainerdLabelsToOCIAnnotations()) + opts = append(opts, propagateInternalContainerdLabelsToOCIAnnotations(), + oci.WithAnnotations(strutil.ConvertKVStringsToMap(options.Annotations))) var s specs.Spec spec := containerd.WithSpec(&s, opts...) @@ -506,6 +509,13 @@ func withContainerLabels(label, labelFile []string) ([]containerd.NewContainerOp if err != nil { return nil, err } + for k := range labelMap { + if strings.HasPrefix(k, annotations.Bypass4netns) { + log.L.Warnf("Label %q is deprecated, use an annotation instead", k) + } else if strings.HasPrefix(k, labels.Prefix) { + return nil, fmt.Errorf("internal label %q must not be specified manually", k) + } + } o := containerd.WithAdditionalContainerLabels(labelMap) return []containerd.NewContainerOpts{o}, nil } @@ -704,9 +714,15 @@ func processeds(mountPoints []dockercompat.MountPoint) []*mountutil.Processed { return result } -func propagateContainerdLabelsToOCIAnnotations() oci.SpecOpts { +func propagateInternalContainerdLabelsToOCIAnnotations() oci.SpecOpts { return func(ctx context.Context, oc oci.Client, c *containers.Container, s *oci.Spec) error { - return oci.WithAnnotations(c.Labels)(ctx, oc, c, s) + allowed := make(map[string]string) + for k, v := range c.Labels { + if strings.Contains(k, labels.Prefix) { + allowed[k] = v + } + } + return oci.WithAnnotations(allowed)(ctx, oc, c, s) } } diff --git a/pkg/cmd/container/run_linux.go b/pkg/cmd/container/run_linux.go index 83773477782..89fdb73cd8d 100644 --- a/pkg/cmd/container/run_linux.go +++ b/pkg/cmd/container/run_linux.go @@ -59,10 +59,7 @@ func setPlatformOptions(ctx context.Context, client *containerd.Client, id, uts } opts = append(opts, cgOpts...) - labelsMap, err := readKVStringsMapfFromLabel(options.Label, options.LabelFile) - if err != nil { - return nil, err - } + annotations := strutil.ConvertKVStringsToMap(options.Annotations) capOpts, err := generateCapOpts( strutil.DedupeStrSlice(options.CapAdd), @@ -78,7 +75,7 @@ func setPlatformOptions(ctx context.Context, client *containerd.Client, id, uts } opts = append(opts, secOpts...) - b4nnOpts, err := bypass4netnsutil.GenerateBypass4netnsOpts(securityOptsMaps, labelsMap, id) + b4nnOpts, err := bypass4netnsutil.GenerateBypass4netnsOpts(securityOptsMaps, annotations, id) if err != nil { return nil, err } diff --git a/pkg/composer/serviceparser/serviceparser.go b/pkg/composer/serviceparser/serviceparser.go index 60dd5ee8d86..05c829d4212 100644 --- a/pkg/composer/serviceparser/serviceparser.go +++ b/pkg/composer/serviceparser/serviceparser.go @@ -54,6 +54,7 @@ const Separator = "-" func warnUnknownFields(svc types.ServiceConfig) { if unknown := reflectutil.UnknownNonEmptyFields(&svc, "Name", + "Annotations", "Build", "BlkioConfig", "CapAdd", @@ -477,6 +478,14 @@ func newContainer(project *types.Project, parsed *Service, i int) (*Container, e "--pull=never", // because image will be ensured before running replicas with `nerdctl run`. } + for k, v := range svc.Annotations { + if v == "" { + c.RunArgs = append(c.RunArgs, fmt.Sprintf("--annotation=%s", k)) + } else { + c.RunArgs = append(c.RunArgs, fmt.Sprintf("--annotation=%s=%s", k, v)) + } + } + if svc.BlkioConfig != nil && svc.BlkioConfig.Weight != 0 { c.RunArgs = append(c.RunArgs, fmt.Sprintf("--blkio-weight=%d", svc.BlkioConfig.Weight)) } diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 40e82971fd4..d88aee02a7d 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -15,7 +15,7 @@ */ // Package labels defines labels that are set to containerd containers as labels. -// The labels are also passed to OCI containers as annotations. +// The labels defined in this package are also passed to OCI containers as annotations. package labels const ( @@ -79,11 +79,6 @@ const ( // Mounts is the mount points for the container. Mounts = Prefix + "mounts" - // Bypass4netns is the flag for acceleration with bypass4netns - // Boolean value which can be parsed with strconv.ParseBool() is required. - // (like "nerdctl/bypass4netns=true" or "nerdctl/bypass4netns=false") - Bypass4netns = Prefix + "bypass4netns" - // StopTimeout is seconds to wait for stop a container. StopTimeout = Prefix + "stop-timeout" @@ -106,9 +101,3 @@ const ( // (like "nerdctl/default-network=true" or "nerdctl/default-network=false") NerdctlDefaultNetwork = Prefix + "default-network" ) - -var ShellCompletions = []string{ - Bypass4netns + "=true", - Bypass4netns + "=false", - // Other labels should not be set via CLI -} From fc4c8e788dd4b05f9ea3111ebe0e5d8515c2decb Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Sun, 31 Mar 2024 21:26:13 +0900 Subject: [PATCH 2/2] annotations: add `nerdctl/bypass4netns-ignore-subnets` (`[]string`) For experiments of additional `bypass4netns --ignore` Signed-off-by: Akihiro Suda --- pkg/annotations/annotations.go | 6 +++++- pkg/bypass4netnsutil/bypass.go | 20 +++++++++++++++----- pkg/ocihook/ocihook.go | 4 ++-- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/pkg/annotations/annotations.go b/pkg/annotations/annotations.go index 751d231541f..e89648d7abb 100644 --- a/pkg/annotations/annotations.go +++ b/pkg/annotations/annotations.go @@ -25,10 +25,14 @@ const ( // Boolean value which can be parsed with strconv.ParseBool() is required. // (like "nerdctl/bypass4netns=true" or "nerdctl/bypass4netns=false") Bypass4netns = Prefix + "bypass4netns" + + // Bypass4netnsIgnoreSubnets is a JSON of []string that is appended to + // the `bypass4netns --ignore` list. + Bypass4netnsIgnoreSubnets = Bypass4netns + "-ignore-subnets" ) var ShellCompletions = []string{ Bypass4netns + "=true", Bypass4netns + "=false", - // Other annotations should not be set via CLI + Bypass4netnsIgnoreSubnets + "=", } diff --git a/pkg/bypass4netnsutil/bypass.go b/pkg/bypass4netnsutil/bypass.go index 6dfb4db3c86..08853500558 100644 --- a/pkg/bypass4netnsutil/bypass.go +++ b/pkg/bypass4netnsutil/bypass.go @@ -18,31 +18,41 @@ package bypass4netnsutil import ( "context" + "encoding/json" "fmt" "net" "path/filepath" "github.com/containerd/containerd/errdefs" gocni "github.com/containerd/go-cni" + "github.com/containerd/nerdctl/v2/pkg/annotations" b4nnapi "github.com/rootless-containers/bypass4netns/pkg/api" "github.com/rootless-containers/bypass4netns/pkg/api/daemon/client" rlkclient "github.com/rootless-containers/rootlesskit/v2/pkg/api/client" ) -func NewBypass4netnsCNIBypassManager(client client.Client, rlkClient rlkclient.Client) (*Bypass4netnsCNIBypassManager, error) { +func NewBypass4netnsCNIBypassManager(client client.Client, rlkClient rlkclient.Client, annotationsMap map[string]string) (*Bypass4netnsCNIBypassManager, error) { if client == nil || rlkClient == nil { return nil, errdefs.ErrInvalidArgument } + var ignoreSubnets []string + if v := annotationsMap[annotations.Bypass4netnsIgnoreSubnets]; v != "" { + if err := json.Unmarshal([]byte(v), &ignoreSubnets); err != nil { + return nil, fmt.Errorf("failed to unmarshal annotation %q: %q: %w", annotations.Bypass4netnsIgnoreSubnets, v, err) + } + } pm := &Bypass4netnsCNIBypassManager{ - Client: client, - rlkClient: rlkClient, + Client: client, + rlkClient: rlkClient, + ignoreSubnets: ignoreSubnets, } return pm, nil } type Bypass4netnsCNIBypassManager struct { client.Client - rlkClient rlkclient.Client + rlkClient rlkclient.Client + ignoreSubnets []string } func (b4nnm *Bypass4netnsCNIBypassManager) StartBypass(ctx context.Context, ports []gocni.PortMapping, id, stateDir string) error { @@ -73,7 +83,7 @@ func (b4nnm *Bypass4netnsCNIBypassManager) StartBypass(ctx context.Context, port PidFilePath: pidFilePath, LogFilePath: logFilePath, // "auto" can detect CNI CIDRs automatically - IgnoreSubnets: []string{"127.0.0.0/8", rlkCIDR, "auto"}, + IgnoreSubnets: append([]string{"127.0.0.0/8", rlkCIDR, "auto"}, b4nnm.ignoreSubnets...), } portMap := []b4nnapi.PortSpec{} for _, p := range ports { diff --git a/pkg/ocihook/ocihook.go b/pkg/ocihook/ocihook.go index 243d9a9af9a..7ce72386910 100644 --- a/pkg/ocihook/ocihook.go +++ b/pkg/ocihook/ocihook.go @@ -449,7 +449,7 @@ func applyNetworkSettings(opts *handlerOpts) error { if rootlessutil.IsRootlessChild() { if b4nnEnabled { - bm, err := bypass4netnsutil.NewBypass4netnsCNIBypassManager(opts.bypassClient, opts.rootlessKitClient) + bm, err := bypass4netnsutil.NewBypass4netnsCNIBypassManager(opts.bypassClient, opts.rootlessKitClient, opts.state.Annotations) if err != nil { return err } @@ -493,7 +493,7 @@ func onPostStop(opts *handlerOpts) error { } if rootlessutil.IsRootlessChild() { if b4nnEnabled { - bm, err := bypass4netnsutil.NewBypass4netnsCNIBypassManager(opts.bypassClient, opts.rootlessKitClient) + bm, err := bypass4netnsutil.NewBypass4netnsCNIBypassManager(opts.bypassClient, opts.rootlessKitClient, opts.state.Annotations) if err != nil { return err }