[WIP] store: Make additionalimagestores configurable as layerstore source #20604
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. 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/test-infra repository. |
openshift-ci
bot
added
the
do-not-merge/release-note-label-needed
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: idleroamer The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Cockpit tests failed for commit 76f3bcd. @martinpitt, @jelly, @mvollmer please check. |
|
@idleroamer thank you for the PR! However, you are making changes in code that is vendored into podman and need to be fixed in the original Can you please open a PR in https://github.com/containers/storage with your changes - once your changes are merged into containers/storage you will have to update this PR to pull in those new changes. The steps for those would be |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
could you explain the proposed fix? If you don't want an additional store, why would you specify it in the config file? |
AdditionalImageStores may alter independent of current RWLayerStore resulting in layers references to the ROImageStore, created during the reuseBlob operations, to break. Optionally allow ignoring stores in AdditionalImageStores as layer store. Closes: Closes: #20603 Signed-off-by: Mostafa Emami <mustafaemami@gmail.com> Signed-off-by: 😎Mostafa Emami <mustafaemami@gmail.com>
rhatdan
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Cockpit tests failed for commit 2e14a94. @martinpitt, @jelly, @mvollmer please check. |
| continue | ||
| } | ||
| store = strings.Split(store, "|")[0] | ||
| } |
There was a problem hiding this comment.
I can’t see how this would work; layers in these stores would be effectively invisible, but images in those stores would be visible as usual.
The immediate effect is that all images from a no-reuse store are visible but have unresolvable layers = look broken to all existing tooling.
| @@ -344,7 +344,7 @@ Deleted: $pauseID" | |||
| driver="overlay" | |||
|
|
|||
| [storage.options] | |||
| additionalimagestores = [ "$imstore/root" ] | |||
| additionalimagestores = [ "$imstore/root|NO_REUSE" ] | |||
There was a problem hiding this comment.
We can invent an infinite number of new options. Please don’t add ambiguous in-line syntaxes unless there is really no other alternative.
There was a problem hiding this comment.
I agree, this is just merely a compact version to collect feedback. There should be more agreeable manners to achieve it.
@giuseppe thanks for the review, I suppose @mtrmac summarize it very well containers/storage#1751 (comment) |
thanks, opened containers/storage#1752 |
github-actions
bot
added
the
locked - please file new issue/PR
AdditionalImageStores may alter independent of current RWLayerStore resulting in layers references to the ROImageStore, created during the reuseBlob operations, to break.
Optionally allow ignoring stores in AdditionalImageStores as layer store.
Closes: Closes: containers/storage#1751
Does this PR introduce a user-facing change?