-
Notifications
You must be signed in to change notification settings - Fork 71
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
Export Run Image Metadata #313
Conversation
Maintainers, As you review this RFC please queue up issues to be created using the following commands:
Issues(none) |
Signed-off-by: Joey Brown <[email protected]>
ddb1828
to
6c56aa2
Compare
Signed-off-by: Joey Brown <[email protected]>
eba0bcf
to
0af349b
Compare
Signed-off-by: Joey Brown <[email protected]>
8dd588f
to
7ccbed1
Compare
- Rebase process may remain suboptimal for some platform providers. | ||
|
||
2. Write all the metadata labels to `report.toml`. | ||
- This could break platform operators that are using the `terminationMessagePath` [pattern](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#pod-v1-core). Because some of the metadata includes unbounded arrays, we could explode this report beyond the max size of 4096 bytes. |
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.
@tomkennedy513 do you have any feedback here?
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.
FWIW I don't think we're close to hitting the limit yet (though it's been said before...)
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.
IIRC kpack doesn't blindly dump the report.toml directly, it parses out only the stuff it cares about and writes that to terminationMessagePath
instead.
I don't think we've encountered any issues with length yet, but I image that's because we limit the unbounded array to just the id, version, and homepage metadata of buildpacks.
Signed-off-by: Joey Brown <[email protected]>
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.
💥
…n-image-metadata.md Updating for merge Signed-off-by: Jesse Brown <[email protected]> Signed-off-by: Jesse Brown <[email protected]>
readable