-
Notifications
You must be signed in to change notification settings - Fork 615
imagetools: metadata-file flag #3638
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
base: master
Are you sure you want to change the base?
Conversation
e6927ef to
6c3c4c7
Compare
thaJeztah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM
|
Oh! Tests are failing though, and could be related? |
6c3c4c7 to
f5568d2
Compare
tests/imagetools.go
Outdated
| t.Logf("digest: %s", outputDigest) | ||
| _, err = digest.Parse(outputDigest) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was the t.Logf for debugging? (I always try to avoid logs in tests, and instead only log on failure; e.g.;
| t.Logf("digest: %s", outputDigest) | |
| _, err = digest.Parse(outputDigest) | |
| require.NoError(t, err) | |
| _, err = digest.Parse(outputDigest) | |
| require.NoError(t, err, "digest: %s", outputDigest) |
|
I think I prefer |
f5568d2 to
1db71a1
Compare
Should be good now |
commands/imagetools/create.go
Outdated
|
|
||
| if err == nil && len(in.metadataFile) > 0 { | ||
| if err := writeMetadataFile(in.metadataFile, map[string]any{ | ||
| exptypes.ExporterImageDigestKey: desc.Digest.String(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe already better to just add ExporterImageDescriptorKey ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added both and image names as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the ExporterImageDigestKey? The value is already in the descriptor.
4308a15 to
b6dfe04
Compare
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
b6dfe04 to
bd08e43
Compare
In the Docker GitHub Builder workflows we are merging manifests with the
imagetools createcommand. This command doesn't output the pushed digest whilebuildorbakecommands do it.This PR add a
metadata-fileflag similar tobuildandbakecommands.This will be necessary in our github builder so we can have an output for this digest that can then be used in subsequent jobs.