Fixed a bug where we always pulled an image if image name had a transport prefix #88
Conversation
Images prefixed with a transport where always pulled. Added image url parser
|
@krishicks : please review, this also fixes #90 when using podman 2.x |
driver.go
Outdated
| // 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 | ||
|
|
||
| // 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) | ||
| } | ||
| } | ||
|
|
||
| named, err := dockerref.ParseNormalizedNamed(image) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| var tag, digest string | ||
|
|
||
| tagged, ok := named.(dockerref.Tagged) | ||
| if ok { | ||
| tag = tagged.Tag() | ||
| } | ||
|
|
||
| digested, ok := named.(dockerref.Digested) | ||
| if ok { | ||
| digest = digested.Digest().String() | ||
| } | ||
| if len(tag) == 0 && len(digest) == 0 { | ||
| tag = "latest" | ||
| } |
There was a problem hiding this comment.
It might be nice to extract all of this into its own function, since most of this function exists to manipulate a given image. That could then have its own set of tests to make it clear what the expected output for a given input are.
At that point I'd remove the ensureImage function entirely, as it would be about the same length as it was before.
There was a problem hiding this comment.
@krishicks PTAL, i added a separate, tested, function. I think it's ok to also keep ensureImage() because of possible, future, improvements regarding oci-archive etc.
There was a problem hiding this comment.
I like the new tested function. I would still remove ensureImage, as any future change that warrants bringing it back can do so at that time. Otherwise, it's YAGNI.
My approval still applies, in any event.
Images prefixed with a transport where always pulled.
I noticed this while examining podman logs during unit test runs. It re-pulled busybox for each single test.
This PR improves image url parsing and also makes the code a bit nicer by adding an extra method for image handling.