-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
DO NOT MERGE: Smoke-test of a costly “always compute old-style IDs” approach #24419
base: main
Are you sure you want to change the base?
DO NOT MERGE: Smoke-test of a costly “always compute old-style IDs” approach #24419
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mtrmac The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
c179f32
to
5345c94
Compare
Ephemeral COPR build failed. @containers/packit-build please check. |
1 similar comment
Ephemeral COPR build failed. @containers/packit-build please check. |
3d26edf
to
85bf387
Compare
Note to self: If/after containers/storage#2156 merges, this PR will need to re-enable Edit: done. |
85bf387
to
776b38d
Compare
776b38d
to
93405a1
Compare
|
7a0271e
to
b14da5e
Compare
Would you care to follow (and test and review) my new simplified automation_images doc? You will need to import https://github.com/containers/automation_images/pull/388/files but apart from that it really should be fairly simple. |
b14da5e
to
4148a66
Compare
Thanks, I’ll definitely try that. |
Note to self: containers/automation_images#395 . |
ac0bb3f
to
9b2c391
Compare
07052cc
to
fca988f
Compare
e8de55a
to
d8a0cea
Compare
d8a0cea
to
ed2d593
Compare
|
b128c72
to
782aa77
Compare
6828853
to
7cf1875
Compare
This is now on top of #25007, and differs from it by:
|
7cf1875
to
8f28e37
Compare
... because (podman system reset) will delete all of it, interfering with the test storing other data in the directory. Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
... from containers/automation_images#395 . Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
8f28e37
to
2c318f6
Compare
This is a variant of #24287 which reverts (some of) the system test changes to again expect old-style IDs, and includes containers/storage#2155 / containers/image#2613 .
This exists purely to give us options. With this, zstd:chunked loses some of the performance wins, and it should revert to old-style IDs. We then have the option to reintroduce the ID change and performance improvement (perhaps even based on measurements) later.
And, separately, this enforces that layers should match against the config’s DiffIDs, resolving the signing ambiguity.
FYI @giuseppe @edsantiago @mheon @baude
Does this PR introduce a user-facing change?