storage
storage copied to clipboard
Add support for drop-in config
Helps in use cases where a drop-in config could help by allowing users to update a subsection of the config instead of touching an entire config file.
e.g.
- https://github.com/containers/storage/issues/1306
- https://github.com/openshift/enhancements/pull/1600/files#diff-10bfa811cdabc678cbfd947dbeaf04b63ebd24e8d9ab5a05c7abbb998cb72f8aR135
- https://github.com/kubernetes/enhancements/issues/4191
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: harche Once this PR has been reviewed and has the lgtm label, please assign nalind for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/hold
Could you open a [WIP] PR against Podman to test this feature, and make sure it does not break anything in it's ci/cd system.
Could you open a [WIP] PR against Podman to test this feature, and make sure it does not break anything in it's ci/cd system.
@rhatdan I opened https://github.com/containers/podman/pull/22484. Most of the jobs were green, apart from the one for the mac. I am not able to trigger the retest of that job, it needs /ok-to-test label.
@nalind @giuseppe @saschagrunert @vrothberg PTAL
This would be really quite useful to have, it's just completely annoying right now to e.g. enable composefs because one needs to copy all of the storage config.
@harche Hi, thanks for working on this PR. This PR looks beneficial for several use cases so it would be great if we can move this PR forward.
So it seems like our only option is to use meta.Keys()?
If you have the patch for the meta.Keys() approach, could you update this PR with that patch? Or, if you don't have enough time for this PR, I'm willing to take over this PR.
@harche Hi, thanks for working on this PR. This PR looks beneficial for several use cases so it would be great if we can move this PR forward.
So it seems like our only option is to use meta.Keys()?
If you have the patch for the
meta.Keys()approach, could you update this PR with that patch? Or, if you don't have enough time for this PR, I'm willing to take over this PR.
Thanks @ktock but this is an optional (good to have) thing for the lazy image pull. In case of Openshift, there is an operator called MCO which drops the storage configuration on the node. It is totally possible to implement the logic in MCO to drop a storage config file on the node to support lazy image pulls. MCO has been doing that with KubeletConfig for awhile now.
Since this is an optional feature from lazy image point of view, this change is not on high priority. But you can still look into it if you feel this library should support drop-in config nevertheless.
My personal opinion is that this is fairly likely to be brittle and hard-to-maintain code (though I could be well wrong about that, there might be a neat trick that makes it easy); and that the option parsing in c/storage is already convoluted enough that I’d prefer not to add any extra complexity.
I’m not a c/storage maintainer, though, so it’s not up to me.