Skip to content

Commit

Permalink
Adds flag modifying pull behavior for running and creating containers
Browse files Browse the repository at this point in the history
 - Follows the proposal on issue [#34394](moby/moby#34394)
 - Maintains current behavior as default (Pull image if missing)
 - Adds tristate flag allowing modification (PullMissing, PullAlways, PullNever)

Signed-off-by: Zander Mackie <zmackie@gmail.com>
  • Loading branch information
zmackie committed Nov 14, 2018
1 parent 6b71e84 commit a43abec
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 17 deletions.
73 changes: 56 additions & 17 deletions cli/command/container/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,18 @@ import (
"github.com/spf13/pflag"
)

// Pull constants
const (
PullImageAlways = "always"
PullImageMissing = "missing"
PullImageNever = "never"
)

type createOptions struct {
name string
platform string
untrusted bool
pull string // alway, missing, never
}

// NewCreateCommand creates a new cobra.Command for `docker create`
Expand All @@ -48,6 +56,8 @@ func NewCreateCommand(dockerCli command.Cli) *cobra.Command {
flags.SetInterspersed(false)

flags.StringVar(&opts.name, "name", "", "Assign a name to the container")
flags.StringVar(&opts.pull, "pull", PullImageNever,
`Pull image before creating ("`+PullImageAlways+`"|"`+PullImageMissing+`"|"`+PullImageNever+`")`)

// Add an explicit help that doesn't have a `-h` to prevent the conflict
// with hostname
Expand Down Expand Up @@ -159,6 +169,7 @@ func newCIDFile(path string) (*cidFile, error) {
return &cidFile{path: path, file: f}, nil
}

// nolint: gocyclo
func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig *containerConfig, opts *createOptions) (*container.ContainerCreateCreatedBody, error) {
config := containerConfig.Config
hostConfig := containerConfig.HostConfig
Expand Down Expand Up @@ -194,31 +205,59 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig
}

//create the container
response, err := dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name)
var response container.ContainerCreateCreatedBody
if opts.pull == PullImageMissing { // Pull image only if it does not exist locally. Default.
response, err = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name)

//if image not found try to pull it
if err != nil {
if apiclient.IsErrNotFound(err) && namedRef != nil {
fmt.Fprintf(stderr, "Unable to find image '%s' locally\n", reference.FamiliarString(namedRef))
//if image not found try to pull it
if err != nil {
if apiclient.IsErrNotFound(err) && namedRef != nil {
fmt.Fprintf(stderr, "Unable to find image '%s' locally\n", reference.FamiliarString(namedRef))

// we don't want to write to stdout anything apart from container.ID
if err := pullImage(ctx, dockerCli, config.Image, opts.platform, stderr); err != nil {
return nil, err
}
if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil {
if err := image.TagTrusted(ctx, dockerCli, trustedRef, taggedRef); err != nil {
// we don't want to write to stdout anything apart from container.ID
if err := pullImage(ctx, dockerCli, config.Image, opts.platform, stderr); err != nil {
return nil, err
}
if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil {
if err := image.TagTrusted(ctx, dockerCli, trustedRef, taggedRef); err != nil {
return nil, err
}
}
// Retry
var retryErr error
response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name)
if retryErr != nil {
return nil, retryErr
}
} else {
return nil, err
}
// Retry
var retryErr error
response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name)
if retryErr != nil {
return nil, retryErr
}

} else if opts.pull == PullImageAlways { // Always try and pull the image.
if err := pullImage(ctx, dockerCli, config.Image, opts.platform, stderr); err != nil {
return nil, err
}
if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil {
if err := image.TagTrusted(ctx, dockerCli, trustedRef, taggedRef); err != nil {
return nil, err
}
} else {
}
response, err = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name)
if err != nil {
return nil, err
}

} else if opts.pull == PullImageNever { // Never try and pull the image
response, err = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name)

if err != nil {
if apiclient.IsErrNotFound(err) && namedRef != nil {
fmt.Fprintf(stderr, "Unable to find image '%s' locally\nWill not pull due to '%s'", reference.FamiliarString(namedRef), opts.pull)
}
}
} else { // We got something weird
return nil, errors.Errorf("Unknown pull option : %s", opts.pull)
}

for _, warning := range response.Warnings {
Expand Down
47 changes: 47 additions & 0 deletions cli/command/container/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func TestCreateContainerPullsImageIfMissing(t *testing.T) {
name: "name",
platform: runtime.GOOS,
untrusted: true,
pull: PullImageMissing,
})
assert.NilError(t, err)
expected := container.ContainerCreateCreatedBody{ID: containerID}
Expand All @@ -121,6 +122,52 @@ func TestCreateContainerPullsImageIfMissing(t *testing.T) {
assert.Check(t, is.Contains(stderr, "Unable to find image 'does-not-exist-locally:latest' locally"))
}

func TestCreateContainerNeverPullsImage(t *testing.T) {
imageName := "does-not-exist-locally"
responseCounter := 0

client := &fakeClient{
createContainerFunc: func(
config *container.Config,
hostConfig *container.HostConfig,
networkingConfig *network.NetworkingConfig,
containerName string,
) (container.ContainerCreateCreatedBody, error) {
defer func() { responseCounter++ }()
switch responseCounter {
case 0:
return container.ContainerCreateCreatedBody{}, fakeNotFound{}
default:
return container.ContainerCreateCreatedBody{}, errors.New("unexpected")
}
},
imageCreateFunc: func(parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) {
return ioutil.NopCloser(strings.NewReader("")), nil
},
infoFunc: func() (types.Info, error) {
return types.Info{IndexServerAddress: "http://indexserver"}, nil
},
}
cli := test.NewFakeCli(client)
config := &containerConfig{
Config: &container.Config{
Image: imageName,
},
HostConfig: &container.HostConfig{},
}
body, err := createContainer(context.Background(), cli, config, &createOptions{
name: "name",
platform: runtime.GOOS,
untrusted: true,
pull: PullImageNever,
})
assert.NilError(t, err)
expected := container.ContainerCreateCreatedBody{}
assert.Check(t, is.DeepEqual(expected, *body))
stderr := cli.ErrBuffer().String()
assert.Check(t, is.Contains(stderr, "Unable to find image 'does-not-exist-locally:latest' locally"))
}

func TestNewCreateCommandWithContentTrustErrors(t *testing.T) {
testCases := []struct {
name string
Expand Down
2 changes: 2 additions & 0 deletions cli/command/container/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ func NewRunCommand(dockerCli command.Cli) *cobra.Command {
flags.BoolVar(&opts.sigProxy, "sig-proxy", true, "Proxy received signals to the process")
flags.StringVar(&opts.name, "name", "", "Assign a name to the container")
flags.StringVar(&opts.detachKeys, "detach-keys", "", "Override the key sequence for detaching a container")
flags.StringVar(&opts.createOptions.pull, "pull", PullImageNever,
`Pull image before running ("`+PullImageAlways+`"|"`+PullImageMissing+`"|"`+PullImageNever+`")`)

// Add an explicit help that doesn't have a `-h` to prevent the conflict
// with hostname
Expand Down

0 comments on commit a43abec

Please sign in to comment.