From 70ee810a4738af420ac3743f6110229f1f096166 Mon Sep 17 00:00:00 2001 From: David Son Date: Fri, 22 Mar 2024 18:10:21 +0000 Subject: [PATCH] pkg/api/types: remove dependencies on nerdctl pkg/api/types had a dependency on netutil and imgutil packages. This would create issues with circular dependencies, e.g. in #2638 imgutil could not import types due to its dependency on netutil. This change moves NetworkCreateOptions and RemoteSnapshotterFlags into types to remove any future circular dependencies. This should also help us to clean our function headers, since we no longer need to explode the options struct into many different pieces. Signed-off-by: David Son --- cmd/nerdctl/image_pull.go | 3 +-- cmd/nerdctl/network_create.go | 25 +++++++++++-------------- pkg/api/types/image_types.go | 10 +++++++--- pkg/api/types/network_types.go | 15 +++++++++++---- pkg/cmd/compose/compose.go | 4 ++-- pkg/cmd/network/create.go | 10 +++++----- pkg/imgutil/imgutil.go | 5 +++-- pkg/imgutil/snapshotter.go | 17 +++++++---------- pkg/imgutil/snapshotter_test.go | 3 ++- pkg/ipfs/image.go | 3 ++- pkg/netutil/netutil.go | 18 +++--------------- 11 files changed, 54 insertions(+), 59 deletions(-) diff --git a/cmd/nerdctl/image_pull.go b/cmd/nerdctl/image_pull.go index 8c478fb68d..ffb5f45e87 100644 --- a/cmd/nerdctl/image_pull.go +++ b/cmd/nerdctl/image_pull.go @@ -20,7 +20,6 @@ import ( "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/clientutil" "github.com/containerd/nerdctl/v2/pkg/cmd/image" - "github.com/containerd/nerdctl/v2/pkg/imgutil" "github.com/spf13/cobra" ) @@ -113,7 +112,7 @@ func processPullCommandFlags(cmd *cobra.Command) (types.ImagePullOptions, error) Unpack: unpackStr, Quiet: quiet, IPFSAddress: ipfsAddressStr, - RFlags: imgutil.RemoteSnapshotterFlags{ + RFlags: types.RemoteSnapshotterFlags{ SociIndexDigest: sociIndexDigest, }, Stdout: cmd.OutOrStdout(), diff --git a/cmd/nerdctl/network_create.go b/cmd/nerdctl/network_create.go index 252984a45b..c5fba0cac6 100644 --- a/cmd/nerdctl/network_create.go +++ b/cmd/nerdctl/network_create.go @@ -22,7 +22,6 @@ import ( "github.com/containerd/containerd/identifiers" "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/cmd/network" - "github.com/containerd/nerdctl/v2/pkg/netutil" "github.com/containerd/nerdctl/v2/pkg/strutil" "github.com/spf13/cobra" @@ -100,18 +99,16 @@ func networkCreateAction(cmd *cobra.Command, args []string) error { } return network.Create(types.NetworkCreateOptions{ - GOptions: globalOptions, - CreateOptions: netutil.CreateOptions{ - Name: name, - Driver: driver, - Options: strutil.ConvertKVStringsToMap(opts), - IPAMDriver: ipamDriver, - IPAMOptions: strutil.ConvertKVStringsToMap(ipamOpts), - Subnets: subnets, - Gateway: gatewayStr, - IPRange: ipRangeStr, - Labels: labels, - IPv6: ipv6, - }, + GOptions: globalOptions, + Name: name, + Driver: driver, + Options: strutil.ConvertKVStringsToMap(opts), + IPAMDriver: ipamDriver, + IPAMOptions: strutil.ConvertKVStringsToMap(ipamOpts), + Subnets: subnets, + Gateway: gatewayStr, + IPRange: ipRangeStr, + Labels: labels, + IPv6: ipv6, }, cmd.OutOrStdout()) } diff --git a/pkg/api/types/image_types.go b/pkg/api/types/image_types.go index 16a6602b06..084e5bedd7 100644 --- a/pkg/api/types/image_types.go +++ b/pkg/api/types/image_types.go @@ -18,8 +18,6 @@ package types import ( "io" - - "github.com/containerd/nerdctl/v2/pkg/imgutil" ) // ImageListOptions specifies options for `nerdctl image list`. @@ -181,6 +179,12 @@ type ImagePushOptions struct { AllowNondistributableArtifacts bool } +// RemoteSnapshotterFlags are used for pulling with remote snapshotters +// e.g. SOCI, stargz, overlaybd +type RemoteSnapshotterFlags struct { + SociIndexDigest string +} + // ImagePullOptions specifies options for `nerdctl (image) pull`. type ImagePullOptions struct { Stdout io.Writer @@ -198,7 +202,7 @@ type ImagePullOptions struct { // multiaddr of IPFS API (default uses $IPFS_PATH env variable if defined or local directory ~/.ipfs) IPFSAddress string // Flags to pass into remote snapshotters - RFlags imgutil.RemoteSnapshotterFlags + RFlags RemoteSnapshotterFlags } // ImageTagOptions specifies options for `nerdctl (image) tag`. diff --git a/pkg/api/types/network_types.go b/pkg/api/types/network_types.go index c8fa1cc564..5cb26b3ea1 100644 --- a/pkg/api/types/network_types.go +++ b/pkg/api/types/network_types.go @@ -18,16 +18,23 @@ package types import ( "io" - - "github.com/containerd/nerdctl/v2/pkg/netutil" ) // NetworkCreateOptions specifies options for `nerdctl network create`. type NetworkCreateOptions struct { // GOptions is the global options GOptions GlobalCommandOptions - // CreateOptions is the option for creating network - CreateOptions netutil.CreateOptions + + Name string + Driver string + Options map[string]string + IPAMDriver string + IPAMOptions map[string]string + Subnets []string + Gateway string + IPRange string + Labels []string + IPv6 bool } // NetworkInspectOptions specifies options for `nerdctl network inspect`. diff --git a/pkg/cmd/compose/compose.go b/pkg/cmd/compose/compose.go index 6517412088..515a91a8a7 100644 --- a/pkg/cmd/compose/compose.go +++ b/pkg/cmd/compose/compose.go @@ -125,7 +125,7 @@ func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, op ipfsPath = dir } _, err = ipfs.EnsureImage(ctx, client, stdout, stderr, globalOptions.Snapshotter, scheme, ref, - pullMode, ocispecPlatforms, nil, quiet, ipfsPath, imgutil.RemoteSnapshotterFlags{}) + pullMode, ocispecPlatforms, nil, quiet, ipfsPath, types.RemoteSnapshotterFlags{}) return err } @@ -136,7 +136,7 @@ func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, op } _, err = imgutil.EnsureImage(ctx, client, stdout, stderr, globalOptions.Snapshotter, ref, - pullMode, globalOptions.InsecureRegistry, globalOptions.HostsDir, ocispecPlatforms, nil, quiet, imgutil.RemoteSnapshotterFlags{}) + pullMode, globalOptions.InsecureRegistry, globalOptions.HostsDir, ocispecPlatforms, nil, quiet, types.RemoteSnapshotterFlags{}) return err } diff --git a/pkg/cmd/network/create.go b/pkg/cmd/network/create.go index f1dd42a1b6..3115a12764 100644 --- a/pkg/cmd/network/create.go +++ b/pkg/cmd/network/create.go @@ -26,21 +26,21 @@ import ( ) func Create(options types.NetworkCreateOptions, stdout io.Writer) error { - if len(options.CreateOptions.Subnets) == 0 { - if options.CreateOptions.Gateway != "" || options.CreateOptions.IPRange != "" { + if len(options.Subnets) == 0 { + if options.Gateway != "" || options.IPRange != "" { return fmt.Errorf("cannot set gateway or ip-range without subnet, specify --subnet manually") } - options.CreateOptions.Subnets = []string{""} + options.Subnets = []string{""} } e, err := netutil.NewCNIEnv(options.GOptions.CNIPath, options.GOptions.CNINetConfPath) if err != nil { return err } - net, err := e.CreateNetwork(options.CreateOptions) + net, err := e.CreateNetwork(options) if err != nil { if errdefs.IsAlreadyExists(err) { - return fmt.Errorf("network with name %s already exists", options.CreateOptions.Name) + return fmt.Errorf("network with name %s already exists", options.Name) } return err } diff --git a/pkg/imgutil/imgutil.go b/pkg/imgutil/imgutil.go index 1608d64203..e51850920a 100644 --- a/pkg/imgutil/imgutil.go +++ b/pkg/imgutil/imgutil.go @@ -32,6 +32,7 @@ import ( "github.com/containerd/imgcrypt" "github.com/containerd/imgcrypt/images/encryption" "github.com/containerd/log" + "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/errutil" "github.com/containerd/nerdctl/v2/pkg/idutil/imagewalker" "github.com/containerd/nerdctl/v2/pkg/imgutil/dockerconfigresolver" @@ -103,7 +104,7 @@ func GetExistingImage(ctx context.Context, client *containerd.Client, snapshotte // # When insecure is set, skips verifying certs, and also falls back to HTTP when the registry does not speak HTTPS // // FIXME: this func has too many args -func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr io.Writer, snapshotter, rawRef string, mode PullMode, insecure bool, hostsDirs []string, ocispecPlatforms []ocispec.Platform, unpack *bool, quiet bool, rFlags RemoteSnapshotterFlags) (*EnsuredImage, error) { +func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr io.Writer, snapshotter, rawRef string, mode PullMode, insecure bool, hostsDirs []string, ocispecPlatforms []ocispec.Platform, unpack *bool, quiet bool, rFlags types.RemoteSnapshotterFlags) (*EnsuredImage, error) { switch mode { case "always", "missing", "never": // NOP @@ -194,7 +195,7 @@ func ResolveDigest(ctx context.Context, rawRef string, insecure bool, hostsDirs } // PullImage pulls an image using the specified resolver. -func PullImage(ctx context.Context, client *containerd.Client, stdout, stderr io.Writer, snapshotter string, resolver remotes.Resolver, ref string, ocispecPlatforms []ocispec.Platform, unpack *bool, quiet bool, rFlags RemoteSnapshotterFlags) (*EnsuredImage, error) { +func PullImage(ctx context.Context, client *containerd.Client, stdout, stderr io.Writer, snapshotter string, resolver remotes.Resolver, ref string, ocispecPlatforms []ocispec.Platform, unpack *bool, quiet bool, rFlags types.RemoteSnapshotterFlags) (*EnsuredImage, error) { ctx, done, err := client.WithLease(ctx) if err != nil { return nil, err diff --git a/pkg/imgutil/snapshotter.go b/pkg/imgutil/snapshotter.go index 46ff8b5f3a..9fd30e72f8 100644 --- a/pkg/imgutil/snapshotter.go +++ b/pkg/imgutil/snapshotter.go @@ -24,6 +24,7 @@ import ( "github.com/containerd/containerd/images" ctdsnapshotters "github.com/containerd/containerd/pkg/snapshotters" "github.com/containerd/log" + "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/imgutil/pull" "github.com/containerd/stargz-snapshotter/fs/source" ) @@ -48,14 +49,10 @@ var builtinRemoteSnapshotterOpts = map[string]snapshotterOpts{ snapshotterNameCvmfs: &remoteSnapshotterOpts{snapshotter: "cvmfs-snapshotter"}, } -type RemoteSnapshotterFlags struct { - SociIndexDigest string -} - // snapshotterOpts is used to update pull config // for different snapshotters type snapshotterOpts interface { - apply(config *pull.Config, ref string, rFlags RemoteSnapshotterFlags) + apply(config *pull.Config, ref string, rFlags types.RemoteSnapshotterFlags) isRemote() bool } @@ -77,14 +74,14 @@ func getSnapshotterOpts(snapshotter string) snapshotterOpts { // interface `snapshotterOpts.isRemote()` function type remoteSnapshotterOpts struct { snapshotter string - extraLabels func(func(images.Handler) images.Handler, RemoteSnapshotterFlags) func(images.Handler) images.Handler + extraLabels func(func(images.Handler) images.Handler, types.RemoteSnapshotterFlags) func(images.Handler) images.Handler } func (rs *remoteSnapshotterOpts) isRemote() bool { return true } -func (rs *remoteSnapshotterOpts) apply(config *pull.Config, ref string, rFlags RemoteSnapshotterFlags) { +func (rs *remoteSnapshotterOpts) apply(config *pull.Config, ref string, rFlags types.RemoteSnapshotterFlags) { h := ctdsnapshotters.AppendInfoHandlerWrapper(ref) if rs.extraLabels != nil { h = rs.extraLabels(h, rFlags) @@ -102,7 +99,7 @@ type defaultSnapshotterOpts struct { snapshotter string } -func (dsn *defaultSnapshotterOpts) apply(config *pull.Config, _ref string, rFlags RemoteSnapshotterFlags) { +func (dsn *defaultSnapshotterOpts) apply(config *pull.Config, _ref string, rFlags types.RemoteSnapshotterFlags) { config.RemoteOpts = append( config.RemoteOpts, containerd.WithPullSnapshotter(dsn.snapshotter)) @@ -113,10 +110,10 @@ func (dsn *defaultSnapshotterOpts) isRemote() bool { return false } -func stargzExtraLabels(f func(images.Handler) images.Handler, rFlags RemoteSnapshotterFlags) func(images.Handler) images.Handler { +func stargzExtraLabels(f func(images.Handler) images.Handler, rFlags types.RemoteSnapshotterFlags) func(images.Handler) images.Handler { return source.AppendExtraLabelsHandler(prefetchSize, f) } -func sociExtraLabels(f func(images.Handler) images.Handler, rFlags RemoteSnapshotterFlags) func(images.Handler) images.Handler { +func sociExtraLabels(f func(images.Handler) images.Handler, rFlags types.RemoteSnapshotterFlags) func(images.Handler) images.Handler { return socisource.AppendDefaultLabelsHandlerWrapper(rFlags.SociIndexDigest, f) } diff --git a/pkg/imgutil/snapshotter_test.go b/pkg/imgutil/snapshotter_test.go index f09da01401..e719b8086b 100644 --- a/pkg/imgutil/snapshotter_test.go +++ b/pkg/imgutil/snapshotter_test.go @@ -23,6 +23,7 @@ import ( "github.com/containerd/containerd" ctdsnapshotters "github.com/containerd/containerd/pkg/snapshotters" + "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/imgutil/pull" digest "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -93,7 +94,7 @@ func sameOpts(want snapshotterOpts) func(*testing.T, snapshotterOpts) { func getAndApplyRemoteOpts(t *testing.T, sn string) *containerd.RemoteContext { config := &pull.Config{} snOpts := getSnapshotterOpts(sn) - rFlags := RemoteSnapshotterFlags{} + rFlags := types.RemoteSnapshotterFlags{} snOpts.apply(config, testRef, rFlags) rc := &containerd.RemoteContext{} diff --git a/pkg/ipfs/image.go b/pkg/ipfs/image.go index ffe2db4eb3..41e1b40f6c 100644 --- a/pkg/ipfs/image.go +++ b/pkg/ipfs/image.go @@ -27,6 +27,7 @@ import ( "github.com/containerd/containerd/images/converter" "github.com/containerd/containerd/remotes" "github.com/containerd/log" + "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/idutil/imagewalker" "github.com/containerd/nerdctl/v2/pkg/imgutil" "github.com/containerd/nerdctl/v2/pkg/platformutil" @@ -40,7 +41,7 @@ import ( const ipfsPathEnv = "IPFS_PATH" // EnsureImage pull the specified image from IPFS. -func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr io.Writer, snapshotter string, scheme string, ref string, mode imgutil.PullMode, ocispecPlatforms []ocispec.Platform, unpack *bool, quiet bool, ipfsPath string, rFlags imgutil.RemoteSnapshotterFlags) (*imgutil.EnsuredImage, error) { +func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr io.Writer, snapshotter string, scheme string, ref string, mode imgutil.PullMode, ocispecPlatforms []ocispec.Platform, unpack *bool, quiet bool, ipfsPath string, rFlags types.RemoteSnapshotterFlags) (*imgutil.EnsuredImage, error) { switch mode { case "always", "missing", "never": // NOP diff --git a/pkg/netutil/netutil.go b/pkg/netutil/netutil.go index 1a29d7dc67..5691e27e8d 100644 --- a/pkg/netutil/netutil.go +++ b/pkg/netutil/netutil.go @@ -34,6 +34,7 @@ import ( "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/namespaces" "github.com/containerd/log" + "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/labels" "github.com/containerd/nerdctl/v2/pkg/lockutil" "github.com/containerd/nerdctl/v2/pkg/netutil/nettype" @@ -225,20 +226,7 @@ type cniNetworkConfig struct { Plugins []CNIPlugin `json:"plugins"` } -type CreateOptions struct { - Name string - Driver string - Options map[string]string - IPAMDriver string - IPAMOptions map[string]string - Subnets []string - Gateway string - IPRange string - Labels []string - IPv6 bool -} - -func (e *CNIEnv) CreateNetwork(opts CreateOptions) (*NetworkConfig, error) { //nolint:revive +func (e *CNIEnv) CreateNetwork(opts types.NetworkCreateOptions) (*NetworkConfig, error) { //nolint:revive var net *NetworkConfig netMap, err := e.NetworkMap() if err != nil { @@ -350,7 +338,7 @@ func (e *CNIEnv) createDefaultNetworkConfig() error { if _, err := os.Stat(filename); err == nil { return fmt.Errorf("already found existing network config at %q, cannot create new network named %q", filename, DefaultNetworkName) } - opts := CreateOptions{ + opts := types.NetworkCreateOptions{ Name: DefaultNetworkName, Driver: DefaultNetworkName, Subnets: []string{DefaultCIDR},