diff options
Diffstat (limited to 'pkg')
-rw-r--r-- | pkg/adapter/containers.go | 21 | ||||
-rw-r--r-- | pkg/adapter/runtime_remote.go | 8 | ||||
-rw-r--r-- | pkg/adapter/sigproxy_linux.go | 4 | ||||
-rw-r--r-- | pkg/adapter/terminal_linux.go | 4 | ||||
-rw-r--r-- | pkg/cgroups/blkio.go | 4 | ||||
-rw-r--r-- | pkg/cgroups/cgroups.go | 93 | ||||
-rw-r--r-- | pkg/cgroups/cpu.go | 19 | ||||
-rw-r--r-- | pkg/cgroups/cpuset.go | 22 | ||||
-rw-r--r-- | pkg/cgroups/memory.go | 5 | ||||
-rw-r--r-- | pkg/cgroups/pids.go | 6 | ||||
-rw-r--r-- | pkg/channelwriter/channelwriter.go | 34 | ||||
-rw-r--r-- | pkg/errorhandling/errorhandling.go | 23 | ||||
-rw-r--r-- | pkg/firewall/firewalld.go | 5 | ||||
-rw-r--r-- | pkg/firewall/iptables.go | 9 | ||||
-rw-r--r-- | pkg/hooks/exec/exec.go | 5 | ||||
-rw-r--r-- | pkg/logs/logs.go | 7 | ||||
-rw-r--r-- | pkg/netns/netns_linux.go | 4 | ||||
-rw-r--r-- | pkg/rootless/rootless_linux.go | 54 | ||||
-rw-r--r-- | pkg/spec/spec.go | 8 | ||||
-rw-r--r-- | pkg/systemdgen/systemdgen.go | 28 | ||||
-rw-r--r-- | pkg/systemdgen/systemdgen_test.go | 18 | ||||
-rw-r--r-- | pkg/util/utils.go | 16 | ||||
-rw-r--r-- | pkg/util/utils_supported.go | 9 | ||||
-rw-r--r-- | pkg/varlinkapi/images.go | 108 | ||||
-rw-r--r-- | pkg/varlinkapi/util.go | 40 |
25 files changed, 401 insertions, 153 deletions
diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go index 10720886b..1cf9d686a 100644 --- a/pkg/adapter/containers.go +++ b/pkg/adapter/containers.go @@ -95,8 +95,8 @@ func (r *LocalRuntime) StopContainers(ctx context.Context, cli *cliconfig.StopVa } pool.Add(shared.Job{ - c.ID(), - func() error { + ID: c.ID(), + Fn: func() error { err := c.StopWithTimeout(*timeout) if err != nil { if errors.Cause(err) == define.ErrCtrStopped { @@ -134,8 +134,8 @@ func (r *LocalRuntime) KillContainers(ctx context.Context, cli *cliconfig.KillVa c := c pool.Add(shared.Job{ - c.ID(), - func() error { + ID: c.ID(), + Fn: func() error { return c.Kill(uint(signal)) }, }) @@ -163,8 +163,8 @@ func (r *LocalRuntime) InitContainers(ctx context.Context, cli *cliconfig.InitVa ctr := c pool.Add(shared.Job{ - ctr.ID(), - func() error { + ID: ctr.ID(), + Fn: func() error { err := ctr.Init(ctx) if err != nil { // If we're initializing all containers, ignore invalid state errors @@ -1058,7 +1058,14 @@ func (r *LocalRuntime) GenerateSystemd(c *cliconfig.GenerateSystemdValues) (stri if c.Name { name = ctr.Name() } - return systemdgen.CreateSystemdUnitAsString(name, ctr.ID(), c.RestartPolicy, ctr.Config().StaticDir, timeout) + + config := ctr.Config() + conmonPidFile := config.ConmonPidFile + if conmonPidFile == "" { + return "", errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") + } + + return systemdgen.CreateSystemdUnitAsString(name, ctr.ID(), c.RestartPolicy, conmonPidFile, timeout) } // GetNamespaces returns namespace information about a container for PS diff --git a/pkg/adapter/runtime_remote.go b/pkg/adapter/runtime_remote.go index 800ed7569..db3f23629 100644 --- a/pkg/adapter/runtime_remote.go +++ b/pkg/adapter/runtime_remote.go @@ -97,6 +97,14 @@ func GetRuntime(ctx context.Context, c *cliconfig.PodmanCommand) (*LocalRuntime, }, nil } +// DeferredShutdown is a bogus wrapper for compaat with the libpod +// runtime and should only be run when a defer is being used +func (r RemoteRuntime) DeferredShutdown(force bool) { + if err := r.Shutdown(force); err != nil { + logrus.Error("unable to shutdown runtime") + } +} + // Shutdown is a bogus wrapper for compat with the libpod runtime func (r RemoteRuntime) Shutdown(force bool) error { return nil diff --git a/pkg/adapter/sigproxy_linux.go b/pkg/adapter/sigproxy_linux.go index af968cb89..efa6afa7b 100644 --- a/pkg/adapter/sigproxy_linux.go +++ b/pkg/adapter/sigproxy_linux.go @@ -27,7 +27,9 @@ func ProxySignals(ctr *libpod.Container) { if err := ctr.Kill(uint(s.(syscall.Signal))); err != nil { logrus.Errorf("Error forwarding signal %d to container %s: %v", s, ctr.ID(), err) signal.StopCatch(sigBuffer) - syscall.Kill(syscall.Getpid(), s.(syscall.Signal)) + if err := syscall.Kill(syscall.Getpid(), s.(syscall.Signal)); err != nil { + logrus.Errorf("failed to kill pid %d", syscall.Getpid()) + } } } }() diff --git a/pkg/adapter/terminal_linux.go b/pkg/adapter/terminal_linux.go index 3c4c3bd38..be7dc0cb6 100644 --- a/pkg/adapter/terminal_linux.go +++ b/pkg/adapter/terminal_linux.go @@ -35,7 +35,9 @@ func StartAttachCtr(ctx context.Context, ctr *libpod.Container, stdout, stderr, } logrus.SetFormatter(&RawTtyFormatter{}) - term.SetRawTerminal(os.Stdin.Fd()) + if _, err := term.SetRawTerminal(os.Stdin.Fd()); err != nil { + return err + } defer restoreTerminal(oldTermState) } diff --git a/pkg/cgroups/blkio.go b/pkg/cgroups/blkio.go index ca9107d97..bacd4eb93 100644 --- a/pkg/cgroups/blkio.go +++ b/pkg/cgroups/blkio.go @@ -30,14 +30,14 @@ func (c *blkioHandler) Apply(ctr *CgroupControl, res *spec.LinuxResources) error // Create the cgroup func (c *blkioHandler) Create(ctr *CgroupControl) (bool, error) { if ctr.cgroup2 { - return false, fmt.Errorf("io create not implemented for cgroup v2") + return false, nil } return ctr.createCgroupDirectory(Blkio) } // Destroy the cgroup func (c *blkioHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(Blkio)) + return rmDirRecursively(ctr.getCgroupv1Path(Blkio)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/cgroups/cgroups.go b/pkg/cgroups/cgroups.go index d6c19212b..081db772f 100644 --- a/pkg/cgroups/cgroups.go +++ b/pkg/cgroups/cgroups.go @@ -149,6 +149,51 @@ func (c *CgroupControl) getCgroupv1Path(name string) string { return filepath.Join(cgroupRoot, name, c.path) } +// createCgroupv2Path creates the cgroupv2 path and enables all the available controllers +func createCgroupv2Path(path string) (Err error) { + content, err := ioutil.ReadFile("/sys/fs/cgroup/cgroup.controllers") + if err != nil { + return errors.Wrapf(err, "read /sys/fs/cgroup/cgroup.controllers") + } + if !filepath.HasPrefix(path, "/sys/fs/cgroup") { + return fmt.Errorf("invalid cgroup path %s", path) + } + + res := "" + for i, c := range strings.Split(strings.TrimSpace(string(content)), " ") { + if i == 0 { + res = fmt.Sprintf("+%s", c) + } else { + res = res + fmt.Sprintf(" +%s", c) + } + } + resByte := []byte(res) + + current := "/sys/fs" + elements := strings.Split(path, "/") + for i, e := range elements[3:] { + current = filepath.Join(current, e) + if i > 0 { + if err := os.Mkdir(current, 0755); err != nil { + if !os.IsExist(err) { + return errors.Wrapf(err, "mkdir %s", path) + } + } else { + // If the directory was created, be sure it is not left around on errors. + defer func() { + if Err != nil { + os.Remove(current) + } + }() + } + } + if err := ioutil.WriteFile(filepath.Join(current, "cgroup.subtree_control"), resByte, 0755); err != nil { + return errors.Wrapf(err, "write %s", filepath.Join(current, "cgroup.subtree_control")) + } + } + return nil +} + // initialize initializes the specified hierarchy func (c *CgroupControl) initialize() (err error) { createdSoFar := map[string]controllerHandler{} @@ -161,6 +206,11 @@ func (c *CgroupControl) initialize() (err error) { } } }() + if c.cgroup2 { + if err := createCgroupv2Path(filepath.Join(cgroupRoot, c.path)); err != nil { + return errors.Wrapf(err, "error creating cgroup path %s", c.path) + } + } for name, handler := range handlers { created, err := handler.Create(c) if err != nil { @@ -278,6 +328,13 @@ func Load(path string) (*CgroupControl, error) { systemd: false, } if !cgroup2 { + controllers, err := getAvailableControllers(handlers, false) + if err != nil { + return nil, err + } + control.additionalControllers = controllers + } + if !cgroup2 { for name := range handlers { p := control.getCgroupv1Path(name) if _, err := os.Stat(p); err != nil { @@ -305,11 +362,40 @@ func (c *CgroupControl) Delete() error { return c.DeleteByPath(c.path) } +// rmDirRecursively delete recursively a cgroup directory. +// It differs from os.RemoveAll as it doesn't attempt to unlink files. +// On cgroupfs we are allowed only to rmdir empty directories. +func rmDirRecursively(path string) error { + if err := os.Remove(path); err == nil || os.IsNotExist(err) { + return nil + } + entries, err := ioutil.ReadDir(path) + if err != nil { + return errors.Wrapf(err, "read %s", path) + } + for _, i := range entries { + if i.IsDir() { + if err := rmDirRecursively(filepath.Join(path, i.Name())); err != nil { + return err + } + } + } + if os.Remove(path); err != nil { + if !os.IsNotExist(err) { + return errors.Wrapf(err, "remove %s", path) + } + } + return nil +} + // DeleteByPath deletes the specified cgroup path func (c *CgroupControl) DeleteByPath(path string) error { if c.systemd { return systemdDestroy(path) } + if c.cgroup2 { + return rmDirRecursively(filepath.Join(cgroupRoot, c.path)) + } var lastError error for _, h := range handlers { if err := h.Destroy(c); err != nil { @@ -318,8 +404,11 @@ func (c *CgroupControl) DeleteByPath(path string) error { } for _, ctr := range c.additionalControllers { + if ctr.symlink { + continue + } p := c.getCgroupv1Path(ctr.name) - if err := os.Remove(p); err != nil { + if err := rmDirRecursively(p); err != nil { lastError = errors.Wrapf(err, "remove %s", p) } } @@ -341,7 +430,7 @@ func (c *CgroupControl) AddPid(pid int) error { pidString := []byte(fmt.Sprintf("%d\n", pid)) if c.cgroup2 { - p := filepath.Join(cgroupRoot, c.path, "tasks") + p := filepath.Join(cgroupRoot, c.path, "cgroup.procs") if err := ioutil.WriteFile(p, pidString, 0644); err != nil { return errors.Wrapf(err, "write %s", p) } diff --git a/pkg/cgroups/cpu.go b/pkg/cgroups/cpu.go index 8640d490e..03677f1ef 100644 --- a/pkg/cgroups/cpu.go +++ b/pkg/cgroups/cpu.go @@ -61,14 +61,14 @@ func (c *cpuHandler) Apply(ctr *CgroupControl, res *spec.LinuxResources) error { // Create the cgroup func (c *cpuHandler) Create(ctr *CgroupControl) (bool, error) { if ctr.cgroup2 { - return false, fmt.Errorf("cpu create not implemented for cgroup v2") + return false, nil } return ctr.createCgroupDirectory(CPU) } // Destroy the cgroup func (c *cpuHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(CPU)) + return rmDirRecursively(ctr.getCgroupv1Path(CPU)) } // Stat fills a metrics structure with usage stats for the controller @@ -98,15 +98,24 @@ func (c *cpuHandler) Stat(ctr *CgroupControl, m *Metrics) error { } else { usage.Total, err = readAcct(ctr, "cpuacct.usage") if err != nil { - return err + if !os.IsNotExist(errors.Cause(err)) { + return err + } + usage.Total = 0 } usage.Kernel, err = readAcct(ctr, "cpuacct.usage_sys") if err != nil { - return err + if !os.IsNotExist(errors.Cause(err)) { + return err + } + usage.Kernel = 0 } usage.PerCPU, err = readAcctList(ctr, "cpuacct.usage_percpu") if err != nil { - return err + if !os.IsNotExist(errors.Cause(err)) { + return err + } + usage.PerCPU = nil } } m.CPU = CPUMetrics{Usage: usage} diff --git a/pkg/cgroups/cpuset.go b/pkg/cgroups/cpuset.go index 9aef493c9..46d0484f2 100644 --- a/pkg/cgroups/cpuset.go +++ b/pkg/cgroups/cpuset.go @@ -3,7 +3,6 @@ package cgroups import ( "fmt" "io/ioutil" - "os" "path/filepath" "strings" @@ -14,19 +13,23 @@ import ( type cpusetHandler struct { } -func cpusetCopyFileFromParent(dir, file string) ([]byte, error) { +func cpusetCopyFileFromParent(dir, file string, cgroupv2 bool) ([]byte, error) { if dir == cgroupRoot { return nil, fmt.Errorf("could not find parent to initialize cpuset %s", file) } path := filepath.Join(dir, file) - data, err := ioutil.ReadFile(path) + parentPath := path + if cgroupv2 { + parentPath = fmt.Sprintf("%s.effective", parentPath) + } + data, err := ioutil.ReadFile(parentPath) if err != nil { return nil, errors.Wrapf(err, "open %s", path) } if len(strings.Trim(string(data), "\n")) != 0 { return data, nil } - data, err = cpusetCopyFileFromParent(filepath.Dir(dir), file) + data, err = cpusetCopyFileFromParent(filepath.Dir(dir), file, cgroupv2) if err != nil { return nil, err } @@ -36,9 +39,9 @@ func cpusetCopyFileFromParent(dir, file string) ([]byte, error) { return data, nil } -func cpusetCopyFromParent(path string) error { +func cpusetCopyFromParent(path string, cgroupv2 bool) error { for _, file := range []string{"cpuset.cpus", "cpuset.mems"} { - if _, err := cpusetCopyFileFromParent(path, file); err != nil { + if _, err := cpusetCopyFileFromParent(path, file, cgroupv2); err != nil { return err } } @@ -60,19 +63,20 @@ func (c *cpusetHandler) Apply(ctr *CgroupControl, res *spec.LinuxResources) erro // Create the cgroup func (c *cpusetHandler) Create(ctr *CgroupControl) (bool, error) { if ctr.cgroup2 { - return false, fmt.Errorf("cpuset create not implemented for cgroup v2") + path := filepath.Join(cgroupRoot, ctr.path) + return true, cpusetCopyFromParent(path, true) } created, err := ctr.createCgroupDirectory(CPUset) if !created || err != nil { return created, err } - return true, cpusetCopyFromParent(ctr.getCgroupv1Path(CPUset)) + return true, cpusetCopyFromParent(ctr.getCgroupv1Path(CPUset), false) } // Destroy the cgroup func (c *cpusetHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(CPUset)) + return rmDirRecursively(ctr.getCgroupv1Path(CPUset)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/cgroups/memory.go b/pkg/cgroups/memory.go index 0505eac40..b3991f7e3 100644 --- a/pkg/cgroups/memory.go +++ b/pkg/cgroups/memory.go @@ -2,7 +2,6 @@ package cgroups import ( "fmt" - "os" "path/filepath" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -26,14 +25,14 @@ func (c *memHandler) Apply(ctr *CgroupControl, res *spec.LinuxResources) error { // Create the cgroup func (c *memHandler) Create(ctr *CgroupControl) (bool, error) { if ctr.cgroup2 { - return false, fmt.Errorf("memory create not implemented for cgroup v2") + return false, nil } return ctr.createCgroupDirectory(Memory) } // Destroy the cgroup func (c *memHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(Memory)) + return rmDirRecursively(ctr.getCgroupv1Path(Memory)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/cgroups/pids.go b/pkg/cgroups/pids.go index c90dc1c02..65b9b5b34 100644 --- a/pkg/cgroups/pids.go +++ b/pkg/cgroups/pids.go @@ -3,7 +3,6 @@ package cgroups import ( "fmt" "io/ioutil" - "os" "path/filepath" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -35,15 +34,12 @@ func (c *pidHandler) Apply(ctr *CgroupControl, res *spec.LinuxResources) error { // Create the cgroup func (c *pidHandler) Create(ctr *CgroupControl) (bool, error) { - if ctr.cgroup2 { - return false, fmt.Errorf("pid create not implemented for cgroup v2") - } return ctr.createCgroupDirectory(Pids) } // Destroy the cgroup func (c *pidHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(Pids)) + return rmDirRecursively(ctr.getCgroupv1Path(Pids)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/channelwriter/channelwriter.go b/pkg/channelwriter/channelwriter.go new file mode 100644 index 000000000..d51400eb3 --- /dev/null +++ b/pkg/channelwriter/channelwriter.go @@ -0,0 +1,34 @@ +package channelwriter + +import "github.com/pkg/errors" + +// Writer is an io.writer-like object that "writes" to a channel +// instead of a buffer or file, etc. It is handy for varlink endpoints when +// needing to handle endpoints that do logging "real-time" +type Writer struct { + ByteChannel chan []byte +} + +// NewChannelWriter creates a new channel writer and adds a +// byte slice channel into it. +func NewChannelWriter() *Writer { + byteChannel := make(chan []byte) + return &Writer{ + ByteChannel: byteChannel, + } +} + +// Write method for Writer +func (c *Writer) Write(w []byte) (int, error) { + if c.ByteChannel == nil { + return 0, errors.New("channel writer channel cannot be nil") + } + c.ByteChannel <- w + return len(w), nil +} + +// Close method for Writer +func (c *Writer) Close() error { + close(c.ByteChannel) + return nil +} diff --git a/pkg/errorhandling/errorhandling.go b/pkg/errorhandling/errorhandling.go new file mode 100644 index 000000000..970d47636 --- /dev/null +++ b/pkg/errorhandling/errorhandling.go @@ -0,0 +1,23 @@ +package errorhandling + +import ( + "os" + + "github.com/sirupsen/logrus" +) + +// SyncQuiet syncs a file and logs any error. Should only be used within +// a defer. +func SyncQuiet(f *os.File) { + if err := f.Sync(); err != nil { + logrus.Errorf("unable to sync file %s: %q", f.Name(), err) + } +} + +// CloseQuiet closes a file and logs any error. Should only be used within +// a defer. +func CloseQuiet(f *os.File) { + if err := f.Close(); err != nil { + logrus.Errorf("unable to close file %s: %q", f.Name(), err) + } +} diff --git a/pkg/firewall/firewalld.go b/pkg/firewall/firewalld.go index 32c2337a0..15e845cb7 100644 --- a/pkg/firewall/firewalld.go +++ b/pkg/firewall/firewalld.go @@ -18,6 +18,7 @@ package firewall import ( "fmt" + "github.com/sirupsen/logrus" "strings" "github.com/godbus/dbus" @@ -113,7 +114,9 @@ func (fb *fwdBackend) Del(conf *FirewallNetConf) error { // Remove firewalld rules which assigned the given source IP to the given zone firewalldObj := fb.conn.Object(firewalldName, firewalldPath) var res string - firewalldObj.Call(firewalldZoneInterface+"."+firewalldRemoveSourceMethod, 0, getFirewalldZone(conf), ipStr).Store(&res) + if err := firewalldObj.Call(firewalldZoneInterface+"."+firewalldRemoveSourceMethod, 0, getFirewalldZone(conf), ipStr).Store(&res); err != nil { + logrus.Errorf("unable to store firewallobj") + } } return nil } diff --git a/pkg/firewall/iptables.go b/pkg/firewall/iptables.go index 59d81b287..92d249f7b 100644 --- a/pkg/firewall/iptables.go +++ b/pkg/firewall/iptables.go @@ -21,6 +21,7 @@ package firewall import ( "fmt" + "github.com/sirupsen/logrus" "net" "github.com/coreos/go-iptables/iptables" @@ -53,7 +54,9 @@ func generateFilterRule(privChainName string) []string { func cleanupRules(ipt *iptables.IPTables, privChainName string, rules [][]string) { for _, rule := range rules { - ipt.Delete("filter", privChainName, rule...) + if err := ipt.Delete("filter", privChainName, rule...); err != nil { + logrus.Errorf("failed to delete iptables rule %s", privChainName) + } } } @@ -185,7 +188,9 @@ func (ib *iptablesBackend) Add(conf *FirewallNetConf) error { func (ib *iptablesBackend) Del(conf *FirewallNetConf) error { for proto, ipt := range ib.protos { - ib.delRules(conf, ipt, proto) + if err := ib.delRules(conf, ipt, proto); err != nil { + logrus.Errorf("failed to delete iptables backend rule %s", conf.IptablesAdminChainName) + } } return nil } diff --git a/pkg/hooks/exec/exec.go b/pkg/hooks/exec/exec.go index 0dd091561..4038e3d94 100644 --- a/pkg/hooks/exec/exec.go +++ b/pkg/hooks/exec/exec.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "fmt" + "github.com/sirupsen/logrus" "io" osexec "os/exec" "time" @@ -54,7 +55,9 @@ func Run(ctx context.Context, hook *rspec.Hook, state []byte, stdout io.Writer, case err = <-exit: return err, err case <-ctx.Done(): - cmd.Process.Kill() + if err := cmd.Process.Kill(); err != nil { + logrus.Errorf("failed to kill pid %v", cmd.Process) + } timer := time.NewTimer(postKillTimeout) defer timer.Stop() select { diff --git a/pkg/logs/logs.go b/pkg/logs/logs.go index 0f684750e..89e4e5686 100644 --- a/pkg/logs/logs.go +++ b/pkg/logs/logs.go @@ -30,6 +30,7 @@ import ( "github.com/containers/libpod/libpod" "github.com/containers/libpod/libpod/define" + "github.com/containers/libpod/pkg/errorhandling" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -153,7 +154,7 @@ func ReadLogs(logPath string, ctr *libpod.Container, opts *LogOptions) error { if err != nil { return errors.Wrapf(err, "failed to open log file %q", logPath) } - defer file.Close() + defer errorhandling.CloseQuiet(file) msg := &logMessage{} opts.bytes = -1 @@ -161,9 +162,9 @@ func ReadLogs(logPath string, ctr *libpod.Container, opts *LogOptions) error { reader := bufio.NewReader(file) if opts.Follow { - followLog(reader, writer, opts, ctr, msg, logPath) + err = followLog(reader, writer, opts, ctr, msg, logPath) } else { - dumpLog(reader, writer, opts, msg, logPath) + err = dumpLog(reader, writer, opts, msg, logPath) } return err } diff --git a/pkg/netns/netns_linux.go b/pkg/netns/netns_linux.go index a72a2d098..4a515c72a 100644 --- a/pkg/netns/netns_linux.go +++ b/pkg/netns/netns_linux.go @@ -83,7 +83,9 @@ func NewNS() (ns.NetNS, error) { if err != nil { return nil, err } - mountPointFd.Close() + if err := mountPointFd.Close(); err != nil { + return nil, err + } // Ensure the mount point is cleaned up on errors; if the namespace // was successfully mounted this will have no effect because the file diff --git a/pkg/rootless/rootless_linux.go b/pkg/rootless/rootless_linux.go index 8028a359c..d7c2de81d 100644 --- a/pkg/rootless/rootless_linux.go +++ b/pkg/rootless/rootless_linux.go @@ -17,6 +17,7 @@ import ( "syscall" "unsafe" + "github.com/containers/libpod/pkg/errorhandling" "github.com/containers/storage/pkg/idtools" "github.com/docker/docker/pkg/signal" "github.com/godbus/dbus" @@ -41,8 +42,7 @@ const ( ) func runInUser() error { - os.Setenv("_CONTAINERS_USERNS_CONFIGURED", "done") - return nil + return os.Setenv("_CONTAINERS_USERNS_CONFIGURED", "done") } var ( @@ -57,9 +57,15 @@ func IsRootless() bool { rootlessGIDInit := int(C.rootless_gid()) if rootlessUIDInit != 0 { // This happens if we joined the user+mount namespace as part of - os.Setenv("_CONTAINERS_USERNS_CONFIGURED", "done") - os.Setenv("_CONTAINERS_ROOTLESS_UID", fmt.Sprintf("%d", rootlessUIDInit)) - os.Setenv("_CONTAINERS_ROOTLESS_GID", fmt.Sprintf("%d", rootlessGIDInit)) + if err := os.Setenv("_CONTAINERS_USERNS_CONFIGURED", "done"); err != nil { + logrus.Errorf("failed to set environment variable %s as %s", "_CONTAINERS_USERNS_CONFIGURED", "done") + } + if err := os.Setenv("_CONTAINERS_ROOTLESS_UID", fmt.Sprintf("%d", rootlessUIDInit)); err != nil { + logrus.Errorf("failed to set environment variable %s as %d", "_CONTAINERS_ROOTLESS_UID", rootlessUIDInit) + } + if err := os.Setenv("_CONTAINERS_ROOTLESS_GID", fmt.Sprintf("%d", rootlessGIDInit)); err != nil { + logrus.Errorf("failed to set environment variable %s as %d", "_CONTAINERS_ROOTLESS_GID", rootlessGIDInit) + } } isRootless = os.Geteuid() != 0 || os.Getenv("_CONTAINERS_USERNS_CONFIGURED") != "" }) @@ -185,18 +191,24 @@ func getUserNSFirstChild(fd uintptr) (*os.File, error) { } if ns == currentNS { - syscall.Close(int(nextFd)) + if err := syscall.Close(int(nextFd)); err != nil { + return nil, err + } // Drop O_CLOEXEC for the fd. _, _, errno := syscall.Syscall(syscall.SYS_FCNTL, fd, syscall.F_SETFD, 0) if errno != 0 { - syscall.Close(int(fd)) + if err := syscall.Close(int(fd)); err != nil { + logrus.Errorf("failed to close file descriptor %d", fd) + } return nil, errno } return os.NewFile(fd, "userns child"), nil } - syscall.Close(int(fd)) + if err := syscall.Close(int(fd)); err != nil { + return nil, err + } fd = nextFd } } @@ -252,7 +264,9 @@ func EnableLinger() (string, error) { if lingerEnabled && lingerFile != "" { f, err := os.Create(lingerFile) if err == nil { - f.Close() + if err := f.Close(); err != nil { + logrus.Errorf("failed to close %s", f.Name()) + } } else { logrus.Debugf("could not create linger file: %v", err) } @@ -348,8 +362,8 @@ func becomeRootInUserNS(pausePid, fileToRead string, fileOutput *os.File) (bool, } r, w := os.NewFile(uintptr(fds[0]), "sync host"), os.NewFile(uintptr(fds[1]), "sync child") - defer r.Close() - defer w.Close() + defer errorhandling.CloseQuiet(r) + defer errorhandling.CloseQuiet(w) defer w.Write([]byte("0")) pidC := C.reexec_in_user_namespace(C.int(r.Fd()), cPausePid, cFileToRead, fileOutputFD) @@ -361,9 +375,9 @@ func becomeRootInUserNS(pausePid, fileToRead string, fileOutput *os.File) (bool, var uids, gids []idtools.IDMap username := os.Getenv("USER") if username == "" { - user, err := user.LookupId(fmt.Sprintf("%d", os.Getuid())) + userID, err := user.LookupId(fmt.Sprintf("%d", os.Getuid())) if err == nil { - username = user.Username + username = userID.Username } } mappings, err := idtools.NewIDMappings(username, username) @@ -458,7 +472,9 @@ func becomeRootInUserNS(pausePid, fileToRead string, fileOutput *os.File) (bool, continue } - syscall.Kill(int(pidC), s.(syscall.Signal)) + if err := syscall.Kill(int(pidC), s.(syscall.Signal)); err != nil { + logrus.Errorf("failed to kill %d", int(pidC)) + } } }() @@ -519,17 +535,19 @@ func TryJoinFromFilePaths(pausePidPath string, needNewNamespace bool, paths []st r, w := os.NewFile(uintptr(fds[0]), "read file"), os.NewFile(uintptr(fds[1]), "write file") - defer w.Close() - defer r.Close() + defer errorhandling.CloseQuiet(w) + defer errorhandling.CloseQuiet(r) if _, _, err := becomeRootInUserNS("", path, w); err != nil { lastErr = err continue } - w.Close() + if err := w.Close(); err != nil { + return false, 0, err + } defer func() { - r.Close() + errorhandling.CloseQuiet(r) C.reexec_in_user_namespace_wait(-1, 0) }() diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index c9548f0d3..d44beb3e4 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -481,7 +481,9 @@ func addPidNS(config *CreateConfig, g *generate.Generator) error { func addUserNS(config *CreateConfig, g *generate.Generator) error { if IsNS(string(config.UsernsMode)) { - g.AddOrReplaceLinuxNamespace(spec.UserNamespace, NS(string(config.UsernsMode))) + if err := g.AddOrReplaceLinuxNamespace(spec.UserNamespace, NS(string(config.UsernsMode))); err != nil { + return err + } // runc complains if no mapping is specified, even if we join another ns. So provide a dummy mapping g.AddLinuxUIDMapping(uint32(0), uint32(0), uint32(1)) @@ -489,7 +491,9 @@ func addUserNS(config *CreateConfig, g *generate.Generator) error { } if (len(config.IDMappings.UIDMap) > 0 || len(config.IDMappings.GIDMap) > 0) && !config.UsernsMode.IsHost() { - g.AddOrReplaceLinuxNamespace(spec.UserNamespace, "") + if err := g.AddOrReplaceLinuxNamespace(spec.UserNamespace, ""); err != nil { + return err + } } return nil } diff --git a/pkg/systemdgen/systemdgen.go b/pkg/systemdgen/systemdgen.go index 3d1c31b5d..06c5ebde5 100644 --- a/pkg/systemdgen/systemdgen.go +++ b/pkg/systemdgen/systemdgen.go @@ -2,17 +2,18 @@ package systemdgen import ( "fmt" - "path/filepath" + "os" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) var template = `[Unit] Description=%s Podman Container [Service] Restart=%s -ExecStart=/usr/bin/podman start %s -ExecStop=/usr/bin/podman stop -t %d %s +ExecStart=%s start %s +ExecStop=%s stop -t %d %s KillMode=none Type=forking PIDFile=%s @@ -33,11 +34,26 @@ func ValidateRestartPolicy(restart string) error { // CreateSystemdUnitAsString takes variables to create a systemd unit file used to control // a libpod container -func CreateSystemdUnitAsString(name, cid, restart, pidPath string, stopTimeout int) (string, error) { +func CreateSystemdUnitAsString(name, cid, restart, pidFile string, stopTimeout int) (string, error) { + podmanExe := getPodmanExecutable() + return createSystemdUnitAsString(podmanExe, name, cid, restart, pidFile, stopTimeout) +} + +func createSystemdUnitAsString(exe, name, cid, restart, pidFile string, stopTimeout int) (string, error) { if err := ValidateRestartPolicy(restart); err != nil { return "", err } - pidFile := filepath.Join(pidPath, fmt.Sprintf("%s.pid", cid)) - unit := fmt.Sprintf(template, name, restart, name, stopTimeout, name, pidFile) + + unit := fmt.Sprintf(template, name, restart, exe, name, exe, stopTimeout, name, pidFile) return unit, nil } + +func getPodmanExecutable() string { + podmanExe, err := os.Executable() + if err != nil { + podmanExe = "/usr/bin/podman" + logrus.Warnf("Could not obtain podman executable location, using default %s", podmanExe) + } + + return podmanExe +} diff --git a/pkg/systemdgen/systemdgen_test.go b/pkg/systemdgen/systemdgen_test.go index f2f49e750..e413b24ce 100644 --- a/pkg/systemdgen/systemdgen_test.go +++ b/pkg/systemdgen/systemdgen_test.go @@ -41,7 +41,7 @@ ExecStart=/usr/bin/podman start 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4 ExecStop=/usr/bin/podman stop -t 10 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 KillMode=none Type=forking -PIDFile=/var/lib/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid +PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid [Install] WantedBy=multi-user.target` @@ -53,15 +53,16 @@ ExecStart=/usr/bin/podman start foobar ExecStop=/usr/bin/podman stop -t 10 foobar KillMode=none Type=forking -PIDFile=/var/lib/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid +PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid [Install] WantedBy=multi-user.target` type args struct { + exe string name string cid string restart string - pidPath string + pidFile string stopTimeout int } tests := []struct { @@ -73,10 +74,11 @@ WantedBy=multi-user.target` {"good with id", args{ + "/usr/bin/podman", "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", "always", - "/var/lib/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/", + "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", 10, }, goodID, @@ -84,10 +86,11 @@ WantedBy=multi-user.target` }, {"good with name", args{ + "/usr/bin/podman", "foobar", "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", "always", - "/var/lib/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/", + "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", 10, }, goodName, @@ -95,10 +98,11 @@ WantedBy=multi-user.target` }, {"bad restart policy", args{ + "/usr/bin/podman", "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", "never", - "/var/lib/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/", + "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", 10, }, "", @@ -107,7 +111,7 @@ WantedBy=multi-user.target` } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := CreateSystemdUnitAsString(tt.args.name, tt.args.cid, tt.args.restart, tt.args.pidPath, tt.args.stopTimeout) + got, err := createSystemdUnitAsString(tt.args.exe, tt.args.name, tt.args.cid, tt.args.restart, tt.args.pidFile, tt.args.stopTimeout) if (err != nil) != tt.wantErr { t.Errorf("CreateSystemdUnitAsString() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 9e49f08a0..fba34a337 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -12,12 +12,14 @@ import ( "github.com/BurntSushi/toml" "github.com/containers/image/types" "github.com/containers/libpod/cmd/podman/cliconfig" + "github.com/containers/libpod/pkg/errorhandling" "github.com/containers/libpod/pkg/namespaces" "github.com/containers/libpod/pkg/rootless" "github.com/containers/storage" "github.com/containers/storage/pkg/idtools" "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" + "github.com/sirupsen/logrus" "github.com/spf13/pflag" "golang.org/x/crypto/ssh/terminal" ) @@ -272,16 +274,20 @@ func getTomlStorage(storeOptions *storage.StoreOptions) *tomlConfig { // WriteStorageConfigFile writes the configuration to a file func WriteStorageConfigFile(storageOpts *storage.StoreOptions, storageConf string) error { - os.MkdirAll(filepath.Dir(storageConf), 0755) - file, err := os.OpenFile(storageConf, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666) + if err := os.MkdirAll(filepath.Dir(storageConf), 0755); err != nil { + return err + } + storageFile, err := os.OpenFile(storageConf, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666) if err != nil { return errors.Wrapf(err, "cannot open %s", storageConf) } tomlConfiguration := getTomlStorage(storageOpts) - defer file.Close() - enc := toml.NewEncoder(file) + defer errorhandling.CloseQuiet(storageFile) + enc := toml.NewEncoder(storageFile) if err := enc.Encode(tomlConfiguration); err != nil { - os.Remove(storageConf) + if err := os.Remove(storageConf); err != nil { + logrus.Errorf("unable to remove file %s", storageConf) + } return err } return nil diff --git a/pkg/util/utils_supported.go b/pkg/util/utils_supported.go index 99c9e4f1e..af55689a6 100644 --- a/pkg/util/utils_supported.go +++ b/pkg/util/utils_supported.go @@ -13,6 +13,7 @@ import ( "github.com/containers/libpod/pkg/rootless" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) // GetRootlessRuntimeDir returns the runtime directory when running as non root @@ -24,7 +25,9 @@ func GetRootlessRuntimeDir() (string, error) { uid := fmt.Sprintf("%d", rootless.GetRootlessUID()) if runtimeDir == "" { tmpDir := filepath.Join("/run", "user", uid) - os.MkdirAll(tmpDir, 0700) + if err := os.MkdirAll(tmpDir, 0700); err != nil { + logrus.Errorf("unable to make temp dir %s", tmpDir) + } st, err := os.Stat(tmpDir) if err == nil && int(st.Sys().(*syscall.Stat_t).Uid) == os.Geteuid() && st.Mode().Perm() == 0700 { runtimeDir = tmpDir @@ -32,7 +35,9 @@ func GetRootlessRuntimeDir() (string, error) { } if runtimeDir == "" { tmpDir := filepath.Join(os.TempDir(), fmt.Sprintf("run-%s", uid)) - os.MkdirAll(tmpDir, 0700) + if err := os.MkdirAll(tmpDir, 0700); err != nil { + logrus.Errorf("unable to make temp dir %s", tmpDir) + } st, err := os.Stat(tmpDir) if err == nil && int(st.Sys().(*syscall.Stat_t).Uid) == os.Geteuid() && st.Mode().Perm() == 0700 { runtimeDir = tmpDir diff --git a/pkg/varlinkapi/images.go b/pkg/varlinkapi/images.go index 0e2ad6bbf..2bebfd406 100644 --- a/pkg/varlinkapi/images.go +++ b/pkg/varlinkapi/images.go @@ -25,6 +25,7 @@ import ( "github.com/containers/libpod/libpod" "github.com/containers/libpod/libpod/define" "github.com/containers/libpod/libpod/image" + "github.com/containers/libpod/pkg/channelwriter" "github.com/containers/libpod/pkg/util" "github.com/containers/libpod/utils" "github.com/containers/storage/pkg/archive" @@ -495,9 +496,19 @@ func (i *LibpodAPI) DeleteUnusedImages(call iopodman.VarlinkCall) error { // Commit ... func (i *LibpodAPI) Commit(call iopodman.VarlinkCall, name, imageName string, changes []string, author, message string, pause bool, manifestType string) error { - var newImage *image.Image + var ( + newImage *image.Image + log []string + mimeType string + ) + output := channelwriter.NewChannelWriter() + channelClose := func() { + if err := output.Close(); err != nil { + logrus.Errorf("failed to close channel writer: %q", err) + } + } + defer channelClose() - output := bytes.NewBuffer([]byte{}) ctr, err := i.Runtime.LookupContainer(name) if err != nil { return call.ReplyContainerNotFound(name, err.Error()) @@ -507,7 +518,6 @@ func (i *LibpodAPI) Commit(call iopodman.VarlinkCall, name, imageName string, ch return call.ReplyErrorOccurred(err.Error()) } sc := image.GetSystemContext(rtc.SignaturePolicyPath, "", false) - var mimeType string switch manifestType { case "oci", "": //nolint mimeType = buildah.OCIv1ImageManifest @@ -535,6 +545,7 @@ func (i *LibpodAPI) Commit(call iopodman.VarlinkCall, name, imageName string, ch } c := make(chan error) + defer close(c) go func() { newImage, err = ctr.Commit(getContext(), imageName, options) @@ -542,48 +553,22 @@ func (i *LibpodAPI) Commit(call iopodman.VarlinkCall, name, imageName string, ch c <- err } c <- nil - close(c) }() - var log []string - done := false - for { - line, err := output.ReadString('\n') - if err == nil { - log = append(log, line) - continue - } else if err == io.EOF { - select { - case err := <-c: - if err != nil { - logrus.Errorf("reading of output during commit failed for %s", name) - return call.ReplyErrorOccurred(err.Error()) - } - done = true - default: - if !call.WantsMore() { - break - } - br := iopodman.MoreResponse{ - Logs: log, - } - call.ReplyCommit(br) - log = []string{} - } - } else { - return call.ReplyErrorOccurred(err.Error()) - } - if done { - break - } + // reply is the func being sent to the output forwarder. in this case it is replying + // with a more response struct + reply := func(br iopodman.MoreResponse) error { + return call.ReplyCommit(br) + } + log, err = forwardOutput(log, c, call.WantsMore(), output, reply) + if err != nil { + return call.ReplyErrorOccurred(err.Error()) } call.Continues = false - br := iopodman.MoreResponse{ Logs: log, Id: newImage.ID(), } - return call.ReplyCommit(br) } @@ -636,6 +621,7 @@ func (i *LibpodAPI) ExportImage(call iopodman.VarlinkCall, name, destination str func (i *LibpodAPI) PullImage(call iopodman.VarlinkCall, name string) error { var ( imageID string + err error ) dockerRegistryOptions := image.DockerRegistryOptions{} so := image.SigningOptions{} @@ -643,8 +629,16 @@ func (i *LibpodAPI) PullImage(call iopodman.VarlinkCall, name string) error { if call.WantsMore() { call.Continues = true } - output := bytes.NewBuffer([]byte{}) + output := channelwriter.NewChannelWriter() + channelClose := func() { + if err := output.Close(); err != nil { + logrus.Errorf("failed to close channel writer: %q", err) + } + } + defer channelClose() c := make(chan error) + defer close(c) + go func() { if strings.HasPrefix(name, dockerarchive.Transport.Name()+":") { srcRef, err := alltransports.ParseImageName(name) @@ -666,43 +660,17 @@ func (i *LibpodAPI) PullImage(call iopodman.VarlinkCall, name string) error { } } c <- nil - close(c) }() var log []string - done := false - for { - line, err := output.ReadString('\n') - if err == nil { - log = append(log, line) - continue - } else if err == io.EOF { - select { - case err := <-c: - if err != nil { - logrus.Errorf("reading of output during pull failed for %s", name) - return call.ReplyErrorOccurred(err.Error()) - } - done = true - default: - if !call.WantsMore() { - break - } - br := iopodman.MoreResponse{ - Logs: log, - } - call.ReplyPullImage(br) - log = []string{} - } - } else { - return call.ReplyErrorOccurred(err.Error()) - } - if done { - break - } + reply := func(br iopodman.MoreResponse) error { + return call.ReplyPullImage(br) + } + log, err = forwardOutput(log, c, call.WantsMore(), output, reply) + if err != nil { + return call.ReplyErrorOccurred(err.Error()) } call.Continues = false - br := iopodman.MoreResponse{ Logs: log, Id: imageID, diff --git a/pkg/varlinkapi/util.go b/pkg/varlinkapi/util.go index a74105795..e8f74e6aa 100644 --- a/pkg/varlinkapi/util.go +++ b/pkg/varlinkapi/util.go @@ -13,6 +13,7 @@ import ( "github.com/containers/libpod/cmd/podman/varlink" "github.com/containers/libpod/libpod" "github.com/containers/libpod/libpod/define" + "github.com/containers/libpod/pkg/channelwriter" "github.com/containers/storage/pkg/archive" ) @@ -196,3 +197,42 @@ func makePsOpts(inOpts iopodman.PsOpts) shared.PsOptions { Sync: derefBool(inOpts.Sync), } } + +// forwardOutput is a helper method for varlink endpoints that employ both more and without +// more. it is capable of sending updates as the output writer gets them or append them +// all to a log. the chan error is the error from the libpod call so we can honor +// and error event in that case. +func forwardOutput(log []string, c chan error, wantsMore bool, output *channelwriter.Writer, reply func(br iopodman.MoreResponse) error) ([]string, error) { + done := false + for { + select { + // We need to check if the libpod func being called has returned an + // error yet + case err := <-c: + if err != nil { + return nil, err + } + done = true + // if no error is found, we pull what we can from the log writer and + // append it to log string slice + case line := <-output.ByteChannel: + log = append(log, string(line)) + // If the end point is being used in more mode, send what we have + if wantsMore { + br := iopodman.MoreResponse{ + Logs: log, + } + if err := reply(br); err != nil { + return nil, err + } + // "reset" the log to empty because we are sending what we + // get as we get it + log = []string{} + } + } + if done { + break + } + } + return log, nil +} |