Make types.Image Docker-independent, add docker.GenericImageFromSource#79
Merged
mtrmac merged 7 commits intocontainers:masterfrom May 30, 2016
Merged
Conversation
We abort on failure to get the data anyway, so there is no need to use temporaries to avoid modifying outputData on failure. This is not a simplification yet, but handling optional (e.g. Docker-specific) data this way will be simpler, and handling non-optional data the same way will be more consistent.
This does not change the code at all, only moving things around now.
This is the only Docker-specific aspect of types.Image.Inspect. This does not change behavior; plausibly we might want to replace the Name value in (skopeo inspect) by something else which is not dependent on Docker, but that can be a separate work later. Adds a FIXME? in docker_image.go for consistency with dockerImage.GetRepositoryTags, both will be removed later in the patchset.
The code not dependent on specifics of DockerImageSource now lives in docker.genericImage; the rest directly in docker.Image. docker.Image remains the only implementation of types.Image at this point, but that will change.
This finally makes genericImage Docker-independent. (dockerImage is still the only implementation of types.Image.)
The remaining uses of the dependencies, in (skopeo inspect), now check whether their types.Image is a docker.Image and call the docker.Image functions directly. This does not change behavior for Docker images. For non-Docker images (which can't happen yet), the Name field is removed; RepoTags remain and are reported as empty, because using json:",omitempty" would also omit an empty list for Docker images.
This is not expected to be that useful in production; for now it serves as a demonstration of the concept, and it allows (skopeo inspect) to be clumsily used as parser of stand-alone manifests (by creating a dir: structure with that manifest). (skopeo layers) support follows naturally, but is even less useful.
Member
|
👍 fine to me for now, feel free to merge it! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following up on the discussions in #52 and #58 , this allows using
types.Imagefor non-Dockertypes.ImageSourcebackends, instead moving the Docker-specific knowledge intodocker.Imageand theskopeo inspectimplementation.This will be useful for policy evaluation unit tests, by allowing testing against a
dir:…types.Imagewithout having to construct testing-only mocks and reimplementImagefunctionality likeMatchesManifestDigest(#75) in the mocks.As is, this is not a complete API cleanup (e.g. having the generic Image constructor as
docker.GenericImageFromSourceis clearly not the right thing to do) but it does move us closer to the desired end state, at least in the implementation if not in the external API.See the individual commits for detailed comments.