From 8afeb5839872650880b15909167b2feeada0c2f0 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 22fe0f9b4..0c7a0128c 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 8212a3316..c99c8a6ee 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 a7651958a..ed08f5f40 100644 --- a/libcontainer/criu_linux.go +++ b/libcontainer/criu_linux.go @@ -1113,7 +1113,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 dde6aecf1..82b5f6015 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -740,7 +740,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 93091e6ac2cb8c1a60b4565c04bdaad478da0e5e 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 82b5f6015..eefd202b5 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -615,9 +615,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) } @@ -779,16 +786,6 @@ func (p *initProcess) start() (retErr error) { return nil } -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 f7cd95dd1..158e03c56 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 9f7fa45d5..ce6a896ce 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