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

registry: remove ping v1 #2588

Merged
merged 1 commit into from
Oct 2, 2024
Merged

registry: remove ping v1 #2588

merged 1 commit into from
Oct 2, 2024

Conversation

carlosrodfern
Copy link
Contributor

The ping v1 happens when the ping v2 fails, however, it causes the ping v2 error to be skipped and not output to the user. As result, when a registry has v1 and v2 enabled, and there are, for example, intermittent connectivity issues making the ping v2 fail, the user is presented with a misleading error saying "can't talk to V1 registry."

Since the only use of v1 is for the search API as a workaround for docker.io, and new container registries setups are very unlikely to be v1-only, there is little utility in keeping this v1 detection in the attempt to help the user realize their setup is v1-only, hence not compatible. On the contratry, it just presents the user with a misleading error in certain circumstances.

@cgwalters
Copy link
Contributor

/approve

Makes sense to me...

Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

Thanks!

This makes sense to me, but we must not break API.

docker/errors.go Show resolved Hide resolved
types/types.go Show resolved Hide resolved
docker/docker_client.go Show resolved Hide resolved
docker/docker_client.go Show resolved Hide resolved
@carlosrodfern
Copy link
Contributor Author

Thank you for the feedback. I believe I addressed all the comments. Let me know if it needs anything else 👍

Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

Thanks again!

The ping v1 happens when the ping v2 fails, however, it causes the ping
v2 error to be skipped and not output to the user. As result, when a
registry has v1 and v2 enabled, and there are, for example, intermittent
connectivity issues making the ping v2 fail, the user is presented with
a misleading error saying "can't talk to V1 registry."

Since the only use of v1 is for the search API as a workaround for
docker.io, and new container registries setups are very unlikely to be
v1-only, there is little utility in keeping this v1 detection in the
attempt to help the user realize their setup is v1-only, hence not
compatible. On the contratry, it just presents the user with a
misleading error in certain circumstances.

Signed-off-by: Carlos Rodriguez-Fernandez <[email protected]>
@mtrmac mtrmac merged commit 687363a into containers:main Oct 2, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants