Skip to content

Commit

Permalink
pkg/api/types: remove dependencies on nerdctl
Browse files Browse the repository at this point in the history 
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 <davbson@amazon.com>
  • Loading branch information
@sondavidb
sondavidb committed Mar 25, 2024
1 parent e0c24cf commit 70ee810
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 59 deletions.
3 changes: 1 addition & 2 deletions cmd/nerdctl/image_pull.go
View file
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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(),
Expand Down
25 changes: 11 additions & 14 deletions cmd/nerdctl/network_create.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
}
10 changes: 7 additions & 3 deletions pkg/api/types/image_types.go
View file
Expand Up @@ -18,8 +18,6 @@ package types

import (
"io"

"github.com/containerd/nerdctl/v2/pkg/imgutil"
)

// ImageListOptions specifies options for `nerdctl image list`.
Expand Down Expand Up @@ -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
Expand All @@ -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`.
Expand Down
15 changes: 11 additions & 4 deletions pkg/api/types/network_types.go
View file
Expand Up @@ -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`.
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/compose/compose.go
View file
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/cmd/network/create.go
View file
Expand Up @@ -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
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/imgutil/imgutil.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
17 changes: 7 additions & 10 deletions pkg/imgutil/snapshotter.go
View file
Expand Up @@ -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"
)
Expand All @@ -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
}

Expand All @@ -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)
Expand All @@ -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))
Expand All @@ -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)
}
3 changes: 2 additions & 1 deletion pkg/imgutil/snapshotter_test.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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{}
Expand Down
3 changes: 2 additions & 1 deletion pkg/ipfs/image.go
View file
Expand Up @@ -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"
Expand All @@ -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
Expand Down
18 changes: 3 additions & 15 deletions pkg/netutil/netutil.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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},
Expand Down

0 comments on commit 70ee810

Please sign in to comment.