feat: add confirmation label when volume spec changes#13808
Conversation
76c9cda to
395e345
Compare
|
This is a useful feature for CD pipeline automation — the interactive volume recreation prompt is a common friction point in CI/CD environments. A few thoughts on the implementation: Label naming convention
Scope: volumes only, or also other resources?The same "recreate on spec change" problem applies to networks (e.g., network subnet changes) and potentially configs/secrets. While it's fine to scope this PR to volumes only, it would be worth noting in the PR description whether this label pattern is intended to be extensible to other resource types in the future, or if it's intentionally volume-specific. Safety consideration: data loss riskReproducing a named volume destroys existing data. The label effectively silences a prompt that exists specifically to prevent accidental data loss. The implementation should clearly document this in:
CI workflow exampleA concrete usage example in the PR description would help reviewers understand the target use case: volumes:
my_data:
labels:
com.docker.compose.volume.recreate-when-spec-updated: "true"This makes it immediately clear whether the label value matters (boolean string) or just the presence of the label key is sufficient. Good feature for automation-first workflows. The main thing I'd want to see before merge is explicit data-loss documentation. |
|
I checked the code and couldn't find any other resource types that currently support this kind of recreation behavior. Because of that, I'd prefer to keep the scope limited to volumes for now. I'm also not a big fan of a value-based label such as At the moment, the behavior is essentially a yes/no decision, similar to how the |
74d0fb6 to
0f9e44a
Compare
|
@Quantum0uasar I updated the code comments and added warning logs to show what will happen when the volume spec changes. But I couldn't find where to add the documentation. Should I add it to |
ec6d7dc to
76e9a92
Compare
|
Thanks for adding the PR and tests. One edge case I would check before this moves forward: the current patch calls That means these two cases can behave unexpectedly:
A useful regression test would be Small secondary point: if the label is present with an invalid or false-ish value, it currently skips the prompt and returns false. If the label is only meant to opt in to auto-confirm, false/invalid values may be safer as normal prompt behavior instead of silently changing the decision. |
close: docker#13807 Signed-off-by: qianlongzt <18493471+qianlongzt@users.noreply.github.com>
76e9a92 to
97612be
Compare
|
@vaguul I updated it to use a Docker Compose volume label and prompt the user for confirmation when the label is set to false. |
|
Thanks for updating it. The current version addresses the edge case I was worried about: confirmVolumeRecreate(volume.Labels, ...) now uses the desired Compose spec labels, and the new e2e case covers the old-file-without-label -> new-file-with-label path.\n\nI also like that false/no-label now falls back to the existing prompt behavior instead of silently changing the decision. From my side, this looks much safer. |
What I did
Add support for label:
com.docker.compose.volume.recreate-when-spec-updatedThis allows automatic handling of volume recreation when specs change, improving compatibility with CD workflows by avoiding interactive prompts.
Related issue
closes: #13807
(not mandatory) A picture of a cute animal, if possible in relation to what you did