-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
driver/docker: pull image with digest #4298
Changes from 1 commit
e77d8a5
ce63b79
4492333
f4670f6
ba2d957
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1500,18 +1500,15 @@ func (d *DockerDriver) Periodic() (bool, time.Duration) { | |
// loading it from the file system | ||
func (d *DockerDriver) createImage(driverConfig *DockerDriverConfig, client *docker.Client, taskDir *allocdir.TaskDir) (string, error) { | ||
image := driverConfig.ImageName | ||
repo, tag := docker.ParseRepositoryTag(image) | ||
if tag == "" { | ||
tag = "latest" | ||
} | ||
repo, tag := parseDockerImage(image) | ||
|
||
coordinator, callerID := d.getDockerCoordinator(client) | ||
|
||
// We're going to check whether the image is already downloaded. If the tag | ||
// is "latest", or ForcePull is set, we have to check for a new version every time so we don't | ||
// bother to check and cache the id here. We'll download first, then cache. | ||
if driverConfig.ForcePull { | ||
d.logger.Printf("[DEBUG] driver.docker: force pull image '%s:%s' instead of inspecting local", repo, tag) | ||
d.logger.Printf("[DEBUG] driver.docker: force pull image '%s' instead of inspecting local", dockerImageRef(repo, tag)) | ||
} else if tag != "latest" { | ||
if dockerImage, _ := client.InspectImage(image); dockerImage != nil { | ||
// Image exists so just increment its reference count | ||
|
@@ -1544,7 +1541,7 @@ func (d *DockerDriver) pullImage(driverConfig *DockerDriverConfig, client *docke | |
d.logger.Printf("[DEBUG] driver.docker: did not find docker auth for repo %q", repo) | ||
} | ||
|
||
d.emitEvent("Downloading image %s:%s", repo, tag) | ||
d.emitEvent("Downloading image %s", dockerImageRef(repo, tag)) | ||
coordinator, callerID := d.getDockerCoordinator(client) | ||
|
||
return coordinator.PullImage(driverConfig.ImageName, authOptions, callerID, d.emitEvent) | ||
|
@@ -2183,3 +2180,24 @@ type createContainerClient interface { | |
ListContainers(docker.ListContainersOptions) ([]docker.APIContainers, error) | ||
RemoveContainer(opts docker.RemoveContainerOptions) error | ||
} | ||
|
||
func parseDockerImage(image string) (repo, tag string) { | ||
repo, tag = docker.ParseRepositoryTag(image) | ||
if tag == "" { | ||
if i := strings.IndexRune(image, '@'); i > -1 { // Has digest (@sha256:...) | ||
// when pulling images with a digest, the repository contains the sha hash, and the tag is empty | ||
// see: https://github.com/fsouza/go-dockerclient/blob/master/image_test.go#L471 | ||
repo = image | ||
} else { | ||
tag = "latest" | ||
} | ||
} | ||
return | ||
} | ||
|
||
func dockerImageRef(repo string, tag string) string { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you mind adding a quick test for this. Could even just be a line in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
if tag == "" { | ||
return repo | ||
} | ||
return fmt.Sprintf("%s:%s", repo, tag) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1092,6 +1092,30 @@ func TestDockerDriver_ForcePull(t *testing.T) { | |
} | ||
} | ||
|
||
func TestDockerDriver_ForcePull_RepoDigest(t *testing.T) { | ||
if !tu.IsTravis() { | ||
t.Parallel() | ||
} | ||
if !testutil.DockerIsConnected(t) { | ||
t.Skip("Docker not connected") | ||
} | ||
|
||
task, _, _ := dockerTask(t) | ||
task.Config["load"] = "" | ||
task.Config["image"] = "library/busybox@sha256:58ac43b2cc92c687a32c8be6278e50a063579655fe3090125dcb2af0ff9e1a64" | ||
task.Config["force_pull"] = "true" | ||
|
||
client, handle, cleanup := dockerSetup(t, task) | ||
defer cleanup() | ||
|
||
waitForExist(t, client, handle) | ||
|
||
_, err := client.InspectContainer(handle.ContainerID()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check that the inspected container's image is whats expected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can do this, but the remote digest won't match the local digest due to "reasons" so I'll have to just hard-code in another id for the local digest to match against. Not ideal |
||
if err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
t.Fatalf("err: %v", err) | ||
} | ||
} | ||
|
||
func TestDockerDriver_SecurityOpt(t *testing.T) { | ||
if !tu.IsTravis() { | ||
t.Parallel() | ||
|
@@ -2458,3 +2482,27 @@ func TestDockerDriver_AdvertiseIPv6Address(t *testing.T) { | |
t.Fatalf("Got GlobalIPv6address %s want GlobalIPv6address with prefix %s", expectedPrefix, container.NetworkSettings.GlobalIPv6Address) | ||
} | ||
} | ||
|
||
func TestParseDockerImage(t *testing.T) { | ||
tests := []struct { | ||
Image string | ||
Repo string | ||
Tag string | ||
}{ | ||
{"library/hello-world:1.0", "library/hello-world", "1.0"}, | ||
{"library/hello-world", "library/hello-world", "latest"}, | ||
{"library/hello-world:latest", "library/hello-world", "latest"}, | ||
{"library/hello-world@sha256:f5233545e43561214ca4891fd1157e1c3c563316ed8e237750d59bde73361e77", "library/hello-world@sha256:f5233545e43561214ca4891fd1157e1c3c563316ed8e237750d59bde73361e77", ""}, | ||
} | ||
for _, test := range tests { | ||
t.Run(test.Image, func(t *testing.T) { | ||
repo, tag := parseDockerImage(test.Image) | ||
if repo != test.Repo { | ||
t.Errorf("expected repo '%s' but got '%s'", test.Repo, repo) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you change these to use the |
||
} | ||
if repo != test.Repo { | ||
t.Errorf("expected tag '%s' but got '%s'", test.Tag, tag) | ||
} | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if tag != "" { return repo, tag }
reduce indentation and make it explicit whats returned :)