From aed9c22bb2e7a59d2b75cef30f909f91feb86a94 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 9 Oct 2024 11:00:48 -0700 Subject: [PATCH 1/2] libct: add/use configs.HasHook This allows to omit a call to c.currentOCIState (which can be somewhat costly when there are many annotations) when the hooks of a given kind won't be run. Signed-off-by: Kir Kolyshkin --- libcontainer/configs/config.go | 13 +++++++++++++ libcontainer/container_linux.go | 2 +- libcontainer/criu_linux.go | 2 +- libcontainer/process_linux.go | 2 +- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/libcontainer/configs/config.go b/libcontainer/configs/config.go index 22fe0f9b4c1..0c7a0128c74 100644 --- a/libcontainer/configs/config.go +++ b/libcontainer/configs/config.go @@ -332,6 +332,19 @@ const ( Poststop HookName = "poststop" ) +// HasHook checks if config has any hooks with any given names configured. +func (c *Config) HasHook(names ...HookName) bool { + if c.Hooks == nil { + return false + } + for _, h := range names { + if len(c.Hooks[h]) > 0 { + return true + } + } + return false +} + // KnownHookNames returns the known hook names. // Used by `runc features`. func KnownHookNames() []string { diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index c02116177ad..5e1887afce7 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -350,7 +350,7 @@ func (c *Container) start(process *Process) (retErr error) { if process.Init { c.fifo.Close() - if c.config.Hooks != nil { + if c.config.HasHook(configs.Poststart) { s, err := c.currentOCIState() if err != nil { return err diff --git a/libcontainer/criu_linux.go b/libcontainer/criu_linux.go index fed34e79148..cfc5823d15a 100644 --- a/libcontainer/criu_linux.go +++ b/libcontainer/criu_linux.go @@ -1110,7 +1110,7 @@ func (c *Container) criuNotifications(resp *criurpc.CriuResp, process *Process, return err } case "setup-namespaces": - if c.config.Hooks != nil { + if c.config.HasHook(configs.Prestart, configs.CreateRuntime) { s, err := c.currentOCIState() if err != nil { return nil diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index fcbb54a3e41..9b34258dd5f 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -750,7 +750,7 @@ func (p *initProcess) start() (retErr error) { return fmt.Errorf("error setting Intel RDT config for procHooks process: %w", err) } } - if len(p.config.Config.Hooks) != 0 { + if p.config.Config.HasHook(configs.Prestart, configs.CreateRuntime) { s, err := p.container.currentOCIState() if err != nil { return err From 298c19f6dfc9d57c02d14870a3d1746b0e1d0740 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 9 Oct 2024 11:46:51 -0700 Subject: [PATCH 2/2] libct: don't pass SpecState to init unless needed SpecState field of initConfig is only needed to run hooks that are executed inside a container -- namely CreateContainer and StartContainer. If these hooks are not configured, there is no need to fill, marshal and unmarshal SpecState. While at it, inline updateSpecState as it is trivial and only has one user. Signed-off-by: Kir Kolyshkin --- libcontainer/process_linux.go | 21 +++++++++------------ libcontainer/rootfs_linux.go | 11 ++++++----- libcontainer/standard_init_linux.go | 11 ++++++----- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 9b34258dd5f..726cb6a55ed 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -625,9 +625,16 @@ func (p *initProcess) start() (retErr error) { if err := p.createNetworkInterfaces(); err != nil { return fmt.Errorf("error creating network interfaces: %w", err) } - if err := p.updateSpecState(); err != nil { - return fmt.Errorf("error updating spec state: %w", err) + + // initConfig.SpecState is only needed to run hooks that are executed + // inside a container, i.e. CreateContainer and StartContainer. + if p.config.Config.HasHook(configs.CreateContainer, configs.StartContainer) { + p.config.SpecState, err = p.container.currentOCIState() + if err != nil { + return fmt.Errorf("error getting current state: %w", err) + } } + if err := utils.WriteJSON(p.comm.initSockParent, p.config); err != nil { return fmt.Errorf("error sending config to init process: %w", err) } @@ -810,16 +817,6 @@ func (p *initProcess) startTime() (uint64, error) { return stat.StartTime, err } -func (p *initProcess) updateSpecState() error { - s, err := p.container.currentOCIState() - if err != nil { - return err - } - - p.config.SpecState = s - return nil -} - func (p *initProcess) createNetworkInterfaces() error { for _, config := range p.config.Config.Networks { strategy, err := getStrategy(config.Type) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index f7cd95dd189..158e03c56d9 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -195,11 +195,12 @@ func prepareRootfs(pipe *syncSocket, iConfig *initConfig) (err error) { return &os.PathError{Op: "chdir", Path: config.Rootfs, Err: err} } - s := iConfig.SpecState - s.Pid = unix.Getpid() - s.Status = specs.StateCreating - if err := iConfig.Config.Hooks.Run(configs.CreateContainer, s); err != nil { - return err + if s := iConfig.SpecState; s != nil { + s.Pid = unix.Getpid() + s.Status = specs.StateCreating + if err := iConfig.Config.Hooks.Run(configs.CreateContainer, s); err != nil { + return err + } } if config.NoPivotRoot { diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index 9f7fa45d533..ce6a896ce29 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -267,11 +267,12 @@ func (l *linuxStandardInit) Init() error { // https://github.com/torvalds/linux/blob/v4.9/fs/exec.c#L1290-L1318 _ = l.fifoFile.Close() - s := l.config.SpecState - s.Pid = unix.Getpid() - s.Status = specs.StateCreated - if err := l.config.Config.Hooks.Run(configs.StartContainer, s); err != nil { - return err + if s := l.config.SpecState; s != nil { + s.Pid = unix.Getpid() + s.Status = specs.StateCreated + if err := l.config.Config.Hooks.Run(configs.StartContainer, s); err != nil { + return err + } } // Close all file descriptors we are not passing to the container. This is