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 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 8212a331676..c99c8a6eea5 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 a7651958a88..ed08f5f40ed 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 dde6aecf13a..82b5f601559 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 82b5f601559..eefd202b514 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 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