Skip to content

Commit

Permalink
cmd/run: Optimize 'enter' and 'run' for already running containers
Browse files Browse the repository at this point in the history
Currently, the 'enter' and 'run' commands always invoke 'podman start'
even if the Toolbx container's entry point is already running.  There's
no need for that.  The commands already invoke 'podman inspect' to find
out if the org.freedesktop.Flatpak.SessionHelper D-Bus service needs to
be started.  Thus, they already have what is needed to find out if the
container is stopped and 'podman start' is necessary before it can be
used with 'podman exec', or if it's already running.

The unconditional 'podman start' invocation was followed by a second
'podman inspect' invocation to find out if the 'podman start' managed to
start the container's entry point.  There's no need for this second
'podman inspect' either, just like the 'podman start', when it's already
known from the first 'podman inspect' that the container is running.

The extra 'podman start' and 'podman inspect' invocations are
sufficiently expensive to add a noticeable overhead to the 'enter' and
'run' commands.  It's common to use a container that's already running,
just like having multiple terminals with the same working directory, and
terminal emulation applications like Ptyxis try to make it easier [1].
Therefore, it's worth optimizing this code path.

[1] https://gitlab.gnome.org/chergert/ptyxis
    https://flathub.org/apps/app.devsuite.Ptyxis

#1070
  • Loading branch information
debarshiray committed May 16, 2024
1 parent 77187f1 commit a33e656
Showing 1 changed file with 34 additions and 38 deletions.
72 changes: 34 additions & 38 deletions src/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,20 +237,16 @@ func runCommand(container string,
}
}

if err := callFlatpakSessionHelper(container); err != nil {
return err
}

logrus.Debugf("Starting container %s", container)
if err := startContainer(container); err != nil {
return err
}

entryPoint, entryPointPID, err := getEntryPointAndPID(container)
logrus.Debugf("Inspecting container %s", container)
containerObj, err := podman.InspectContainer(container)
if err != nil {
return err
return fmt.Errorf("failed to inspect container %s", container)
}

entryPoint := containerObj.EntryPoint()
entryPointPID := containerObj.EntryPointPID()
logrus.Debugf("Entry point of container %s is %s (PID=%d)", container, entryPoint, entryPointPID)

if entryPoint != "toolbox" {
var builder strings.Builder
fmt.Fprintf(&builder, "container %s is too old and no longer supported \n", container)
Expand All @@ -260,11 +256,31 @@ func runCommand(container string,
return errors.New(errMsg)
}

if entryPointPID <= 0 {
return fmt.Errorf("invalid entry point PID of container %s", container)
if err := callFlatpakSessionHelper(containerObj); err != nil {
return err
}

logrus.Debugf("Waiting for container %s to finish initializing", container)
if entryPointPID <= 0 {
logrus.Debugf("Starting container %s", container)
if err := startContainer(container); err != nil {
return err
}

logrus.Debugf("Inspecting container %s", container)
containerObj, err := podman.InspectContainer(container)
if err != nil {
return fmt.Errorf("failed to inspect container %s", container)
}

entryPointPID = containerObj.EntryPointPID()
logrus.Debugf("Entry point of container %s is %s (PID=%d)", container, entryPoint, entryPointPID)

if entryPointPID <= 0 {
return fmt.Errorf("invalid entry point PID of container %s", container)
}

logrus.Debugf("Waiting for container %s to finish initializing", container)
}

toolboxRuntimeDirectory, err := utils.GetRuntimeDirectory(currentUser)
if err != nil {
Expand Down Expand Up @@ -430,17 +446,13 @@ func runHelp(cmd *cobra.Command, args []string) {
}
}

func callFlatpakSessionHelper(container string) error {
logrus.Debugf("Inspecting mounts of container %s", container)

containerObj, err := podman.InspectContainer(container)
if err != nil {
return fmt.Errorf("failed to inspect entry point of container %s", container)
}
func callFlatpakSessionHelper(container podman.Container) error {
name := container.Name()
logrus.Debugf("Inspecting mounts of container %s", name)

var needsFlatpakSessionHelper bool

mounts := containerObj.Mounts()
mounts := container.Mounts()
for _, mount := range mounts {
if mount == "/run/host/monitor" {
logrus.Debug("Requires org.freedesktop.Flatpak.SessionHelper")
Expand Down Expand Up @@ -522,22 +534,6 @@ func constructExecArgs(container, preserveFDs string,
return execArgs
}

func getEntryPointAndPID(container string) (string, int, error) {
logrus.Debugf("Inspecting entry point of container %s", container)

containerObj, err := podman.InspectContainer(container)
if err != nil {
return "", 0, fmt.Errorf("failed to inspect entry point of container %s", container)
}

entryPoint := containerObj.EntryPoint()
entryPointPID := containerObj.EntryPointPID()

logrus.Debugf("Entry point of container %s is %s (PID=%d)", container, entryPoint, entryPointPID)

return entryPoint, entryPointPID, nil
}

func isCommandPresent(container, command string) (bool, error) {
logrus.Debugf("Looking up command %s in container %s", command, container)

Expand Down

0 comments on commit a33e656

Please sign in to comment.