Honor podman shortnames behavior and allow non-docker transports

Closes hashicorp#97
Closes hashicorp#69

api/image_inspect.go

@@ -0,0 +1,39 @@
+package api
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+)
+
+// This is terribly simplified but should be enough
+type inspectImageResponse struct {
+	Id string `json:"Id"`
+}
+
+func (c *API) ImageInspectID(ctx context.Context, image string) (string, error) {
+	var inspectData inspectImageResponse
+
+	res, err := c.Get(ctx, fmt.Sprintf("/v1.0.0/libpod/images/%s/json", image))
+	if err != nil {
+		return "", err
+	}
+
+	defer res.Body.Close()
+	body, err := ioutil.ReadAll(res.Body)
+	if err != nil {
+		return "", err
+	}
+
+	if res.StatusCode != http.StatusOK {
+		return "", fmt.Errorf("unknown error, status code: %d: %s", res.StatusCode, body)
+	}
+	err = json.Unmarshal(body, &inspectData)
+	if err != nil {
+		return "", err
+	}
+
+	return inspectData.Id, nil
+}

api/image_load.go

@@ -0,0 +1,36 @@
+package api
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"os"
+)
+
+// ImageLoad uploads a tar archive as an image
+func (c *API) ImageLoad(ctx context.Context, path string) (string, error) {
+	archive, err := os.Open(path)
+	defer archive.Close()
+	names := []string{}
+
+	res, err := c.Post(ctx, "/v1.0.0/libpod/images/load", archive)
+	if err != nil {
+		return "", err
+	}
+
+	defer res.Body.Close()
+	body, err := ioutil.ReadAll(res.Body)
+	if err != nil {
+		return "", err
+	}
+	if res.StatusCode != http.StatusOK {
+		return "", fmt.Errorf("unknown error, status code: %d: %s", res.StatusCode, body)
+	}
+	err = json.Unmarshal(body, &names)
+	if err != nil {
+		return "", err
+	}
+	return names[0], nil
+}

api/image_pull.go

@@ -11,17 +11,18 @@ import (
 )
 
 // ImagePull pulls a image from a remote location to local storage
-func (c *API) ImagePull(ctx context.Context, nameWithTag string) error {
+func (c *API) ImagePull(ctx context.Context, nameWithTag string) (string, error) {
+	var id string
 
 	res, err := c.Post(ctx, fmt.Sprintf("/v1.0.0/libpod/images/pull?reference=%s", nameWithTag), nil)
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	defer res.Body.Close()
 	if res.StatusCode != http.StatusOK {
 		body, _ := ioutil.ReadAll(res.Body)
-		return fmt.Errorf("unknown error, status code: %d: %s", res.StatusCode, body)
+		return "", fmt.Errorf("unknown error, status code: %d: %s", res.StatusCode, body)
 	}
 
 	dec := json.NewDecoder(res.Body)
@@ -30,13 +31,16 @@ func (c *API) ImagePull(ctx context.Context, nameWithTag string) error {
 		if err = dec.Decode(&report); err == io.EOF {
 			break
 		} else if err != nil {
-			return fmt.Errorf("Error reading response: %w", err)
+			return "", fmt.Errorf("Error reading response: %w", err)
 		}
 
 		if report.Error != "" {
-			return errors.New(report.Error)
+			return "", errors.New(report.Error)
 		}
-	}
 
-	return nil
+		if report.ID != "" {
+			id = report.ID
+		}
+	}
+	return id, nil
 }

api/image_pull_test.go

@@ -22,9 +22,10 @@ func TestApi_Image_Pull(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
-		err := api.ImagePull(ctx, testCase.Image)
+		id, err := api.ImagePull(ctx, testCase.Image)
 		if testCase.Exists {
 			assert.NoError(t, err)
+			assert.NotEqual(t, "", id)
 		} else {
 			assert.Error(t, err)
 		}

driver.go

@@ -25,9 +25,14 @@ import (
 	"github.com/hashicorp/nomad/plugins/shared/hclspec"
 	pstructs "github.com/hashicorp/nomad/plugins/shared/structs"
 
-	dockerref "github.com/docker/distribution/reference"
 	shelpers "github.com/hashicorp/nomad/helper/stats"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
+
+	"github.com/containers/image/v5/docker"
+	dockerArchive "github.com/containers/image/v5/docker/archive"
+	ociArchive "github.com/containers/image/v5/oci/archive"
+	"github.com/containers/image/v5/pkg/shortnames"
+	"github.com/containers/image/v5/types"
 )
 
 const (
@@ -484,30 +489,11 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
 	}
 
 	if !recoverRunningContainer {
-		// FIXME: there are more variations of image sources, we should handle it
-		//        e.g. oci-archive:/... etc
-		//        see also https://github.com/hashicorp/nomad-driver-podman/issues/69
-
-		imageName, err := parseImage(createOpts.Image)
-		if err != nil {
-			return nil, nil, fmt.Errorf("failed to start task, unable to parse image reference %s: %v", createOpts.Image, err)
-		}
-
-		// do we already have this image in local storage?
-		haveImage, err := d.podman.ImageExists(d.ctx, imageName)
+		imageID, err := d.createImage(createOpts.Image)
 		if err != nil {
-			d.logger.Warn("Unable to check for local image", "image", imageName, "err", err)
-			// do NOT fail this operation, instead try to pull the image
-			haveImage = false
+			return nil, nil, fmt.Errorf("failed to create image: %s: %w", createOpts.Image, err)
 		}
-		if !haveImage {
-			d.logger.Debug("Pull image", "image", imageName)
-			// image is not in local storage, so we need to pull it
-			if err = d.podman.ImagePull(d.ctx, imageName); err != nil {
-				return nil, nil, fmt.Errorf("failed to start task, unable to pull image %s : %v", imageName, err)
-			}
-		}
-		createOpts.Image = imageName
+		createOpts.Image = imageID
 
 		createResponse, err := d.podman.ContainerCreate(d.ctx, createOpts)
 		for _, w := range createResponse.Warnings {
@@ -608,35 +594,85 @@ func memoryInBytes(strmem string) (int64, error) {
 	}
 }
 
-func parseImage(image string) (string, error) {
-	// strip http/https and docker transport prefix
-	for _, prefix := range []string{"http://", "https://", "docker://"} {
-		if strings.HasPrefix(image, prefix) {
-			image = strings.Replace(image, prefix, "", 1)
+// Creates the requested image if missing from storage
+// returns the 64-byte image ID as an unique image identifier
+func (d *Driver) createImage(image string) (string, error) {
+	var imageID string
+	imageName := image
+	// If it is a shortname, we should not have to worry
+	// Let podman deal with it according to user configuration
+	if !shortnames.IsShortName(image) {
+		imageRef, err := parseImage(image)
+		if err != nil {
+			return imageID, fmt.Errorf("invalid image reference %s: %w\n", image, err)
 		}
+		// If its non-docker transport, we cannot ask for a pull or
+		// check for existance in the API without image plumbing
+		if imageRef.Transport().Name() != "docker" {
+			if imageRef.Transport().Name() != "oci-archive" &&
+				imageRef.Transport().Name() != "docker-archive" {
+				return imageID, fmt.Errorf("unssupported transport: %s", imageRef.Transport().Name())
+			}
+			// docker-archive:path[:docker-reference]
+			// oci-archive:path:tag
+			path := strings.Split(image, ":")[1]
+			d.logger.Debug("Load image archive", "path", path)
+			imageID, err = d.podman.ImageLoad(d.ctx, path)
+			if err != nil {
+				return imageID, fmt.Errorf("error while loading image: %w", err)
+			}
+			return imageID, nil
+		}
+		imageName = imageRef.DockerReference().String()
 	}
 
-	named, err := dockerref.ParseNormalizedNamed(image)
+	imageID, err := d.podman.ImageInspectID(d.ctx, imageName)
 	if err != nil {
-		return "", err
+		d.logger.Warn("Unable to check for local image", "image", imageName, "err", err)
+		// do NOT fail this operation, instead try to pull the
 	}
-
-	var tag, digest string
-
-	tagged, ok := named.(dockerref.Tagged)
-	if ok {
-		tag = tagged.Tag()
-		return fmt.Sprintf("%s:%s", named.Name(), tag), nil
+	if imageID != "" {
+		d.logger.Info("Found imageID", imageID, "for image", imageName, "in local storage")
+		return imageID, nil
 	}
 
-	digested, ok := named.(dockerref.Digested)
-	if ok {
-		digest = digested.Digest().String()
-		return fmt.Sprintf("%s@%s", named.Name(), digest), nil
+	d.logger.Debug("Pull image", "image", imageName)
+	if imageID, err = d.podman.ImagePull(d.ctx, imageName); err != nil {
+		return imageID, fmt.Errorf("failed to start task, unable to pull image %s : %w", imageName, err)
 	}
+	d.logger.Debug("Pulled image ID", imageID, "for image", imageName)
+	return imageID, nil
+}
 
-	// Image is neither diggested, nor tagged. Default to 'latest'
-	return fmt.Sprintf("%s:%s", named.Name(), "latest"), nil
+func parseImage(image string) (types.ImageReference, error) {
+	var transport, name string
+	parts := strings.SplitN(image, ":", 2)
+	// In case the transport is missing, assume docker://
+	if len(parts) == 1 {
+		transport = "docker"
+		name = "//" + image
+	} else {
+		transport = parts[0]
+		name = parts[1]
+	}
+
+	// 1: name
+	switch transport {
+	case "docker":
+		return docker.ParseReference(name)
+	case "oci-archive":
+		return ociArchive.ParseReference(name)
+	case "docker-archive":
+		return dockerArchive.ParseReference(name)
+	default:
+		// We could have both an unknown/malformed transport
+		// or an image:tag with default "docker://" transport ommited
+		ref, err := docker.ParseReference("//" + image)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing image %s: %w", image, err)
+		}
+		return ref, nil
+	}
 }
 
 // WaitTask function is expected to return a channel that will send an *ExitResult when the task