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

DO NOT MERGE: Smoke-test of a costly “always compute old-style IDs” approach #24419

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Oct 30, 2024

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 .

⚠️ WARNING: all of this is a quick sketch, absolutely untested.

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?

None

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none labels Oct 30, 2024
Copy link
Contributor

openshift-ci bot commented Oct 30, 2024

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 30, 2024
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 30, 2024
@openshift-merge-robot
Copy link
Collaborator

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@mtrmac mtrmac force-pushed the mitr-zstd-costly-old-ids-integration branch from c179f32 to 5345c94 Compare October 30, 2024 15:19
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

1 similar comment
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@mtrmac mtrmac changed the title DO NOT MERGE: Smoke-test of costly “always compute old-style IDs” approach DO NOT MERGE: Smoke-test of a costly “always compute old-style IDs” approach Oct 30, 2024
…ifferent methods"

This reverts commit 1d7ec1e.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Read the full one, not the truncated one

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac mtrmac force-pushed the mitr-zstd-costly-old-ids-integration branch 5 times, most recently from 3f58ebf to 3d26edf Compare October 31, 2024 17:40
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac mtrmac force-pushed the mitr-zstd-costly-old-ids-integration branch from 3d26edf to 85bf387 Compare October 31, 2024 18:29
@mtrmac
Copy link
Collaborator Author

mtrmac commented Nov 4, 2024

Note to self: If/after containers/storage#2156 merges, this PR will need to re-enable enable_partial_images.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants