Clean up image pulling policy

Change how we represent image pulling policy from a pair of booleans to
a single field which can take three values (never, if-missing, always),
with the default value being if-missing.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

Closes: #71
Approved by: rhatdan

buildah.go

@@ -19,6 +19,21 @@ const (
 	stateFile     = Package + ".json"
 )
 
+const (
+	// PullIfMissing is one of the values that BuilderOptions.PullPolicy
+	// can take, signalling that the source image should be pulled from a
+	// registry if a local copy of it is not already present.
+	PullIfMissing = iota
+	// PullAlways is one of the values that BuilderOptions.PullPolicy can
+	// take, signalling that a fresh, possibly updated, copy of the image
+	// should be pulled from a registry before the build proceeds.
+	PullAlways
+	// PullNever is one of the values that BuilderOptions.PullPolicy can
+	// take, signalling that the source image should not be pulled from a
+	// registry if a local copy of it is not already present.
+	PullNever
+)
+
 // Builder objects are used to represent containers which are being used to
 // build images.  They also carry potential updates which will be applied to
 // the image's configuration when the container's contents are used to build an
@@ -100,12 +115,10 @@ type BuilderOptions struct {
 	FromImage string
 	// Container is a desired name for the build container.
 	Container string
-	// PullIfMissing signals to NewBuilder() that it should pull the image
+	// PullPolicy decides whether or not we should pull the image that
-	// if it is not present in the local Store.
+	// we're using as a base image.  It should be PullIfMissing,
-	PullIfMissing bool
+	// PullAlways, or PullNever.
-	// PullAlways signals to NewBuilder() that it should pull the source
+	PullPolicy int
-	// image before creating the container.
-	PullAlways bool
 	// Registry is a value which is prepended to the image's name, if it
 	// needs to be pulled and the image name alone can not be resolved to a
 	// reference to a source image.

cmd/buildah/bud.go

@@ -77,6 +77,15 @@ func budCmd(c *cli.Context) error {
 	if c.IsSet("pull-always") {
 		pull = c.Bool("pull-always")
 	}
+
+	pullPolicy := imagebuildah.PullNever
+	if pull {
+		pullPolicy = imagebuildah.PullIfMissing
+	}
+	if pullAlways {
+		pullPolicy = imagebuildah.PullAlways
+	}
+
 	signaturePolicy := ""
 	if c.IsSet("signature-policy") {
 		signaturePolicy = c.String("signature-policy")
@@ -162,8 +171,7 @@ func budCmd(c *cli.Context) error {
 
 	options := imagebuildah.BuildOptions{
 		ContextDirectory:    contextDir,
-		PullIfMissing:       pull,
+		PullPolicy:          pullPolicy,
-		PullAlways:          pullAlways,
 		Registry:            registry,
 		Compression:         archive.Gzip,
 		Quiet:               quiet,

cmd/buildah/from.go

@@ -77,6 +77,15 @@ func fromCmd(c *cli.Context) error {
 	if c.IsSet("pull-always") {
 		pull = c.Bool("pull-always")
 	}
+
+	pullPolicy := buildah.PullNever
+	if pull {
+		pullPolicy = buildah.PullIfMissing
+	}
+	if pullAlways {
+		pullPolicy = buildah.PullAlways
+	}
+
 	name := ""
 	if c.IsSet("name") {
 		name = c.String("name")
@@ -98,8 +107,7 @@ func fromCmd(c *cli.Context) error {
 	options := buildah.BuilderOptions{
 		FromImage:           image,
 		Container:           name,
-		PullIfMissing:       pull,
+		PullPolicy:          pullPolicy,
-		PullAlways:          pullAlways,
 		Mount:               mount,
 		Registry:            registry,
 		SignaturePolicyPath: signaturePolicy,

imagebuildah/build.go

@@ -23,17 +23,20 @@ import (
 	"github.com/projectatomic/buildah"
 )
 
+const (
+	PullIfMissing = buildah.PullIfMissing
+	PullAlways    = buildah.PullAlways
+	PullNever     = buildah.PullNever
+)
+
 // BuildOptions can be used to alter how an image is built.
 type BuildOptions struct {
 	// ContextDirectory is the default source location for COPY and ADD
 	// commands.
 	ContextDirectory string
-	// PullIfMissing signals to NewBuilder() that it should pull the image
+	// PullPolicy controls whether or not we pull images.  It should be one
-	// if it is not present in the local Store.
+	// of PullIfMissing, PullAlways, or PullNever.
-	PullIfMissing bool
+	PullPolicy int
-	// PullAlways signals to NewBuilder() that it should pull the source
-	// image before creating the container.
-	PullAlways bool
 	// Registry is a value which is prepended to the image's name, if it
 	// needs to be pulled and the image name alone can not be resolved to a
 	// reference to a source image.
@@ -72,8 +75,7 @@ type Executor struct {
 	store                          storage.Store
 	contextDir                     string
 	builder                        *buildah.Builder
-	pullIfMissing                  bool
+	pullPolicy                     int
-	pullAlways                     bool
 	registry                       string
 	ignoreUnrecognizedInstructions bool
 	counter                        int
@@ -332,8 +334,7 @@ func NewExecutor(store storage.Store, options BuildOptions) (*Executor, error) {
 	exec := Executor{
 		store:                          store,
 		contextDir:                     options.ContextDirectory,
-		pullIfMissing:                  options.PullIfMissing,
+		pullPolicy:                     options.PullPolicy,
-		pullAlways:                     options.PullAlways,
 		registry:                       options.Registry,
 		ignoreUnrecognizedInstructions: options.IgnoreUnrecognizedInstructions,
 		quiet:               options.Quiet,
@@ -366,8 +367,7 @@ func (b *Executor) Prepare(ib *imagebuilder.Builder, node *parser.Node) error {
 	b.counter++
 	builderOptions := buildah.BuilderOptions{
 		FromImage:           from,
-		PullIfMissing:       b.pullIfMissing,
+		PullPolicy:          b.pullPolicy,
-		PullAlways:          b.pullAlways,
 		Registry:            b.registry,
 		SignaturePolicyPath: b.signaturePolicyPath,
 	}

new.go

@@ -64,7 +64,7 @@ func newBuilder(store storage.Store, options BuilderOptions) (*Builder, error) {
 
 	imageID := ""
 	if image != "" {
-		if options.PullAlways {
+		if options.PullPolicy == PullAlways {
 			err := pullImage(store, options, systemContext)
 			if err != nil {
 				return nil, fmt.Errorf("error pulling image %q: %v", image, err)
@@ -76,7 +76,7 @@ func newBuilder(store storage.Store, options BuilderOptions) (*Builder, error) {
 		}
 		img, err = is.Transport.GetStoreImage(store, ref)
 		if err != nil {
-			if err == storage.ErrImageUnknown && !options.PullIfMissing {
+			if err == storage.ErrImageUnknown && options.PullPolicy != PullIfMissing {
 				return nil, fmt.Errorf("no such image %q: %v", image, err)
 			}
 			err = pullImage(store, options, systemContext)