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.

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

Document docker transport is the only supported remote transport #8465

Merged
merged 1 commit into from
Nov 30, 2020

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Nov 24, 2020

The goal is to improve errors when users use the wrong transport
in certain cases we stutter, in other cases we don't give enough
information.

Remove stutters when failing to pull remote images, because of
lack of support.

Fix errors returned by reference.Parse to wrap in image that was being
checked.

Fixes: #7116

Signed-off-by: Daniel J Walsh [email protected]

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 24, 2020
@@ -22,6 +22,28 @@ Default settings for flags are defined in `containers.conf`. Most settings for
remote connections use the server's containers.conf, except when documented in
man pages.

## IMAGE

The `image` uses a "transport":"details" format.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The image is specified using transport:path format. If no transport is specified, the docker transport will be used by default. For remote Podman, docker is the only allowed transport."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

An image in a registry implementing the "Docker Registry HTTP API V2". By default, uses the authorization state in `$XDG_RUNTIME_DIR/containers/auth.json`, which is set using `(podman login)`. If the authorization state is not found there, `$HOME/.docker/config.json` is checked, which is set using `(docker login)`.

**docker-archive:**_path_[**:**_docker-reference_]
An image is stored in the `docker save` formatted file. _docker-reference_ is only used when creating such a file, and it must not contain a digest.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Accepts images in archives created by the podman create command."

Also, please explain docker-reference better - I have no idea when and how I should be using it from this description.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an example.

An existing local directory _path_ storing the manifest, layer tarballs and signatures as individual files. This is a non-standardized format, primarily useful for debugging or noninvasive container inspection.

**docker://**_docker-reference_
An image in a registry implementing the "Docker Registry HTTP API V2". By default, uses the authorization state in `$XDG_RUNTIME_DIR/containers/auth.json`, which is set using `(podman login)`. If the authorization state is not found there, `$HOME/.docker/config.json` is checked, which is set using `(docker login)`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Accepts a reference to an image in a remote registry. The reference can include a path to a specific registry; if it does not, the registries listed in registries.conf will be queried to find a matching image. By default, credentials from podman login (stored at $XDG_RUNTIME_DIR/containers/auth.json by default) will be used to authenticate; if these cannot be found, we will fall back to using credentials in $HOME/.docker/config.json.

transport is allowed for remote access.

**dir:**_path_
An existing local directory _path_ storing the manifest, layer tarballs and signatures as individual files. This is a non-standardized format, primarily useful for debugging or noninvasive container inspection.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I make a directory like this? Providing a command would greatly help understanding.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

podman push DIR
Would create this or podman pull DIR.

An image _docker-reference_ stored in the docker daemon internal storage. _docker-reference_ must contain either a tag or a digest. Alternatively, when reading images, the format can also be docker-daemon:algo:digest (an image ID).

**oci-archive:**_path_**:**_tag_
An image _tag_ in a directory compliant with "Open Container Image Layout Specification" at _path_.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I make such an archive?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Podman pull or podman push, podman save.
podman save IMAGE --format docker-dir

@@ -22,6 +22,28 @@ Default settings for flags are defined in `containers.conf`. Most settings for
remote connections use the server's containers.conf, except when documented in
man pages.

## IMAGE

The `image` uses a "transport":"details" format.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `image` uses a "transport":"details" format.
The specified `image` uses a "transport":"details" format.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rewritten.

An image is stored in the `docker save` formatted file. _docker-reference_ is only used when creating such a file, and it must not contain a digest.

**docker-daemon:**_docker-reference_
An image _docker-reference_ stored in the docker daemon internal storage. _docker-reference_ must contain either a tag or a digest. Alternatively, when reading images, the format can also be docker-daemon:algo:digest (an image ID).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last two sentences seem to be redundant - the format can be a tag, or a digest, or a digest?

@@ -22,6 +22,28 @@ Default settings for flags are defined in `containers.conf`. Most settings for
remote connections use the server's containers.conf, except when documented in
man pages.

## IMAGE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything in this section needs examples - it's extremely unclear how any of this is used in practice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we need to make it clear that some of these transports (I think not all?) do not pull the image if it is present locally in c/storage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added examples.

@@ -27,7 +27,8 @@ Images are stored in local image storage.
## SOURCE

The SOURCE is the location from which the container images are pulled.
The Image "SOURCE" uses a "transport":"details" format.
The Image "SOURCE" uses a "transport":"details" format. Only the `docker` (container registry)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably split what was written for podman run and podman create out into a separate manpage and have all of these reference that page.

@@ -423,7 +423,7 @@ func PushImage(w http.ResponseWriter, r *http.Request) {
source := strings.TrimSuffix(utils.GetName(r), "/push") // GetName returns the entire path
if _, err := utils.ParseStorageReference(source); err != nil {
utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
errors.Wrapf(err, "image source %q is not a containers-storage-transport reference", source))
err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't have the Wrapf this can go on the previous line

@@ -434,7 +434,7 @@ func PushImage(w http.ResponseWriter, r *http.Request) {

if _, err := utils.ParseDockerReference(destination); err != nil {
utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
errors.Wrapf(err, "image destination %q is not a docker-transport reference", destination))
err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to previous line

@@ -54,7 +54,7 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) {
imageRef, err := utils.ParseDockerReference(query.Reference)
if err != nil {
utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
errors.Wrapf(err, "image destination %q is not a docker-transport reference", query.Reference))
err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous line

An existing local directory _path_ storing the manifest, layer tarballs and signatures as individual files. This is a non-standardized format, primarily useful for debugging or noninvasive container inspection.

**docker://**_docker-reference_
An image in a registry implementing the "Docker Registry HTTP API V2". By default, uses the authorization state in `$XDG_RUNTIME_DIR/containers/auth.json`, which is set using `(podman login)`. If the authorization state is not found there, `$HOME/.docker/config.json` is checked, which is set using `(docker login)`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An image in a registry implementing the "Docker Registry HTTP API V2". By default, uses the authorization state in `$XDG_RUNTIME_DIR/containers/auth.json`, which is set using `(podman login)`. If the authorization state is not found there, `$HOME/.docker/config.json` is checked, which is set using `(docker login)`.
An image in a registry implementing the "Docker Registry HTTP API V2" format. By default, uses the authorization state in `$XDG_RUNTIME_DIR/containers/auth.json`, which is set using `(podman login)`. If the authorization state is not found there, `$HOME/.docker/config.json` is checked, which is set using `(docker login)`.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait to review rewrite by @mheon

An image is stored in the `docker save` formatted file. _docker-reference_ is only used when creating such a file, and it must not contain a digest.

**docker-daemon:**_docker-reference_
An image _docker-reference_ stored in the docker daemon internal storage. _docker-reference_ must contain either a tag or a digest. Alternatively, when reading images, the format can also be docker-daemon:algo:digest (an image ID).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An image _docker-reference_ stored in the docker daemon internal storage. _docker-reference_ must contain either a tag or a digest. Alternatively, when reading images, the format can also be docker-daemon:algo:digest (an image ID).
An image in _docker-reference_ format stored in the docker daemon internal storage. _docker-reference_ must contain either a tag or a digest. Alternatively, when reading images, the format can also be docker-daemon:algo:digest (an image ID).

An image _docker-reference_ stored in the docker daemon internal storage. _docker-reference_ must contain either a tag or a digest. Alternatively, when reading images, the format can also be docker-daemon:algo:digest (an image ID).

**oci-archive:**_path_**:**_tag_
An image _tag_ in a directory compliant with "Open Container Image Layout Specification" at _path_.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An image _tag_ in a directory compliant with "Open Container Image Layout Specification" at _path_.
An image in a directory compliant with the "Open Container Image Layout Specification" at the specified _path_ and specified with a _tag_.

Do we have more info io the OCI Layout Spec that we should/can point them to?


**oci-archive:**_path_**:**_tag_
An image _tag_ in a directory compliant with "Open Container Image Layout Specification" at _path_.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as prior comments

@rhatdan rhatdan force-pushed the pull branch 2 times, most recently from 7cead85 to 1ce978f Compare November 24, 2020 18:12
will fall back to using credentials in $HOME/.docker/config.json.

**docker-archive:**_path_[**:**_docker-reference_]
An image stored in the `docker save` formatted file. _docker-reference_ is only used when creating such a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still really unclear about docker-reference - if it's only used when it created, why do we want it here when we're only pulling/using such an image?

@rhatdan
Copy link
Member Author

rhatdan commented Nov 24, 2020

@mheon @TomSweeneyRedHat PTA Fresh Look.

An image reference stored in a remote container image registry. Example: "quay.io/podman/stable:latest".
The reference can include a path to a specific registry; if it does not, the registries listed in
registries.conf will be queried to find a matching image. By default, credentials from podman login (stored at
$XDG_RUNTIME_DIR/containers/auth.json by default) will be used to authenticate; if these cannot be found, we
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$XDG_RUNTIME_DIR/containers/auth.json by default) will be used to authenticate; if these cannot be found, we
$XDG_RUNTIME_DIR/containers/auth.json by default) will be used to authenticate; if these cannot be found,

The reference can include a path to a specific registry; if it does not, the registries listed in
registries.conf will be queried to find a matching image. By default, credentials from podman login (stored at
$XDG_RUNTIME_DIR/containers/auth.json by default) will be used to authenticate; if these cannot be found, we
will fall back to using credentials in $HOME/.docker/config.json.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
will fall back to using credentials in $HOME/.docker/config.json.
credentials in $HOME/.docker/config.json will be used if available.


**docker-daemon:**_docker-reference_
An image in _docker-reference_ format stored in the docker daemon internal storage. _docker-reference_ must
contain either a tag. Alternatively, when reading images, the format can also be docker-daemon:algo:digest (an
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what to do with "either a tag"? At the very least an "or" is missing or perhaps it's a really bad cut/paste?


**docker-daemon:**_docker-reference_
An image _docker-reference_ stored in the docker daemon internal storage. _docker-reference_ must contain either a tag or a digest. Alternatively, when reading images, the format can also be docker-daemon:algo:digest (an image ID).
An image _docker-reference_ stored in the docker daemon internal storage. _docker-reference_ must contain either
a tag. Alternatively, when reading images, the format can also be docker-daemon:algo:digest (an image ID).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comments as previous.

The goal is to improve errors when users use the wrong transport
in certain cases we stutter, in other cases we don't give enough
information.

Remove stutters when failing to pull remote images, because of
lack of support.

Fix errors returned by reference.Parse to wrap in image that was being
checked.

Fixes: containers#7116

Signed-off-by: Daniel J Walsh <[email protected]>
@rhatdan
Copy link
Member Author

rhatdan commented Nov 29, 2020

@mheon @TomSweeneyRedHat PTAL again. Would be nice to get this in to 2.2 release.

## IMAGE

The image is specified using transport:path format. If no transport is specified, the `docker` (container registry)
transport will be used by default. For remote Podman, `docker` is the only allowed transport."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra " at end

@mheon
Copy link
Member

mheon commented Nov 30, 2020

One nit then LGTM

@mheon
Copy link
Member

mheon commented Nov 30, 2020

Merging as is, I will submit a fix PR later.

@mheon
Copy link
Member

mheon commented Nov 30, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2020
@openshift-merge-robot openshift-merge-robot merged commit e2c406f into containers:master Nov 30, 2020
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman-remote: run docker-archive: ... must be a docker reference
5 participants