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

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

Merged
merged 1 commit into from
Oct 12, 2021

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Oct 11, 2021

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 [email protected]

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 <[email protected]>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 11, 2021

[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

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
Member

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
Member 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
Member

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
Copy link
Member

mheon commented Oct 11, 2021

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
Member

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
Copy link
Member Author

rhatdan commented Oct 12, 2021

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
Copy link
Member

mheon commented Oct 12, 2021

/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 containers: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