Skip to content

Commit 1bef5ec

Browse files
committed
disk: Switch to just invoking podman run
This works around a goroutine leak in the `Attach` API. Honestly I think it's also just simpler for our use case here; we don't need the container ID for example because it's ephemeral, etc. Signed-off-by: Colin Walters <[email protected]>
1 parent 6f883d9 commit 1bef5ec

File tree

1 file changed

+33
-104
lines changed

1 file changed

+33
-104
lines changed

pkg/bootc/bootc_disk.go

Lines changed: 33 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88
"io"
99
"os"
10+
"os/exec"
1011
"path/filepath"
1112
"strings"
1213
"sync"
@@ -20,11 +21,10 @@ import (
2021
"github.com/containers/podman/v5/pkg/bindings/containers"
2122
"github.com/containers/podman/v5/pkg/bindings/images"
2223
"github.com/containers/podman/v5/pkg/domain/entities/types"
23-
"github.com/containers/podman/v5/pkg/specgen"
2424
"github.com/docker/go-units"
25-
specs "github.com/opencontainers/runtime-spec/specs-go"
2625
"github.com/sirupsen/logrus"
2726
"golang.org/x/sys/unix"
27+
"golang.org/x/term"
2828
)
2929

3030
// As a baseline heuristic we double the size of
@@ -306,7 +306,7 @@ func (p *BootcDisk) pullImage() (err error) {
306306
}
307307

308308
// runInstallContainer runs the bootc installer in a container to create a disk image
309-
func (p *BootcDisk) runInstallContainer(quiet bool, config DiskImageConfig) (err error) {
309+
func (p *BootcDisk) runInstallContainer(quiet bool, config DiskImageConfig) error {
310310
// Create a temporary external shell script with the contents of our losetup wrapper
311311
losetupTemp, err := os.CreateTemp(p.Directory, "losetup-wrapper")
312312
if err != nil {
@@ -320,55 +320,17 @@ func (p *BootcDisk) runInstallContainer(quiet bool, config DiskImageConfig) (err
320320
return fmt.Errorf("temp losetup wrapper chmod: %w", err)
321321
}
322322

323-
createResponse, err := p.createInstallContainer(config, losetupTemp.Name())
324-
if err != nil {
325-
return fmt.Errorf("failed to create container: %w", err)
326-
}
327-
328-
p.bootcInstallContainerId = createResponse.ID //save the id for possible cleanup
329-
logrus.Debugf("Created install container, id=%s", createResponse.ID)
330-
331-
// run the container to create the disk
332-
err = containers.Start(p.Ctx, p.bootcInstallContainerId, &containers.StartOptions{})
333-
if err != nil {
334-
return fmt.Errorf("failed to start container: %w", err)
335-
}
336-
logrus.Debugf("Started install container")
337-
338-
// Ensure we've cancelled the container attachment when exiting this function, as
339-
// it takes over stdout/stderr handling
340-
attachCancelCtx, cancelAttach := context.WithCancel(p.Ctx)
341-
defer cancelAttach()
342-
var exitCode int32
343-
if !quiet {
344-
attachOpts := new(containers.AttachOptions).WithStream(true)
345-
if err := containers.Attach(attachCancelCtx, p.bootcInstallContainerId, os.Stdin, os.Stdout, os.Stderr, nil, attachOpts); err != nil {
346-
return fmt.Errorf("attaching: %w", err)
347-
}
323+
c := p.createInstallContainer(config, losetupTemp.Name())
324+
if err := c.Run(); err != nil {
325+
return fmt.Errorf("failed to invoke install: %w", err)
348326
}
349-
exitCode, err = containers.Wait(p.Ctx, p.bootcInstallContainerId, nil)
350-
if err != nil {
351-
return fmt.Errorf("failed to wait for container: %w", err)
352-
}
353-
354-
if exitCode != 0 {
355-
return fmt.Errorf("failed to run bootc install")
356-
}
357-
358-
return
327+
return nil
359328
}
360329

361-
// createInstallContainer creates a container to run the bootc installer
362-
func (p *BootcDisk) createInstallContainer(config DiskImageConfig, tempLosetup string) (createResponse types.ContainerCreateResponse, err error) {
363-
privileged := true
364-
autoRemove := true
365-
labelNested := true
366-
367-
targetEnv := make(map[string]string)
368-
if v, ok := os.LookupEnv("BOOTC_INSTALL_LOG"); ok {
369-
targetEnv["RUST_LOG"] = v
370-
}
371-
330+
// createInstallContainer creates a podman command to run the bootc installer.
331+
// Note: This code used to use the Go bindings for the podman remote client, but the
332+
// Attach interface currently leaks goroutines.
333+
func (p *BootcDisk) createInstallContainer(config DiskImageConfig, tempLosetup string) *exec.Cmd {
372334
bootcInstallArgs := []string{
373335
"bootc", "install", "to-disk", "--via-loopback", "--generic-image",
374336
"--skip-fetch-check",
@@ -381,60 +343,27 @@ func (p *BootcDisk) createInstallContainer(config DiskImageConfig, tempLosetup s
381343
}
382344
bootcInstallArgs = append(bootcInstallArgs, "/output/"+filepath.Base(p.file.Name()))
383345

384-
// Allocate pty so we can show progress bars, spinners etc.
385-
trueDat := true
386-
s := &specgen.SpecGenerator{
387-
ContainerBasicConfig: specgen.ContainerBasicConfig{
388-
Command: bootcInstallArgs,
389-
PidNS: specgen.Namespace{NSMode: specgen.Host},
390-
Remove: &autoRemove,
391-
Annotations: map[string]string{"io.podman.annotations.label": "type:unconfined_t"},
392-
Env: targetEnv,
393-
Terminal: &trueDat,
394-
},
395-
ContainerStorageConfig: specgen.ContainerStorageConfig{
396-
Image: p.ImageNameOrId,
397-
Mounts: []specs.Mount{
398-
{
399-
Source: "/var/lib/containers",
400-
Destination: "/var/lib/containers",
401-
Type: "bind",
402-
},
403-
{
404-
Source: "/dev",
405-
Destination: "/dev",
406-
Type: "bind",
407-
},
408-
{
409-
Source: p.Directory,
410-
Destination: "/output",
411-
Type: "bind",
412-
},
413-
{
414-
Source: tempLosetup,
415-
// Note that the default $PATH has /usr/local/sbin first
416-
Destination: "/usr/local/sbin/losetup",
417-
Type: "bind",
418-
Options: []string{"ro"},
419-
},
420-
},
421-
},
422-
ContainerSecurityConfig: specgen.ContainerSecurityConfig{
423-
Privileged: &privileged,
424-
LabelNested: &labelNested,
425-
SelinuxOpts: []string{"type:unconfined_t"},
426-
},
427-
ContainerNetworkConfig: specgen.ContainerNetworkConfig{
428-
NetNS: specgen.Namespace{
429-
NSMode: specgen.Bridge,
430-
},
431-
},
432-
}
433-
434-
createResponse, err = containers.CreateWithSpec(p.Ctx, s, &containers.CreateOptions{})
435-
if err != nil {
436-
return createResponse, fmt.Errorf("failed to create container: %w", err)
346+
// Basic config:
347+
// - force on --remote because we depend on podman machine.
348+
// - add privileged, pid=host, SELinux config and bind mounts per https://containers.github.io/bootc/bootc-install.html
349+
podmanArgs := []string{"--remote", "run", "--rm", "-i", "--pid=host", "--privileged", "--security-opt=label=type:unconfined_t", "--volume=/dev:/dev", "--volume=/var/lib/containers:/var/lib/containers"}
350+
// Custom bind mounts
351+
podmanArgs = append(podmanArgs, fmt.Sprintf("--volume=%s:/output", p.Directory), fmt.Sprintf("--volume=%s:/usr/local/sbin/losetup:ro", tempLosetup))
352+
if term.IsTerminal(int(os.Stdin.Fd())) {
353+
podmanArgs = append(podmanArgs, "-t")
437354
}
438-
439-
return
355+
// Other conditional arguments
356+
if v, ok := os.LookupEnv("BOOTC_INSTALL_LOG"); ok {
357+
podmanArgs = append(podmanArgs, fmt.Sprintf("--env=RUST_LOG=%s", v))
358+
}
359+
// The image name
360+
podmanArgs = append(podmanArgs, p.ImageNameOrId)
361+
// And the remaining arguments for bootc install
362+
podmanArgs = append(podmanArgs, bootcInstallArgs...)
363+
364+
c := exec.Command("podman", podmanArgs...)
365+
c.Stdin = os.Stdin
366+
c.Stdout = os.Stdout
367+
c.Stderr = os.Stderr
368+
return c
440369
}

0 commit comments

Comments
 (0)