Skip to content
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

Jump to bottom

Support default platform config in nerdctl.toml #1184

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Support default platform config in nerdctl.toml #1184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions cmd/nerdctl/build.go
View file
Open in desktop
Expand Up @@ -28,6 +28,7 @@ import (

"path/filepath"

"github.com/compose-spec/compose-go/types"
"github.com/containerd/containerd/errdefs"
dockerreference "github.com/containerd/containerd/reference/docker"
"github.com/containerd/nerdctl/pkg/buildkitutil"
Expand All @@ -39,7 +40,7 @@ import (
"github.com/spf13/cobra"
)

func newBuildCommand() *cobra.Command {
func newBuildCommand(cfg *types.BuildConfig) *cobra.Command {
var buildCommand = &cobra.Command{
Use: "build [flags] PATH",
Short: "Build an image from a Dockerfile. Needs buildkitd to be running.",
Expand All @@ -66,7 +67,7 @@ If Dockerfile is not present and -f is not specified, it will look for Container

// #region platform flags
// platform is defined as StringSlice, not StringArray, to allow specifying "--platform=amd64,arm64"
buildCommand.Flags().StringSlice("platform", []string{}, "Set target platform for build (e.g., \"amd64\", \"arm64\")")
buildCommand.Flags().StringSlice("platform", cfg.Platforms, "Set target platform for build (e.g., \"amd64\", \"arm64\")")
buildCommand.RegisterFlagCompletionFunc("platform", shellCompletePlatforms)
// #endregion

Expand Down
51 changes: 51 additions & 0 deletions cmd/nerdctl/build_test.go
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@ import (
"strings"
"testing"

ncdefaults "github.com/containerd/nerdctl/pkg/defaults"
"github.com/containerd/nerdctl/pkg/testutil"
"gotest.tools/v3/assert"
)
Expand Down Expand Up @@ -420,3 +421,53 @@ CMD ["echo", "nerdctl-build-notag-string"]
base.Cmd("images").AssertOutContains("<none>")
base.Cmd("image", "prune", "--force", "--all").AssertOK()
}

func TestBuildWithConfigFile(t *testing.T) {
testutil.DockerIncompatible(t)
testutil.RequiresBuild(t)
base := testutil.NewBase(t)
defer base.Cmd("builder", "prune").AssertOK()

tomlPath := ncdefaults.NerdctlTOML()
err := os.MkdirAll(filepath.Dir(tomlPath), 0755)
assert.NilError(t, err)
defer func(path string) {
_ = os.Remove(path)
}(tomlPath)

err = os.WriteFile(tomlPath, []byte(`
namespace = "normal"
[default_config]

[default_config.normal]
build = {Platforms=["linux/amd64", "linux/arm64"]}
`), 0755)
assert.NilError(t, err)

if len(base.Env) == 0 {
base.Env = os.Environ()
}
base.Env = append(base.Env, "NERDCTL_TOML="+tomlPath)

imageName := testutil.Identifier(t)
defer base.Cmd("rmi", imageName).Run()

dockerfile := fmt.Sprintf(`FROM %s
CMD ["echo", "dummy"]
`, testutil.CommonImage)

buildCtx, err := createBuildContext(dockerfile)
assert.NilError(t, err)
defer os.RemoveAll(buildCtx)

base.Cmd("build", "-t", imageName, buildCtx).AssertOK()
testCases := map[string]string{
"amd64": "x86_64",
"arm64": "aarch64",
}
for plat, expectedUnameM := range testCases {
t.Logf("Testing %q (%q)", plat, expectedUnameM)
cmd := base.Cmd("run", "--rm", "--platform="+plat, imageName, "uname", "-m")
cmd.AssertOutExactly(expectedUnameM + "\n")
}
}
8 changes: 5 additions & 3 deletions cmd/nerdctl/container.go
View file
Open in desktop
Expand Up @@ -17,10 +17,11 @@
package main

import (
"github.com/compose-spec/compose-go/types"
"github.com/spf13/cobra"
)

func newContainerCommand() *cobra.Command {
func newContainerCommand(cfg *types.ServiceConfig) *cobra.Command {
containerCommand := &cobra.Command{
Annotations: map[string]string{Category: Management},
Use: "container",
Expand All @@ -29,9 +30,10 @@ func newContainerCommand() *cobra.Command {
SilenceUsage: true,
SilenceErrors: true,
}

containerCommand.AddCommand(
newCreateCommand(),
newRunCommand(),
newCreateCommand(cfg),
newRunCommand(cfg),
newUpdateCommand(),
newExecCommand(),
containerLsCommand(),
Expand Down
5 changes: 3 additions & 2 deletions cmd/nerdctl/create.go
View file
Open in desktop
Expand Up @@ -20,10 +20,11 @@ import (
"fmt"
"runtime"

"github.com/compose-spec/compose-go/types"
"github.com/spf13/cobra"
)

func newCreateCommand() *cobra.Command {
func newCreateCommand(cfg *types.ServiceConfig) *cobra.Command {
shortHelp := "Create a new container. Optionally specify \"ipfs://\" or \"ipns://\" scheme to pull image from IPFS."
longHelp := shortHelp
switch runtime.GOOS {
Expand All @@ -45,7 +46,7 @@ func newCreateCommand() *cobra.Command {
SilenceErrors: true,
}
createCommand.Flags().SetInterspersed(false)
setCreateFlags(createCommand)
setCreateFlags(createCommand, cfg)
return createCommand
}

Expand Down
5 changes: 3 additions & 2 deletions cmd/nerdctl/image.go
View file
Open in desktop
Expand Up @@ -17,10 +17,11 @@
package main

import (
"github.com/compose-spec/compose-go/types"
"github.com/spf13/cobra"
)

func newImageCommand() *cobra.Command {
func newImageCommand(cfg *types.BuildConfig) *cobra.Command {
cmd := &cobra.Command{
Annotations: map[string]string{Category: Management},
Use: "image",
Expand All @@ -30,7 +31,7 @@ func newImageCommand() *cobra.Command {
SilenceErrors: true,
}
cmd.AddCommand(
newBuildCommand(),
newBuildCommand(cfg),
// commitCommand is in "container", not in "image"
imageLsCommand(),
newHistoryCommand(),
Expand Down
112 changes: 90 additions & 22 deletions cmd/nerdctl/main.go
View file
Open in desktop
Expand Up @@ -24,11 +24,13 @@ import (
"strconv"
"strings"

"github.com/compose-spec/compose-go/types"
"github.com/containerd/containerd"
"github.com/containerd/containerd/defaults"
"github.com/containerd/containerd/namespaces"
ncdefaults "github.com/containerd/nerdctl/pkg/defaults"
"github.com/containerd/nerdctl/pkg/logging"
"github.com/containerd/nerdctl/pkg/reflectutil"
"github.com/containerd/nerdctl/pkg/rootlessutil"
"github.com/containerd/nerdctl/pkg/version"
"github.com/fatih/color"
Expand Down Expand Up @@ -136,18 +138,70 @@ func xmain() error {
// Config corresponds to nerdctl.toml .
// See docs/config.md .
type Config struct {
Debug bool `toml:"debug"`
DebugFull bool `toml:"debug_full"`
Address string `toml:"address"`
Namespace string `toml:"namespace"`
Snapshotter string `toml:"snapshotter"`
CNIPath string `toml:"cni_path"`
CNINetConfPath string `toml:"cni_netconfpath"`
DataRoot string `toml:"data_root"`
CgroupManager string `toml:"cgroup_manager"`
InsecureRegistry bool `toml:"insecure_registry"`
HostsDir []string `toml:"hosts_dir"`
Experimental bool `toml:"experimental"`
Debug bool `toml:"debug"`
DebugFull bool `toml:"debug_full"`
Address string `toml:"address"`
Namespace string `toml:"namespace"`
Snapshotter string `toml:"snapshotter"`
CNIPath string `toml:"cni_path"`
CNINetConfPath string `toml:"cni_netconfpath"`
DataRoot string `toml:"data_root"`
CgroupManager string `toml:"cgroup_manager"`
InsecureRegistry bool `toml:"insecure_registry"`
HostsDir []string `toml:"hosts_dir"`
Experimental bool `toml:"experimental"`
DefaultConfig ConfigTemplates `toml:"default_config"`
}

type ConfigTemplates map[string]types.ServiceConfig

func (d *ConfigTemplates) UnmarshalTOML(v interface{}) error {
defaultConfig, ok := v.(map[string]interface{})
if !ok {
return fmt.Errorf("parse ConfigTemplates error")
}

res := make(map[string]types.ServiceConfig)
for namespace, vl := range defaultConfig {
dcfg := defaultConfigTemplate()
tree, err := toml.TreeFromMap(vl.(map[string]interface{}))
if err != nil {
return err
}
if err = tree.Unmarshal(&dcfg); err != nil {
return err
}
warnUnknownFields(dcfg, namespace)
res[namespace] = dcfg
}
*d = res
return nil
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we reuse the compose structure? https://pkg.go.dev/github.com/compose-spec/compose-go@v1.2.8/types#ServiceConfig

(Some fields such as image and container_name cannot be supported and should raise an error.)

Copy link
Member

Choose a reason for hiding this comment

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

I think we should not reuse https://pkg.go.dev/github.com/compose-spec/compose-go@v1.2.8/types#ServiceConfig.

I think this would be a little bit complicated for user. For instance, like #1128, maybe people just need config like this

[default_run_config]
network=["test"]

Copy link
Author

Choose a reason for hiding this comment

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

I'm afraid we can't reuse the compose structure. I agree with @Zheaoli ’s option, the ServiceConfig is complicated for users. Another reason is that even if the parameters have the same name, they may not have the same type in different scenarios, e.g. platform only needs to be a string in the run command, but it needs to be a []string in the build command because it needs to support parsing parameters like arm64,amd64

Copy link
Member

Choose a reason for hiding this comment

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

build

Platforms should be added as []string in the BuildConfig struct
https://pkg.go.dev/github.com/compose-spec/compose-go@v1.2.8/types#BuildConfig

Copy link
Author

Choose a reason for hiding this comment

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

Hi @AkihiroSuda, I'd like to do some double-check to make sure I understand your suggestion correctly:

[default_run_config] will be decoded as ServiceConfig and [default_build_config] will be decoded as BuildConfig, right?

And another question,

(Some fields such as image and container_name cannot be supported and should raise an error.)

which means If the user adds these unsupported fields to the [default_run_config] section of the Nerdctl.toml file, we should raise an error when decoding?

Copy link
Member

Choose a reason for hiding this comment

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

[default_run_config] will be decoded as ServiceConfig and [default_build_config] will be decoded as BuildConfig, right?

ServiceConfig contains BuildConfig, so no need to have [default_build_config].
A single default_template (per namespace) will suffice.

which means If the user adds these unsupported fields to the [default_run_config] section of the Nerdctl.toml file, we should raise an error when decoding?

Yes. At least a warning should be printed.
Like this: https://github.com/containerd/nerdctl/search?q=reflectutil.UnknownNonEmptyFields

Copy link
Member

Choose a reason for hiding this comment

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

Linking BuildConfig.Platforms PRs

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your patience and useful hints, I will follow the advice to complete the implementation


func defaultConfigTemplate() types.ServiceConfig {
return types.ServiceConfig{
Build: &types.BuildConfig{
Platforms: []string{},
},
Platform: "",
}
}

func warnUnknownFields(svc types.ServiceConfig, namespace string) {
if unknown := reflectutil.UnknownNonEmptyFields(&svc,
"Build",
"Platform",
); len(unknown) > 0 {
logrus.Warnf("Ignoring: [namespace %s] service %s: %+v", namespace, svc.Name, unknown)
}

if svc.Build != nil {
if unknown := reflectutil.UnknownNonEmptyFields(svc.Build,
"Platforms",
); len(unknown) > 0 {
logrus.Warnf("Ignoring: [namespace %s] service %s: build: %+v", namespace, svc.Name, unknown)
}
}
}

// NewConfig creates a default Config object statically,
Expand All @@ -166,23 +220,24 @@ func NewConfig() *Config {
InsecureRegistry: false,
HostsDir: ncdefaults.HostsDirs(),
Experimental: true,
DefaultConfig: make(map[string]types.ServiceConfig),
}
}

func initRootCmdFlags(rootCmd *cobra.Command, tomlPath string) (*pflag.FlagSet, error) {
func initRootCmdFlags(rootCmd *cobra.Command, tomlPath string) (*pflag.FlagSet, map[string]types.ServiceConfig, error) {
cfg := NewConfig()
if r, err := os.Open(tomlPath); err == nil {
logrus.Debugf("Loading config from %q", tomlPath)
defer r.Close()
dec := toml.NewDecoder(r).Strict(true) // set Strict to detect typo
if err := dec.Decode(cfg); err != nil {
return nil, fmt.Errorf("failed to load nerdctl config (not daemon config) from %q (Hint: don't mix up daemon's `config.toml` with `nerdctl.toml`): %w", tomlPath, err)
return nil, nil, fmt.Errorf("failed to load nerdctl config (not daemon config) from %q (Hint: don't mix up daemon's `config.toml` with `nerdctl.toml`): %w", tomlPath, err)
}
logrus.Debugf("Loaded config %+v", cfg)
} else {
logrus.WithError(err).Debugf("Not loading config from %q", tomlPath)
if !errors.Is(err, os.ErrNotExist) {
return nil, err
return nil, nil, err
}
}
aliasToBeInherited := pflag.NewFlagSet(rootCmd.Name(), pflag.ExitOnError)
Expand All @@ -207,7 +262,7 @@ func initRootCmdFlags(rootCmd *cobra.Command, tomlPath string) (*pflag.FlagSet,
rootCmd.PersistentFlags().StringSlice("hosts-dir", cfg.HostsDir, "A directory that contains <HOST:PORT>/hosts.toml (containerd style) or <HOST:PORT>/{ca.cert, cert.pem, key.pem} (docker style)")
// Experimental enable experimental feature, see in https://github.com/containerd/nerdctl/blob/main/docs/experimental.md
AddPersistentBoolFlag(rootCmd, "experimental", nil, nil, cfg.Experimental, "NERDCTL_EXPERIMENTAL", "Control experimental: https://github.com/containerd/nerdctl/blob/main/docs/experimental.md")
return aliasToBeInherited, nil
return aliasToBeInherited, cfg.DefaultConfig, nil
}

func newApp() (*cobra.Command, error) {
Expand All @@ -232,10 +287,23 @@ Config file ($NERDCTL_TOML): %s
}

rootCmd.SetUsageFunc(usage)
aliasToBeInherited, err := initRootCmdFlags(rootCmd, tomlPath)
aliasToBeInherited, defaultConfigs, err := initRootCmdFlags(rootCmd, tomlPath)
if err != nil {
return nil, err
}

ns, err := rootCmd.Flags().GetString("namespace")
if err != nil {
return nil, err
}
namespaceCfg, ok := defaultConfigs[ns]
if !ok {
namespaceCfg = defaultConfigTemplate()
}
//size := len(namespaceCfg.Build.Platforms)
//if size != 0 && namespaceCfg.Platform != namespaceCfg.Build.Platforms[size-1] {
// return nil, fmt.Errorf("conflict platform config")
//}

rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
debug, err := cmd.Flags().GetBool("debug-full")
Expand Down Expand Up @@ -274,9 +342,9 @@ Config file ($NERDCTL_TOML): %s
}
rootCmd.RunE = unknownSubcommandAction
rootCmd.AddCommand(
newCreateCommand(),
newCreateCommand(&namespaceCfg),
// #region Run & Exec
newRunCommand(),
newRunCommand(&namespaceCfg),
newUpdateCommand(),
newExecCommand(),
// #endregion
Expand All @@ -298,7 +366,7 @@ Config file ($NERDCTL_TOML): %s
// #endregion

// Build
newBuildCommand(),
newBuildCommand(namespaceCfg.Build),

// #region Image management
newImagesCommand(),
Expand All @@ -325,8 +393,8 @@ Config file ($NERDCTL_TOML): %s
newStatsCommand(),

// #region Management
newContainerCommand(),
newImageCommand(),
newContainerCommand(&namespaceCfg),
newImageCommand(namespaceCfg.Build),
newNetworkCommand(),
newVolumeCommand(),
newSystemCommand(),
Expand Down