Skip to content

Replace platforms.Format with platforms.FormatAll in user-visible messages and logs. #49973

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.

Sign up for GitHub

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

Merged
vvoland merged 2 commits into from
May 21, 2025
Merged

Replace platforms.Format with platforms.FormatAll in user-visible messages and logs. #49973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf11cd1
Replace platforms.Format with platforms.FormatAll in user-visible mes…
ctalledo May 13, 2025
bb41e5a
Replace platforms.Format with platforms.FormatAll in functional code.
ctalledo May 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion builder/builder-next/adapters/containerimage/pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func (is *Source) resolveRemote(ctx context.Context, ref string, platform *ocisp
p = *platform
}
// key is used to synchronize resolutions that can happen in parallel when doing multi-stage.
key := "getconfig::" + ref + "::" + platforms.Format(p)
key := "getconfig::" + ref + "::" + platforms.FormatAll(p)
Comment on lines 184 to +185
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah May 16, 2025

Choose a reason for hiding this comment

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

Could use some extra eyes on this one if changing this one is OK to do

Copy link
Copy Markdown
Collaborator Author

@ctalledo ctalledo May 16, 2025

Choose a reason for hiding this comment

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

@tonistiigi: does this PR look OK (in particular the use of FormatAll() above).

res, err := is.g.Do(ctx, key, func(ctx context.Context) (*resolveRemoteResult, error) {
res := resolver.DefaultPool.GetResolver(is.RegistryHosts, ref, "pull", sm, g)
dgst, dt, err := imageutil.Config(ctx, ref, res, is.ContentStore, is.LeaseManager, platform)
Expand Down
2 changes: 1 addition & 1 deletion daemon/containerd/image_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (i *ImageService) pullForBuilder(ctx context.Context, name string, authConf
WARNING: Pulled image with specified platform (%s), but the resulting image's configured platform (%s) does not match.
This is most likely caused by a bug in the build system that created the fetched image (%s).
Please notify the image author to correct the configuration.`,
platforms.Format(p), platforms.Format(imgPlat), name,
platforms.FormatAll(p), platforms.FormatAll(imgPlat), name,
)
log.G(ctx).WithError(err).WithField("image", name).Warn("Ignoring error about platform mismatch where the manifest list points to an image whose configuration does not match the platform in the manifest.")
}
Expand Down
4 changes: 2 additions & 2 deletions daemon/containerd/image_pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (i *ImageService) PullImage(ctx context.Context, baseRef reference.Named, p
func (i *ImageService) pullTag(ctx context.Context, ref reference.Named, platform *ocispec.Platform, metaHeaders map[string][]string, authConfig *registrytypes.AuthConfig, out progress.Output) error {
var opts []containerd.RemoteOpt
if platform != nil {
opts = append(opts, containerd.WithPlatform(platforms.Format(*platform)))
opts = append(opts, containerd.WithPlatform(platforms.FormatAll(*platform)))
}

resolver, _ := i.newResolverFromAuthConfig(ctx, authConfig, ref)
Expand Down Expand Up @@ -223,7 +223,7 @@ func (i *ImageService) pullTag(ctx context.Context, ref reference.Named, platfor
if strings.Contains(err.Error(), "platform") {
platformStr := platforms.DefaultString()
if platform != nil {
platformStr = platforms.Format(*platform)
platformStr = platforms.FormatAll(*platform)
}
return errdefs.NotFound(fmt.Errorf("no matching manifest for %s in the manifest list entries: %w", platformStr, err))
}
Expand Down
2 changes: 1 addition & 1 deletion daemon/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (daemon *Daemon) containerCreate(ctx context.Context, daemonCfg *configStor
}

if !images.OnlyPlatformWithFallback(p).Match(imgPlat) {
warnings = append(warnings, fmt.Sprintf("The requested image's platform (%s) does not match the detected host platform (%s) and no specific platform was requested", platforms.Format(imgPlat), platforms.Format(p)))
warnings = append(warnings, fmt.Sprintf("The requested image's platform (%s) does not match the detected host platform (%s) and no specific platform was requested", platforms.FormatAll(imgPlat), platforms.FormatAll(p)))
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions daemon/images/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (i *ImageService) manifestMatchesPlatform(ctx context.Context, img *image.I
log.G(ctx).WithFields(log.Fields{
"error": err,
"image": img.ID,
"desiredPlatform": platforms.Format(platform),
"desiredPlatform": platforms.FormatAll(platform),
}).Error("Error looking up image leases")
return false, err
}
Expand All @@ -75,7 +75,7 @@ func (i *ImageService) manifestMatchesPlatform(ctx context.Context, img *image.I
for _, r := range ls {
logger := log.G(ctx).WithFields(log.Fields{
"image": img.ID,
"desiredPlatform": platforms.Format(platform),
"desiredPlatform": platforms.FormatAll(platform),
"resourceID": r.ID,
"resourceType": r.Type,
})
Expand Down Expand Up @@ -121,7 +121,7 @@ func (i *ImageService) manifestMatchesPlatform(ctx context.Context, img *image.I
Variant: md.Platform.Variant,
}
if !comparer.Match(p) {
logger.WithField("otherPlatform", platforms.Format(p)).Debug("Manifest is not a match")
logger.WithField("otherPlatform", platforms.FormatAll(p)).Debug("Manifest is not a match")
continue
}

Expand Down Expand Up @@ -195,7 +195,7 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options bac
if ref, err := reference.ParseNamed(refOrID); err == nil {
imgName = reference.FamiliarString(ref)
}
retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but its platform (%s) does not match the specified platform (%s)", imgName, platforms.Format(imgPlat), platforms.Format(p)))
retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but its platform (%s) does not match the specified platform (%s)", imgName, platforms.FormatAll(imgPlat), platforms.FormatAll(p)))
}()
ref, err := reference.ParseAnyReference(refOrID)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion daemon/images/image_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (i *ImageService) pullForBuilder(ctx context.Context, name string, authConf
WARNING: Pulled image with specified platform (%s), but the resulting image's configured platform (%s) does not match.
This is most likely caused by a bug in the build system that created the fetched image (%s).
Please notify the image author to correct the configuration.`,
platforms.Format(p), platforms.Format(imgPlat), name,
platforms.FormatAll(p), platforms.FormatAll(imgPlat), name,
)
log.G(ctx).WithError(err).WithField("image", name).Warn("Ignoring error about platform mismatch where the manifest list points to an image whose configuration does not match the platform in the manifest.")
err = nil
Expand Down
2 changes: 1 addition & 1 deletion distribution/pull_v2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,7 @@ func (p *puller) pullManifestList(ctx context.Context, ref reference.Named, mfst
if pp != nil {
platform = *pp
}
log.G(ctx).Debugf("%s resolved to a manifestList object with %d entries; looking for a %s match", ref, len(mfstList.Manifests), platforms.Format(platform))
log.G(ctx).Debugf("%s resolved to a manifestList object with %d entries; looking for a %s match", ref, len(mfstList.Manifests), platforms.FormatAll(platform))
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah May 14, 2025

Choose a reason for hiding this comment

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

Not for this PR, but we should use structured logs more for entries like these; ideally we would review the names we used for fields in those logs (as I think we're not always consistent); having more structured logs allows easier filtering / querying of logs (more so if they carry consistent fields, which allows correlating logs).


manifestMatches := filterManifests(mfstList.Manifests, platform)

Expand Down
2 changes: 1 addition & 1 deletion distribution/pull_v2_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func filterManifests(manifests []manifestlist.ManifestDescriptor, p ocispec.Plat
if descP == nil || m.Match(*descP) {
matches = append(matches, desc)
if descP != nil {
log.G(context.TODO()).Debugf("found match for %s with media type %s, digest %s", platforms.Format(p), desc.MediaType, desc.Digest.String())
log.G(context.TODO()).Debugf("found match for %s with media type %s, digest %s", platforms.FormatAll(p), desc.MediaType, desc.Digest.String())
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/testutils/specialimage/multiplatform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func MultiPlatform(dir string, imageRef string, imagePlatforms []ocispec.Platfor
var descs []ocispec.Descriptor

for _, platform := range imagePlatforms {
ps := platforms.Format(platform)
ps := platforms.FormatAll(platform)
manifestDesc, _, err := oneLayerPlatformManifest(dir, platform, FileInLayer{Path: "bash", Content: []byte("layer-" + ps)})
if err != nil {
return nil, nil, err
Expand Down
2 changes: 1 addition & 1 deletion internal/testutils/specialimage/partial.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func PartialMultiPlatform(dir string, imageRef string, opts PartialOpts) (*ocisp
var descs []ocispec.Descriptor

for _, platform := range opts.Stored {
ps := platforms.Format(platform)
ps := platforms.FormatAll(platform)
manifestDesc, _, err := oneLayerPlatformManifest(dir, platform, FileInLayer{Path: "bash", Content: []byte("layer-" + ps)})
if err != nil {
return nil, nil, err
Expand Down
Loading