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

Fix hosts.toml resolution for docker domain #3720

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

apostasie
Copy link
Contributor

Fix #3697

@apostasie apostasie marked this pull request as ready for review December 2, 2024 23:51
@apostasie apostasie marked this pull request as draft December 2, 2024 23:59
@apostasie apostasie marked this pull request as ready for review December 3, 2024 00:18
@AkihiroSuda AkihiroSuda added this to the v2.0.2 (tentative) milestone Dec 3, 2024
@@ -82,6 +82,13 @@ func NewHostOptions(ctx context.Context, refHostname string, optFuncs ...Opt) (*

ho.HostDir = func(hostURL string) (string, error) {
regURL, err := Parse(hostURL)
// Docker inconsistencies handling: `index.docker.io` actually expects `docker.io` for hosts.toml on the filesystem
// See https://github.com/containerd/nerdctl/issues/3697
// FIXME: we need to reevaluate this comparing with what docker does. What should happen for FQ images with alternate docker domains? (eg: registry-1.docker.io)
Copy link
Member

Choose a reason for hiding this comment

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

What is FQ?

Copy link
Contributor Author

@apostasie apostasie Dec 3, 2024

Choose a reason for hiding this comment

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

Fully qualified (eg: registry-1.docker.io/library/alpine).
Acronyms are stupid. Will avoid in the future.

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 051b047 into containerd:main Dec 3, 2024
30 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.

nerdctl 2.0.0 fails to use configuration acceleration
2 participants