Skip to content

Remove a volume with --force if container is running - #11925

Merged
openshift-merge-robot merged 1 commit into
podman-container-tools:mainfrom
rhatdan:volume
Oct 12, 2021
Merged

Remove a volume with --force if container is running#11925
openshift-merge-robot merged 1 commit into
podman-container-tools:mainfrom
rhatdan:volume

Conversation

@rhatdan

@rhatdan rhatdan commented Oct 11, 2021

Copy link
Copy Markdown
Contributor

Currently we are not passing the force flag down to the removal of
the running container. If the container is running, and we set
--force when removing the volume, the container should be stopped.

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Currently we are not passing the force flag down to the removal of
the running container. If the container is running, and we set
--force when removing the volume, the container should be stopped.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@openshift-ci

openshift-ci Bot commented Oct 11, 2021

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 11, 2021
cid=$output
run_podman 2 volume rm myvol
is "$output" "Error: volume myvol is being used by the following container(s): $cid: volume is being used" "should error since container is running"
run_podman volume rm myvol --force

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to add a check here, to make sure $cid is stopped?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I figured it was since the rm failed previously complaining about the running container. When this is done the $cid should no longer exists.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. So wouldn't it be nice to confirm it? run_podman 1 container exists $cid ? But I can add it in one of my future grab-bag bats PRs

@mheon

mheon commented Oct 11, 2021

Copy link
Copy Markdown
Contributor

Is there a specific reason we want to make this change? Not really opposed, but it is sort of a breaking change

@TomSweeneyRedHat

Copy link
Copy Markdown
Contributor

We probably ought to note this functionality in a man page or two, and verify that it doesn't now mention the current behavior.
Changes LGTM, I'll let @mheon and @rhatdan thumb wrestle over the need of the change.

@rhatdan

rhatdan commented Oct 12, 2021

Copy link
Copy Markdown
Contributor Author

Huh this seems to be documented to me. And I don't see this as a breaking change, as opposed to a fixed bug. The podman volume rm --help shows this.

-f, --force Remove a volume by force, even if it is being used by a container

The Man page says.

   --force, -f
       Remove  a volume by force.  If it is being used by containers, the con‐
       tainers will be removed first.

How is this a breaking change as opposed to a bug?

@mheon

mheon commented Oct 12, 2021

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 12, 2021
@openshift-merge-robot
openshift-merge-robot merged commit c90beed into podman-container-tools:main Oct 12, 2021
@github-actions github-actions Bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants