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

specifying --force,podman exit with 0 #14782

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 36 additions & 11 deletions cmd/podman/containers/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func removeContainers(namesOrIDs []string, rmOptions entities.RmOptions, setExit
responses, err := registry.ContainerEngine().ContainerRm(context.Background(), namesOrIDs, rmOptions)
if err != nil {
if setExit {
setExitCode(err)
setExitCode(rmOptions.Force, []error{err})
}
return err
}
Expand All @@ -136,25 +136,50 @@ func removeContainers(namesOrIDs []string, rmOptions entities.RmOptions, setExit
if errors.Is(r.Err, define.ErrWillDeadlock) {
logrus.Errorf("Potential deadlock detected - please run 'podman system renumber' to resolve")
}
if setExit {
setExitCode(r.Err)
}
errs = append(errs, r.Err)
} else {
fmt.Println(r.Id)
}
}
if setExit {
setExitCode(rmOptions.Force, errs)
}

return errs.PrintErrors()
}

func setExitCode(err error) {
// If error is set to no such container, do not reset
if registry.GetExitCode() == 1 {
return
func setExitCode(force bool, errs []error) {
var (
// noSuchContainerErrors indicates the requested container does not exist
noSuchContainerErrors bool
// stateInvalidErrors indicates a container is in an improper state
stateInvalidErrors bool
)

if len(errs) == 0 {
registry.SetExitCode(0)
}

for _, err := range errs {
if errors.Is(err, define.ErrNoSuchCtr) || strings.Contains(err.Error(), define.ErrNoSuchCtr.Error()) {
noSuchContainerErrors = true
} else if errors.Is(err, define.ErrCtrStateInvalid) || strings.Contains(err.Error(), define.ErrCtrStateInvalid.Error()) {
stateInvalidErrors = true
}
}
if errors.Is(err, define.ErrNoSuchCtr) || strings.Contains(err.Error(), define.ErrNoSuchCtr.Error()) {
registry.SetExitCode(1)
} else if errors.Is(err, define.ErrCtrStateInvalid) || strings.Contains(err.Error(), define.ErrCtrStateInvalid.Error()) {

switch {
case stateInvalidErrors:
registry.SetExitCode(2)
case noSuchContainerErrors:
// One of the specified container did not exist, and no other
// failures.
if force {
registry.SetExitCode(define.ExecErrorCodeIgnore)
} else {
registry.SetExitCode(1)
}
default:
registry.SetExitCode(125)
}
}
4 changes: 4 additions & 0 deletions cmd/podman/images/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/containers/podman/v4/cmd/podman/common"
"github.com/containers/podman/v4/cmd/podman/registry"
"github.com/containers/podman/v4/libpod/define"
"github.com/containers/podman/v4/pkg/domain/entities"
"github.com/containers/podman/v4/pkg/errorhandling"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -82,6 +83,9 @@ func rm(cmd *cobra.Command, args []string) error {
}
}
registry.SetExitCode(report.ExitCode)
if imageOpts.Force && registry.GetExitCode() == 1 {
Copy link
Member

Choose a reason for hiding this comment

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

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

registry.SetExitCode(define.ExecErrorCodeIgnore)
}
}

return errorhandling.JoinErrors(rmErrors)
Expand Down
2 changes: 1 addition & 1 deletion cmd/podman/networks/prune.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func networkPrune(cmd *cobra.Command, _ []string) error {
}
responses, err := registry.ContainerEngine().NetworkPrune(registry.Context(), networkPruneOptions)
if err != nil {
setExitCode(err)
setExitCode(false, []error{err})
return err
}
return utils.PrintNetworkPruneResults(responses, false)
Expand Down
42 changes: 36 additions & 6 deletions cmd/podman/networks/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,24 +63,54 @@ func networkRm(cmd *cobra.Command, args []string) error {
}
responses, err := registry.ContainerEngine().NetworkRm(registry.Context(), args, networkRmOptions)
if err != nil {
setExitCode(err)
setExitCode(networkRmOptions.Force, []error{err})
return err
}
for _, r := range responses {
if r.Err == nil {
fmt.Println(r.Name)
} else {
setExitCode(r.Err)
errs = append(errs, r.Err)
}
}
setExitCode(networkRmOptions.Force, errs)

return errs.PrintErrors()
}

func setExitCode(err error) {
if errors.Is(err, define.ErrNoSuchNetwork) || strings.Contains(err.Error(), define.ErrNoSuchNetwork.Error()) {
registry.SetExitCode(1)
} else if errors.Is(err, define.ErrNetworkInUse) || strings.Contains(err.Error(), define.ErrNetworkInUse.Error()) {
func setExitCode(force bool, errs []error) {
var (
// noSuchNetworkErrors indicates the requested network does not exist.
noSuchNetworkErrors bool
// inUseErrors indicates the requested operation failed because the network was in use
inUseErrors bool
)

if len(errs) == 0 {
registry.SetExitCode(0)
}

for _, err := range errs {
if errors.Is(err, define.ErrNoSuchNetwork) || strings.Contains(err.Error(), define.ErrNoSuchNetwork.Error()) {
noSuchNetworkErrors = true
} else if errors.Is(err, define.ErrNetworkInUse) || strings.Contains(err.Error(), define.ErrNetworkInUse.Error()) {
inUseErrors = true
}
}

switch {
case inUseErrors:
// network is being used.
registry.SetExitCode(2)
case noSuchNetworkErrors && !inUseErrors:
// One of the specified network did not exist, and no other
// failures.
if force {
registry.SetExitCode(define.ExecErrorCodeIgnore)
} else {
registry.SetExitCode(1)
}
default:
registry.SetExitCode(125)
}
}
32 changes: 27 additions & 5 deletions cmd/podman/pods/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func removePods(namesOrIDs []string, rmOptions entities.PodRmOptions, printIDs b

responses, err := registry.ContainerEngine().PodRm(context.Background(), namesOrIDs, rmOptions)
if err != nil {
setExitCode(err)
setExitCode(rmOptions.Force, []error{err})
return err
}

Expand All @@ -104,15 +104,37 @@ func removePods(namesOrIDs []string, rmOptions entities.PodRmOptions, printIDs b
fmt.Println(r.Id)
}
} else {
setExitCode(r.Err)
errs = append(errs, r.Err)
}
}
setExitCode(rmOptions.Force, errs)
return errs.PrintErrors()
}

func setExitCode(err error) {
if errors.Is(err, define.ErrNoSuchPod) || strings.Contains(err.Error(), define.ErrNoSuchPod.Error()) {
registry.SetExitCode(1)
func setExitCode(force bool, errs []error) {
var (
// noSuchNetworkErrors indicates the requested pod does not exist.
noSuchPodErrors bool
)

if len(errs) == 0 {
registry.SetExitCode(0)
}

for _, err := range errs {
if errors.Is(err, define.ErrNoSuchPod) || strings.Contains(err.Error(), define.ErrNoSuchPod.Error()) {
noSuchPodErrors = true
}
}

switch {
case noSuchPodErrors:
if force {
registry.SetExitCode(define.ExecErrorCodeIgnore)
} else {
registry.SetExitCode(1)
}
default:
registry.SetExitCode(125)
}
}
3 changes: 3 additions & 0 deletions cmd/podman/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ func Execute() {
fmt.Fprintln(os.Stderr, "Cannot connect to Podman. Please verify your connection to the Linux system using `podman system connection list`, or try `podman machine init` and `podman machine start` to manage a new Linux VM")
}
}
if registry.GetExitCode() == define.ExecErrorCodeIgnore {
registry.SetExitCode(0)
}
Comment on lines +109 to +111
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

fmt.Fprintln(os.Stderr, formatError(err))
}
os.Exit(registry.GetExitCode())
Expand Down
40 changes: 34 additions & 6 deletions cmd/podman/volumes/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,24 +65,52 @@ func rm(cmd *cobra.Command, args []string) error {
}
responses, err := registry.ContainerEngine().VolumeRm(context.Background(), args, rmOptions)
if err != nil {
setExitCode(err)
setExitCode(rmOptions.Force, []error{err})
return err
}
for _, r := range responses {
if r.Err == nil {
fmt.Println(r.Id)
} else {
setExitCode(r.Err)
errs = append(errs, r.Err)
}
}
setExitCode(rmOptions.Force, errs)
return errs.PrintErrors()
}

func setExitCode(err error) {
if errors.Is(err, define.ErrNoSuchVolume) || strings.Contains(err.Error(), define.ErrNoSuchVolume.Error()) {
registry.SetExitCode(1)
} else if errors.Is(err, define.ErrVolumeBeingUsed) || strings.Contains(err.Error(), define.ErrVolumeBeingUsed.Error()) {
func setExitCode(force bool, errs []error) {
var (
// noSuchVolumeErrors indicates the requested volume does not exist
noSuchVolumeErrors bool
// inUseErrors indicates that a volume is being used by at least one container
inUseErrors bool
)

if len(errs) == 0 {
registry.SetExitCode(0)
}

for _, err := range errs {
if errors.Is(err, define.ErrNoSuchVolume) || strings.Contains(err.Error(), define.ErrNoSuchVolume.Error()) {
noSuchVolumeErrors = true
} else if errors.Is(err, define.ErrVolumeBeingUsed) || strings.Contains(err.Error(), define.ErrVolumeBeingUsed.Error()) {
inUseErrors = true
}
}

switch {
case inUseErrors:
// being used by a container.
registry.SetExitCode(2)
case noSuchVolumeErrors && !inUseErrors:
// One of the specified volumes did not exist, and no other
if force {
registry.SetExitCode(define.ExecErrorCodeIgnore)
} else {
registry.SetExitCode(1)
}
default:
registry.SetExitCode(125)
}
}
2 changes: 2 additions & 0 deletions libpod/define/exec_codes.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const (
ExecErrorCodeCannotInvoke = 126
// ExecErrorCodeNotFound is the error code to return when a command cannot be found
ExecErrorCodeNotFound = 127
// ExecErrorCodeIgnore is the error code to can be simply ignored
ExecErrorCodeIgnore = -128
)

// TranslateExecErrorToExitCode takes an error and checks whether it
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,10 @@ var _ = Describe("Podman network", func() {
session := podmanTest.Podman([]string{"network", "rm", "bogus"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(1))

session = podmanTest.Podman([]string{"network", "rm", "-t", "0", "-f", "bogus"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(1))
})

It("podman network remove --force with pod", func() {
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/pod_rm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ var _ = Describe("Podman pod rm", func() {
session := podmanTest.Podman([]string{"pod", "rm", "bogus"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(1))

session = podmanTest.Podman([]string{"pod", "rm", "-t", "0", "-f", "bogus"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
})

It("podman rm bogus pod and a running pod", func() {
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/rm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,10 @@ var _ = Describe("Podman rm", func() {
session := podmanTest.Podman([]string{"rm", "bogus"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(1))

session = podmanTest.Podman([]string{"rm", "-t", "0", "-f", "bogus"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
})

It("podman rm bogus container and a running container", func() {
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/rmi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ var _ = Describe("Podman rmi", func() {
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(1))

session = podmanTest.Podman([]string{"rmi", "-f", "debian:6.0.10"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
})

It("podman rmi with fq name", func() {
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/volume_rm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ var _ = Describe("Podman volume rm", func() {
session := podmanTest.Podman([]string{"volume", "rm", "bogus"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(1))

session = podmanTest.Podman([]string{"volume", "rm", "-t", "0", "-f", "bogus"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
})

It("podman rm with --all flag", func() {
Expand Down