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.

Sign up for GitHub

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

Jump to bottom

remove image podman no prune #15093

Merged
merged 1 commit into from
Aug 5, 2022
Merged

remove image podman no prune #15093

merged 1 commit into from
Aug 5, 2022

Conversation

karthikelango137
Copy link
Contributor

Docker supports --no-prune for rmi which does not remove dangling parents of the specified image. Podman
should support the same for compatibility.

Task from #14917

Signed-off-by: Karthik Elango [email protected]

Does this PR introduce a user-facing change?

Added --no-prune flag for podman rmi

@openshift-ci openshift-ci bot added release-note do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jul 27, 2022
vrothberg
vrothberg reviewed Jul 28, 2022
View reviewed changes
go.mod Outdated
@@ -74,3 +74,5 @@ require (
require github.com/docker/libnetwork v0.8.0-dev.2.0.20190625141545-5a177b73e316 // indirect

replace github.com/opencontainers/runc => github.com/opencontainers/runc v1.1.1-0.20220617142545-8b9452f75cbc

replace github.com/containers/common => github.com/karthikelango137/common v0.48.1-0.20220727150028-71a5d2c2db12
Copy link
Member

Choose a reason for hiding this comment

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

Just to be extra sure. Before merging, we need a PR in c/common and then vendor from there.

@karthikelango137 karthikelango137 force-pushed the rmiNoPrune1 branch 14 times, most recently from 2c9d283 to 5eaf5f3 Compare August 1, 2022 13:45
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 1, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 1, 2022
@karthikelango137 karthikelango137 force-pushed the rmiNoPrune1 branch 10 times, most recently from bcf7661 to 956e9a3 Compare August 2, 2022 21:23
@karthikelango137 karthikelango137 force-pushed the rmiNoPrune1 branch 7 times, most recently from a55e787 to 252c7eb Compare August 4, 2022 14:31
@karthikelango137 karthikelango137 marked this pull request as ready for review August 4, 2022 17:33
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 4, 2022
@karthikelango137
Copy link
Contributor Author

@containers/podman-maintainers PTAL

@karthikelango137 karthikelango137 changed the title DO NOT MERGE: remove image podman no prune remove image podman no prune Aug 4, 2022
ashley-cui
ashley-cui reviewed Aug 4, 2022
View reviewed changes
Copy link
Member

@ashley-cui ashley-cui left a comment

Choose a reason for hiding this comment

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

LGTM, but looks like you need a rebase or something, as you have a merge conflict.

cdoern
cdoern reviewed Aug 4, 2022
View reviewed changes
Copy link
Contributor

@cdoern cdoern left a comment

Choose a reason for hiding this comment

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

some vendor/rebase leftover stuff, otherwise LGTM

go.sum Outdated
@@ -395,8 +395,13 @@ github.com/containernetworking/plugins v1.1.1/go.mod h1:Sr5TH/eBsGLXK/h71HeLfX19
github.com/containers/buildah v1.26.1-0.20220716095526-d31d27c357ab h1:NeI0DOkTf3Tn4OpdjMhMubAfTPs2oCO5jUY5wnpv4qk=
github.com/containers/buildah v1.26.1-0.20220716095526-d31d27c357ab/go.mod h1:iVtQtU6a+pbETBqIzg0oAWW3gTR1ItrAihJpLFFppmA=
github.com/containers/common v0.48.1-0.20220715075726-2ac10faca05a/go.mod h1:1dA7JPGoSi83kjf5H4NIrGANyLOULyvFqV1bwvYFEek=
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

leftover rebase weirdness

vendor/modules.txt Outdated
@@ -114,7 +114,7 @@ github.com/containers/buildah/pkg/rusage
github.com/containers/buildah/pkg/sshagent
github.com/containers/buildah/pkg/util
github.com/containers/buildah/util
# github.com/containers/common v0.49.2-0.20220804143628-dc97077782d5
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like a downgrade

@karthikelango137
remove image podman no prune
cc8e4d5 
Signed-off-by: Karthik Elango <[email protected]>
vrothberg
vrothberg reviewed Aug 5, 2022
View reviewed changes
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM
@containers/podman-maintainers PTAL

Nice work, @karthikelango137 !

ashley-cui
ashley-cui reviewed Aug 5, 2022
View reviewed changes
Copy link
Member

@ashley-cui ashley-cui left a comment

Choose a reason for hiding this comment

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

/lgtm

Great job @karthikelango137 ! :)

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2022
Luap99
Luap99 approved these changes Aug 5, 2022
View reviewed changes
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 5, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karthikelango137, Luap99

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 Aug 5, 2022
@openshift-ci openshift-ci bot merged commit 66c246d into containers:main Aug 5, 2022
@ankon ankon mentioned this pull request Mar 31, 2023
Closed
vrothberg pushed a commit to vrothberg/libpod that referenced this pull request Apr 20, 2023
@ankon @vrothberg
compat: Translate noprune into ImageRemoveOptions.NoPrune
b65ab52 
PR containers#15093 implemented support for NoPrune in the ImageRemoveOptions,
this PR simply brings that also to the compat API along with
regression tests.

Signed-off-by: Andreas Kohn <[email protected]>
Signed-off-by: Valentin Rothberg <[email protected]>
@vrothberg vrothberg mentioned this pull request Apr 20, 2023
Merged
@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 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ashley-cui ashley-cui ashley-cui left review comments

@cdoern cdoern cdoern left review comments

@vrothberg vrothberg vrothberg left review comments

@Luap99 Luap99 Luap99 approved these changes

Assignees

@ashley-cui ashley-cui

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. release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@karthikelango137 @vrothberg @cdoern @ashley-cui @Luap99 @openshift-merge-robot