Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set tag when pulling image missing in local storage #96

Merged
merged 4 commits into from
Apr 5, 2021
Merged

Set tag when pulling image missing in local storage #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
34e6521
Set tag when pulling non-existent images and test to avoid regression
pabloyoyoista Apr 4, 2021
4d96dec
Refactor parseImage to get whole Image name as in podman
pabloyoyoista Apr 4, 2021
91a769c
Trigger GH workflow also in pull requests
pabloyoyoista Apr 5, 2021
f6e2ee0
Fix test for digest to work as expected
pabloyoyoista Apr 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: build

on: [push]
on: [push, pull_request]

jobs:
build:
Expand Down
20 changes: 11 additions & 9 deletions driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,13 +488,13 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
// e.g. oci-archive:/... etc
// see also https://github.com/hashicorp/nomad-driver-podman/issues/69

imageName, tag, err := parseImage(createOpts.Image)
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, fmt.Sprintf("%s:%s", imageName, tag))
haveImage, err := d.podman.ImageExists(d.ctx, imageName)
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
Expand All @@ -504,9 +504,10 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
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)
return nil, nil, fmt.Errorf("failed to start task, unable to pull image %s : %v", imageName, err)
}
}
createOpts.Image = imageName
towe75 marked this conversation as resolved.
Show resolved Hide resolved

createResponse, err := d.podman.ContainerCreate(d.ctx, createOpts)
for _, w := range createResponse.Warnings {
Expand Down Expand Up @@ -607,7 +608,7 @@ func memoryInBytes(strmem string) (int64, error) {
}
}

func parseImage(image string) (string, string, 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) {
Expand All @@ -617,24 +618,25 @@ func parseImage(image string) (string, string, error) {

named, err := dockerref.ParseNormalizedNamed(image)
if err != nil {
return "", "", nil
return "", err
}

var tag, digest string

tagged, ok := named.(dockerref.Tagged)
if ok {
tag = tagged.Tag()
return fmt.Sprintf("%s:%s", named.Name(), tag), nil
}

digested, ok := named.(dockerref.Digested)
if ok {
digest = digested.Digest().String()
return fmt.Sprintf("%s@%s", named.Name(), digest), nil
}
if len(tag) == 0 && len(digest) == 0 {
tag = "latest"
}
return named.Name(), tag, nil

// Image is neither diggested, nor tagged. Default to 'latest'
return fmt.Sprintf("%s:%s", named.Name(), "latest"), nil
}

// WaitTask function is expected to return a channel that will send an *ExitResult when the task
Expand Down
70 changes: 50 additions & 20 deletions driver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1357,34 +1357,64 @@ func TestPodmanDriver_Sysctl(t *testing.T) {

}

// Make sure we can pull and start "non-latest" containers
func TestPodmanDriver_Pull(t *testing.T) {
if !tu.IsCI() {
t.Parallel()
}

testCases := []struct {
Image string
TaskName string
}{
{Image: "busybox:unstable", TaskName: "pull_tag"},
{Image: "busybox", TaskName: "pull_non_tag"},
{Image: "busybox@sha256:ce98b632acbcbdf8d6fdc50d5f91fea39c770cd5b3a2724f52551dde4d088e96", TaskName: "pull_digest"},
}

for _, testCase := range testCases {
startDestroyInspectImage(t, testCase.Image, testCase.TaskName)
}
}

func startDestroyInspectImage(t *testing.T, image string, taskName string) {
taskCfg := newTaskConfig(image, busyboxLongRunningCmd)
inspectData := startDestroyInspect(t, taskCfg, taskName)

parsedImage, err := parseImage(image)
require.NoError(t, err)
require.Equal(t, parsedImage, inspectData.Config.Image)
}

func Test_parseImage(t *testing.T) {
if !tu.IsCI() {
t.Parallel()
}

digest := "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
testCases := []struct {
Input string
Repo string
Tag string
Image string
}{
{Input: "quay.io/prometheus/busybox:glibc", Repo: "quay.io/prometheus/busybox", Tag: "glibc"},
{Input: "root", Repo: "docker.io/library/root", Tag: "latest"},
{Input: "root:tag", Repo: "docker.io/library/root", Tag: "tag"},
{Input: "docker://root", Repo: "docker.io/library/root", Tag: "latest"},
{Input: "user/repo", Repo: "docker.io/user/repo", Tag: "latest"},
{Input: "https://busybox", Repo: "docker.io/library/busybox", Tag: "latest"},
{Input: "user/repo:tag", Repo: "docker.io/user/repo", Tag: "tag"},
{Input: "url:5000/repo", Repo: "url:5000/repo", Tag: "latest"},
{Input: "http://busybox", Repo: "docker.io/library/busybox", Tag: "latest"},
{Input: "url:5000/repo:tag", Repo: "url:5000/repo", Tag: "tag"},
{Input: "https://quay.io/busybox", Repo: "quay.io/busybox", Tag: "latest"},
{Input: "quay.io/prometheus/busybox:glibc", Image: "quay.io/prometheus/busybox:glibc"},
{Input: "root", Image: "docker.io/library/root:latest"},
{Input: "root:tag", Image: "docker.io/library/root:tag"},
{Input: "root@" + digest, Image: "docker.io/library/root@" + digest},
{Input: "docker://root", Image: "docker.io/library/root:latest"},
{Input: "user/repo", Image: "docker.io/user/repo:latest"},
{Input: "https://busybox", Image: "docker.io/library/busybox:latest"},
{Input: "user/repo:tag", Image: "docker.io/user/repo:tag"},
{Input: "url:5000/repo", Image: "url:5000/repo:latest"},
{Input: "http://busybox@" + digest, Image: "docker.io/library/busybox@" + digest},
{Input: "url:5000/repo:tag", Image: "url:5000/repo:tag"},
{Input: "https://quay.io/busybox", Image: "quay.io/busybox:latest"},
}
for _, testCase := range testCases {
repo, tag, err := parseImage(testCase.Input)
image, err := parseImage(testCase.Input)
if err != nil {
t.Errorf("parseImage(%s) failed: %v", testCase.Input, err)
} else if repo != testCase.Repo || tag != testCase.Tag {
t.Errorf("Expected repo: %q, tag: %q, but got %q and %q", testCase.Repo, testCase.Tag, repo, tag)
} else if image != testCase.Image {
t.Errorf("Expected image: %q, but got %q", testCase.Image, image)
}
}
}
Expand All @@ -1398,10 +1428,10 @@ func readLogfile(t *testing.T, task *drivers.TaskConfig) string {
return string(stdout)
}

func newTaskConfig(variant string, command []string) TaskConfig {
busyboxImageID := "docker://docker.io/library/busybox:latest"

image := busyboxImageID
func newTaskConfig(image string, command []string) TaskConfig {
if len(image) == 0 {
image = "docker://docker.io/library/busybox:latest"
}

return TaskConfig{
Image: image,
Expand Down