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

Add podman rm --depend #12694

Merged
merged 1 commit into from
Jan 11, 2022
Merged
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
3 changes: 2 additions & 1 deletion cmd/podman/containers/rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ func rmFlags(cmd *cobra.Command) {

flags.BoolVarP(&rmOptions.All, "all", "a", false, "Remove all containers")
flags.BoolVarP(&rmOptions.Ignore, "ignore", "i", false, "Ignore errors when a specified container is missing")
flags.BoolVarP(&rmOptions.Force, "force", "f", false, "Force removal of a running or unusable container. The default is false")
flags.BoolVarP(&rmOptions.Force, "force", "f", false, "Force removal of a running or unusable container")
flags.BoolVar(&rmOptions.Depend, "depend", false, "Remove container and all containers that depend on the selected container")
timeFlagName := "time"
flags.UintVarP(&stopTimeout, timeFlagName, "t", containerConfig.Engine.StopTimeout, "Seconds to wait for stop before killing the container")
_ = cmd.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone)
Expand Down
4 changes: 2 additions & 2 deletions cmd/podman/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func persistentPreRunE(cmd *cobra.Command, args []string) error {
runtimeFlag := cmd.Root().Flags().Lookup("runtime")
if runtimeFlag == nil {
return errors.Errorf(
"Unexcpected error setting runtime to '%s' for restore",
"setting runtime to '%s' for restore",
*runtime,
)
}
Expand Down Expand Up @@ -217,7 +217,7 @@ func persistentPreRunE(cmd *cobra.Command, args []string) error {

context := cmd.Root().LocalFlags().Lookup("context")
if context.Value.String() != "default" {
return errors.New("Podman does not support swarm, the only --context value allowed is \"default\"")
return errors.New("podman does not support swarm, the only --context value allowed is \"default\"")
}
if !registry.IsRemote() {
if cmd.Flag("cpu-profile").Changed {
Expand Down
2 changes: 1 addition & 1 deletion cmd/podman/utils/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (o OutputErrors) PrintErrors() (lastError error) {
instead returns a message and we cast it to a new error.

Following function performs parsing on build error and returns
exit status which was exepected for this current build
exit status which was expected for this current build
*/
func ExitCodeFromBuildError(errorMsg string) (int, error) {
if strings.Contains(errorMsg, "exit status") {
Expand Down
4 changes: 2 additions & 2 deletions cmd/winpath/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func addPathToRegistry(dir string) error {
return err
}

// Removes all occurences of a directory path from the Windows path stored in the registry
// Removes all occurrences of a directory path from the Windows path stored in the registry
func removePathFromRegistry(path string) error {
k, err := registry.OpenKey(registry.CURRENT_USER, Environment, registry.READ|registry.WRITE)
if err != nil {
Expand Down Expand Up @@ -155,7 +155,7 @@ func removePathFromRegistry(path string) error {
return err
}

// Sends a notification message to all top level windows informing them the environmental setings have changed.
// Sends a notification message to all top level windows informing them the environmental settings have changed.
// Applications such as the Windows command prompt and powershell will know to stop caching stale values on
// subsequent restarts. Since applications block the sender when receiving a message, we set a 3 second timeout
func broadcastEnvironmentChange() {
Expand Down
9 changes: 9 additions & 0 deletions docs/source/markdown/podman-rm.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ Running or unusable containers will not be removed without the **-f** option.

Remove all containers. Can be used in conjunction with **-f** as well.

#### **--depend**

Remove selected container and recursively remove all containers that depend on it.

#### **--cidfile**

Read container ID from the specified file and remove the container. Can be specified multiple times.
Expand Down Expand Up @@ -56,6 +60,11 @@ Remove a container by its name *mywebserver*
$ podman rm mywebserver
```

Remove a *mywebserver* container and all of the containers that depend on it
```
$ podman rm --depend mywebserver
Copy link
Member

Choose a reason for hiding this comment

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

Does this print every container that was removed? I got a little concerned because there are no changes to cmd/podman

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the container is added to the reports list when being removed in the API. The Podman client just walks the list.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a test to verify.

```

Remove several containers by name and container id.
```
$ podman rm mywebserver myflaskserver 860a4b23
Expand Down
31 changes: 31 additions & 0 deletions libpod/runtime_ctr.go
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,37 @@ func (r *Runtime) evictContainer(ctx context.Context, idOrName string, removeVol
return id, cleanupErr
}

// RemoveDepend removes all dependencies for a container
func (r *Runtime) RemoveDepend(ctx context.Context, rmCtr *Container, force bool, removeVolume bool, timeout *uint) ([]*reports.RmReport, error) {
rmReports := make([]*reports.RmReport, 0)
deps, err := r.state.ContainerInUse(rmCtr)
if err != nil {
if err == define.ErrCtrRemoved {
return rmReports, nil
}
return rmReports, err
}
for _, cid := range deps {
ctr, err := r.state.Container(cid)
if err != nil {
if err == define.ErrNoSuchCtr {
continue
}
return rmReports, err
}

reports, err := r.RemoveDepend(ctx, ctr, force, removeVolume, timeout)
if err != nil {
return rmReports, err
}
rmReports = append(rmReports, reports...)
}
report := reports.RmReport{Id: rmCtr.ID()}
report.Err = r.removeContainer(ctx, rmCtr, force, removeVolume, false, timeout)

return append(rmReports, &report), nil
}

// GetContainer retrieves a container by its ID
func (r *Runtime) GetContainer(id string) (*Container, error) {
r.lock.RLock()
Expand Down
13 changes: 9 additions & 4 deletions pkg/api/handlers/compat/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) {
query := struct {
Force bool `schema:"force"`
Ignore bool `schema:"ignore"`
Depend bool `schema:"depend"`
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want this in the compat API, the return type won't make it clear that multiple containers were removed. Having it only in the fancy version that can already to multiple containers seems better?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is only handled in libpod not in compat.

Link bool `schema:"link"`
Timeout *uint `schema:"timeout"`
DockerVolumes bool `schema:"v"`
Expand All @@ -57,6 +58,7 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) {
if utils.IsLibpodRequest(r) {
options.Volumes = query.LibpodVolumes
options.Timeout = query.Timeout
options.Depend = query.Depend
Copy link
Member

Choose a reason for hiding this comment

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

Does this handle returns properly? As in, if we remove >1 container, we tell the server that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes it should return all of the containers.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a test to verify.

} else {
if query.Link {
utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
Expand All @@ -71,7 +73,7 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) {
// code.
containerEngine := abi.ContainerEngine{Libpod: runtime}
name := utils.GetName(r)
report, err := containerEngine.ContainerRm(r.Context(), []string{name}, options)
reports, err := containerEngine.ContainerRm(r.Context(), []string{name}, options)
if err != nil {
if errors.Cause(err) == define.ErrNoSuchCtr {
utils.ContainerNotFound(w, name, err)
Expand All @@ -81,16 +83,19 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) {
utils.InternalServerError(w, err)
return
}
if len(report) > 0 && report[0].Err != nil {
err = report[0].Err
if len(reports) > 0 && reports[0].Err != nil {
err = reports[0].Err
if errors.Cause(err) == define.ErrNoSuchCtr {
utils.ContainerNotFound(w, name, err)
return
}
utils.InternalServerError(w, err)
return
}

if utils.IsLibpodRequest(r) {
utils.WriteResponse(w, http.StatusOK, reports)
return
}
utils.WriteResponse(w, http.StatusNoContent, nil)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/api/handlers/compat/images_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {

// if layers field not set assume its not from a valid podman-client
// could be a docker client, set `layers=true` since that is the default
// expected behviour
// expected behaviour
if !utils.IsLibpodRequest(r) {
if _, found := r.URL.Query()["layers"]; !found {
query.Layers = true
Expand Down
7 changes: 7 additions & 0 deletions pkg/api/handlers/swagger/swagger.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ type swagLibpodInspectImageResponse struct {
}
}

// Rm containers
// swagger:response DocsLibpodContainerRmReport
type swagLibpodContainerRmReport struct {
// in: body
Body []handlers.LibpodContainersRmReport
}

// Prune containers
// swagger:response DocsContainerPruneReport
type swagContainerPruneReport struct {
Expand Down
11 changes: 11 additions & 0 deletions pkg/api/handlers/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,17 @@ type LibpodContainersPruneReport struct {
PruneError string `json:"Err,omitempty"`
}

type LibpodContainersRmReport struct {
ID string `json:"Id"`
// Error which occurred during Rm operation (if any).
// This field is optional and may be omitted if no error occurred.
//
// Extensions:
// x-omitempty: true
// x-nullable: true
RmError string `json:"Err,omitempty"`
}

type Info struct {
docker.Info
BuildahVersion string
Expand Down
17 changes: 16 additions & 1 deletion pkg/api/server/register_containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -817,16 +817,31 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error {
// required: true
// description: the name or ID of the container
// - in: query
// name: depend
// type: boolean
// description: additionally remove containers that depend on the container to be removed
// - in: query
// name: force
// type: boolean
// description: need something
// description: force stop container if running
// - in: query
// name: ignore
// type: boolean
// description: ignore errors when the container to be removed does not existxo
// - in: query
// name: timeout
// type: integer
// default: 10
// description: number of seconds to wait before killing container when force removing
// - in: query
// name: v
// type: boolean
// description: delete volumes
// produces:
// - application/json
// responses:
// 200:
// $ref: "#/responses/DocsLibpodContainerRmReport"
// 204:
// description: no error
// 400:
Expand Down
11 changes: 6 additions & 5 deletions pkg/bindings/containers/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,25 +78,26 @@ func Prune(ctx context.Context, options *PruneOptions) ([]*reports.PruneReport,
// The volumes bool dictates that a container's volumes should also be removed.
// The All option indicates that all containers should be removed
// The Ignore option indicates that if a container did not exist, ignore the error
func Remove(ctx context.Context, nameOrID string, options *RemoveOptions) error {
func Remove(ctx context.Context, nameOrID string, options *RemoveOptions) ([]*reports.RmReport, error) {
if options == nil {
options = new(RemoveOptions)
}
var reports []*reports.RmReport
conn, err := bindings.GetClient(ctx)
if err != nil {
return err
return reports, err
}
params, err := options.ToParams()
if err != nil {
return err
return reports, err
}
response, err := conn.DoRequest(ctx, nil, http.MethodDelete, "/containers/%s", params, nil, nameOrID)
if err != nil {
return err
return reports, err
}
defer response.Body.Close()

return response.Process(nil)
return reports, response.Process(&reports)
}

// Inspect returns low level information about a Container. The nameOrID can be a container name
Expand Down
1 change: 1 addition & 0 deletions pkg/bindings/containers/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ type PruneOptions struct {
//go:generate go run ../generator/generator.go RemoveOptions
// RemoveOptions are optional options for removing containers
type RemoveOptions struct {
Depend *bool
Copy link
Member

Choose a reason for hiding this comment

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

Also needs changes for the handlers and swagger.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

Ignore *bool
Force *bool
Volumes *bool
Expand Down
15 changes: 15 additions & 0 deletions pkg/bindings/containers/types_remove_options.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading