specifying --force,podman exit with 0 #14782

aonoa · 2022-06-30T02:29:21Z

Does this PR introduce a user-facing change?
The resource disappears at the end of the rm command, with --force podman exit with 0

openshift-ci · 2022-06-30T02:29:26Z

@aonoa: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Signed-off-by: wufan <[email protected]>

rhatdan · 2022-06-30T10:36:38Z

First you have to sign your commits

git commit -a --amend -s
git push --force

rhatdan · 2022-06-30T10:39:22Z

You need to add tests for this on each one of the --rm types.

rhatdan · 2022-06-30T10:37:29Z

cmd/podman/networks/rm.go

@@ -71,6 +71,9 @@ func networkRm(cmd *cobra.Command, args []string) error {
 			fmt.Println(r.Name)
 		} else {
 			setExitCode(r.Err)
+			if networkRmOptions.Force && registry.GetExitCode() == 1 {


This should only happen if the error is network does not exist, and this is the only error returned.

rhatdan · 2022-06-30T10:37:44Z

cmd/podman/pods/rm.go

@@ -94,6 +94,9 @@ func removePods(namesOrIDs []string, rmOptions entities.PodRmOptions, printIDs b
 	responses, err := registry.ContainerEngine().PodRm(context.Background(), namesOrIDs, rmOptions)
 	if err != nil {
 		setExitCode(err)
+		if rmOptions.Force && registry.GetExitCode() == 1 {
+			registry.SetExitCode(define.ExecErrorCodeIgnore)


This should only happen if the error is pod does not exist, and this is the only error returned.

rhatdan · 2022-06-30T10:38:23Z

cmd/podman/volumes/rm.go

@@ -73,6 +73,9 @@ func rm(cmd *cobra.Command, args []string) error {
 			fmt.Println(r.Id)
 		} else {
 			setExitCode(r.Err)
+			if rmOptions.Force && registry.GetExitCode() == 1 {


This should only happen if the error is volume does not exist, and this is the only error returned.

rhatdan · 2022-06-30T10:38:49Z

cmd/podman/images/rm.go

@@ -82,6 +82,9 @@ func rm(cmd *cobra.Command, args []string) error {
 			}
 		}
 		registry.SetExitCode(report.ExitCode)
+		if imageOpts.Force && registry.GetExitCode() == 1 {


This should only happen if the error is image does not exist, and this is the only error returned.

rhatdan · 2022-06-30T10:39:00Z

cmd/podman/containers/rm.go

@@ -140,6 +140,9 @@ func removeContainers(namesOrIDs []string, rmOptions entities.RmOptions, setExit
 			}
 			if setExit {
 				setExitCode(r.Err)
+				if rmOptions.Force && registry.GetExitCode() == 1 {


This should only happen if the error is container does not exist, and this is the only error returned.

I recommend moving this into setExitCode() this will make the code much cleaner, same for the other places.

The problem with that is the NOEXIST errors are all different.

How are they all different? The setExitCode() already take care of setting this, so all what needs to be done is check if --force is set

I am saying the current code is incorrect since errors other then NOTEXIST should be reported --force rm should only not fail if the Object does not exist.

openshift-ci · 2022-06-30T10:39:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aonoa, 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:

~~OWNERS~~ [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Luap99

Please use your real name in the commit sign off, see https://github.com/containers/podman/blob/main/CONTRIBUTING.md#sign-your-prs

This change also requires a release note in the PR description, please do not set it to none. This is a user facing change.

And also add some tests to make sure we do not regress in the future.

Luap99 · 2022-06-30T12:00:00Z

cmd/podman/root.go

+		if registry.GetExitCode() == define.ExecErrorCodeIgnore {
+			registry.SetExitCode(0)
+		}


Do not use a special code for this, this will fail when a container exits with 128. Just set the exit code to 0 when you change it.

Yes, I also think definition 128 is not good, but any error will make the return value not 0

podman/cmd/podman/root.go

Line 100 in 3426d56

if err := rootCmd.ExecuteContext(registry.GetContextWithOptions()); err != nil {

you could make the setExitCode function return the error and then set it to nil if it should not error

Or maybe more easy make --force imply --ignore, that should work al tleast for the commands who have --ignore.

Luap99 · 2022-06-30T12:01:05Z

cmd/podman/containers/rm.go

@@ -140,6 +140,9 @@ func removeContainers(namesOrIDs []string, rmOptions entities.RmOptions, setExit
 			}
 			if setExit {
 				setExitCode(r.Err)
+				if rmOptions.Force && registry.GetExitCode() == 1 {


I recommend moving this into setExitCode() this will make the code much cleaner, same for the other places.

aonoa · 2022-06-30T14:08:44Z

请在提交签名时使用您的真实姓名，请参阅https://github.com/containers/podman/blob/main/CONTRIBUTING.md#sign-your-prs

My user name is not my real name

mheon · 2022-06-30T19:07:25Z

I'll pass the DCO check manually, it's oversensitive about this.

mheon · 2022-06-30T19:07:53Z

You have a lint error

vrothberg · 2022-07-05T14:21:59Z

@aonoa can you fix the lint error?

Running golangci-lint for tunnel
Build Tags tunnel: apparmor,seccomp,selinux,linter,remote
Skipped directories tunnel: pkg/api,pkg/machine/e2e
cmd/podman/images/rm.go:4: File is not goimports-ed (goimports)
"fmt"

aonoa · 2022-07-05T15:22:08Z

@aonoa你能修复 lint 错误吗？

为隧道运行 golangci-lint
构建标签隧道：apparmor，seccomp，selinux，linter，remote
跳过的目录隧道：pkg/api，pkg/machine/e2e
cmd/podman/images/rm.go:4：文件未goimports-ed（ goimports)
"fmt"

I will add a blank line after the 4 line
golangci/golangci-lint#138 (comment)

Signed-off-by: aonoa <[email protected]>

vrothberg · 2022-07-11T12:32:22Z

@aonoa CI isn't yet happy.

Signed-off-by: aonoa <[email protected]>

vrothberg · 2022-07-12T13:29:19Z

Something wrong must have happened while rebasing. Can you rebase another time and squash the commits into one?

rhatdan · 2022-07-18T09:53:25Z

@aonoa Can you rebase and clean this up?

rhatdan · 2022-07-18T14:36:54Z

I opened up a replacement for this PR. #14959 Should I close this PR?

aonoa · 2022-07-19T01:25:59Z

I opened up a replacement for this PR. #14959 Should I close this PR?

Should, I don't know how to rebase, it's a mess now.

openshift-ci bot added the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label Jun 30, 2022

specifying --force,podman exit with 0

57bcd20

Signed-off-by: wufan <[email protected]>

aonoa force-pushed the main branch from d47bd9a to 57bcd20 Compare June 30, 2022 03:22

rhatdan approved these changes Jun 30, 2022

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 30, 2022

Luap99 reviewed Jun 30, 2022

View reviewed changes

aonoa and others added 2 commits July 6, 2022 23:10

Merge branch 'containers:main' into main

a197ff9

consider more and add test

f8d2145

Signed-off-by: aonoa <[email protected]>

aonoa and others added 3 commits July 12, 2022 20:14

Merge code

ef3546f

Signed-off-by: aonoa <[email protected]>

Merge branch 'containers:main' into main

e1c9286

Merge code

cab23bb

Signed-off-by: aonoa <[email protected]>

rhatdan mentioned this pull request Jul 18, 2022

When removing objects specifying --force,podman should exit with 0 #14959

Merged

rhatdan closed this Jul 19, 2022

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 21, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

specifying --force,podman exit with 0 #14782

specifying --force,podman exit with 0 #14782

aonoa commented Jun 30, 2022 •

edited

Loading

openshift-ci bot commented Jun 30, 2022

rhatdan commented Jun 30, 2022

rhatdan commented Jun 30, 2022

rhatdan Jun 30, 2022

rhatdan Jun 30, 2022

rhatdan Jun 30, 2022

rhatdan Jun 30, 2022

rhatdan Jun 30, 2022

Luap99 Jun 30, 2022

rhatdan Jun 30, 2022

Luap99 Jun 30, 2022

rhatdan Jun 30, 2022

openshift-ci bot commented Jun 30, 2022

Luap99 left a comment

Luap99 Jun 30, 2022

aonoa Jun 30, 2022

Luap99 Jul 5, 2022

Luap99 Jun 30, 2022

aonoa commented Jun 30, 2022

mheon commented Jun 30, 2022

mheon commented Jun 30, 2022

vrothberg commented Jul 5, 2022 •

edited

Loading

aonoa commented Jul 5, 2022

vrothberg commented Jul 11, 2022

vrothberg commented Jul 12, 2022

rhatdan commented Jul 18, 2022

rhatdan commented Jul 18, 2022

aonoa commented Jul 19, 2022

specifying --force,podman exit with 0 #14782

specifying --force,podman exit with 0 #14782

Conversation

aonoa commented Jun 30, 2022 • edited Loading

openshift-ci bot commented Jun 30, 2022

rhatdan commented Jun 30, 2022

rhatdan commented Jun 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Jun 30, 2022

Luap99 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aonoa commented Jun 30, 2022

mheon commented Jun 30, 2022

mheon commented Jun 30, 2022

vrothberg commented Jul 5, 2022 • edited Loading

aonoa commented Jul 5, 2022

vrothberg commented Jul 11, 2022

vrothberg commented Jul 12, 2022

rhatdan commented Jul 18, 2022

rhatdan commented Jul 18, 2022

aonoa commented Jul 19, 2022

aonoa commented Jun 30, 2022 •

edited

Loading

vrothberg commented Jul 5, 2022 •

edited

Loading