+
Skip to content

Reloading CRI-O configuration using SIGHUP overrides settings passed through command-line arguments #7586

New issue
New issue
Open
Open
Reloading CRI-O configuration using SIGHUP overrides settings passed through command-line arguments#7586
Assignees
LenkaSeg
Labels
good first issueDenotes an issue ready for a new contributor, according to the "help wanted" guidelines.kind/bugCategorizes issue or PR as related to a bug.kind/featureCategorizes issue or PR as related to a new feature.
Milestone
 
1.34
@kwilczynski

Description

@kwilczynski
kwilczynski
opened on Dec 12, 2023

What happened?

When the SIGHUP signal was sent to a currently running CRI-O process, it reloaded its configuration.

However, the CRI-O process was started with several command-line arguments, which were used to override default configuration options such as the log level or the pause image reference.

Following the request to CRI-O to reload its runtime configuration, the options specific via the command-line arguments were overridden with either a default or an item from the /etc/crio/crio.conf (or a drop-in from /etc/crio/crio.conf.d) file.

From this point onwards, the log level was changed, and the reference for the pause container image was updated to a hardcoded default.

What did you expect to happen?

The configuration to be reloaded with the options set through the applied command-line arguments to be preserved.

The precedence of configuration parsing should remain unchanged, and everything should also be merged accordingly, whether it was a fresh start of a runtime configuration reload request via the associated Linux signal delivered to the running process.

Additionally, the message that notifies the user about configuration reload should always be printed - this is not the case currently, as the log line itself is subjected to the presently set log level.

Preferably, it would also be nice if CRI-O had printed its runtime configuration on startup or even following a configuration reload, mainly to capture this type of detail in the log file for later reference (which can be helpful during troubleshooting). However, if the amount of logs this would produce is substantial, then perhaps the entire configuration will only be printed at the debug log level, and otherwise, only (so at the "info" level) difference to the default options would be printed - not sure if this would be desirable, though.

How can we reproduce it (as minimally and precisely as possible)?

Only a few steps are needed to reproduce this behaviour reliably:

  1. Prepare a configuration file drop-in to override the default log level (which is "info") to the level of warning (or "warn")
# cat > /etc/crio/crio.conf.d/99-log-level.conf
[crio.runtime]
log_level = "warn"
^D
  1. Start a new CRI-O process manually overriding the log level and setting it to the "debug" level
# crio --log-level debug
INFO[2023-12-12 22:13:11.794601654Z] Starting CRI-O, version: 1.29.0, git: 7380d8ff553b9b2ead8bf3173e8c1db0bc9004db(clean)

(... log lines that follow omitted for brevity ...)

At this point, you can observe CRI-O process log level being set to "debug", and a rather substantial number of log lines being produced. Aside from the logs themselves, you can use the crio binary itself (assuming a modern version of CRI-O; otherwise, the crio-status can be used) to verify the current runtime configuration log level and what it's being set to, per:

# crio status config print | grep log_level
INFO[2023-12-12 22:08:57.099952659Z] Starting CRI-O, version: 1.28.1, git: unknown(clean) 
    log_level = "debug"
  1. Send the SIGHUP signal to the running CRI-O process in a different terminal session
$ kill -HUP $(pgrep crio) # Or using pkill -HUP crio, whichever is more convenient.

Please return your attention to the terminal session running the CRI-O process in the foreground. At this point, you can observe the following lines appearing amongst other messages:

(... a great many other log lines omitted for brevity ...)

DEBU[2023-12-12 22:13:13.762844491Z] received signal                               file="crio/main.go:57" signal=hangup
INFO[2023-12-12 22:13:13.763082980Z] Reloading configuration                       file="config/reload.go:18"
DEBU[2023-12-12 22:13:13.763491295Z] Using /sbin/apparmor_parser binary            file="supported/supported.go:61"
DEBU[2023-12-12 22:13:13.763702316Z] Using /sbin/apparmor_parser binary            file="supported/supported.go:61"
INFO[2023-12-12 22:13:13.764018888Z] Updating config from file /etc/crio/crio.conf  file="config/reload.go:27"
INFO[2023-12-12 22:13:13.764863975Z] Updating config from path /etc/crio/crio.conf.d  file="config/reload.go:36"
INFO[2023-12-12 22:13:13.765185045Z] Set config log_level to "warn"                file="config/reload.go:83"

Note the confirmation of signal reception and configuration reload. There is also a log line corresponding to a change in the log level, which is crucial to note.

Similarly to before, you can use the crio binary itself to obtain the current runtime configuration, per:

# crio status config print | grep log_level
INFO[2023-12-12 22:15:27.997584925Z] Starting CRI-O, version: 1.28.1, git: unknown(clean) 
    log_level = "warn"

This observed behaviour following a runtime configuration reload is not necessarily desirable.

Anything else we need to know?

Personally, I believe we should aim to preserve the order of precedence when reloading the runtime configuration. One could argue that "but, what when the user makes a change to the configuration file, and would like or override option set prior via a command-line arguments?" - well, it's a fair point. Nonetheless, what was set via the configuration arguments should remain unchanged and be the default behaviour. Otherwise, we risk violating the principle of least surprise for the user, which we should avoid if it can be helped.

The suggested order of preference when merging configuration would be (higher to lower):

  • Any command-line arguments that were passed
  • Environment variables set
  • Configuration files (including any files from the so-called "conf.d" directory where the drop-ins are to be stored)
  • Default configuration

Now, the "conf.d" directory also has its own rules of how files from there are to be read and in what order - a lot of users respect files to be sorted and read in some order, most likely lexicographic (sort with the "C" locale to remove ambiguity), such that file called "01-foo.conf" would be loaded before files such as "zzz_bar.conf" or "99-baz.conf", I suppose.

Much of this is already working as intended, of course.

The following code is relevant:

  • Main and runtime configuration types

cri-o/pkg/config/config.go

Lines 65 to 80 in 7380d8f

type Config struct {
Comment string
singleConfigPath string // Path to the single config file
dropInConfigDir string // Path to the drop-in config files
RootConfig
APIConfig
RuntimeConfig
ImageConfig
NetworkConfig
MetricsConfig
TracingConfig
StatsConfig
NRI *nri.Config
SystemContext *types.SystemContext
}

cri-o/pkg/config/config.go

Lines 236 to 477 in 7380d8f

type RuntimeConfig struct {
// SeccompUseDefaultWhenEmpty specifies whether the default profile
// should be used when an empty one is specified.
// This option is currently deprecated, and will be replaced by the SeccompDefault FeatureGate in Kubernetes.
SeccompUseDefaultWhenEmpty bool `toml:"seccomp_use_default_when_empty"`
// NoPivot instructs the runtime to not use `pivot_root`, but instead use `MS_MOVE`
NoPivot bool `toml:"no_pivot"`
// SELinux determines whether or not SELinux is used for pod separation.
SELinux bool `toml:"selinux"`
// Whether container output should be logged to journald in addition
// to the kubernetes log file
LogToJournald bool `toml:"log_to_journald"`
// DropInfraCtr determines whether the infra container is dropped when appropriate.
DropInfraCtr bool `toml:"drop_infra_ctr"`
// ReadOnly run all pods/containers in read-only mode.
// This mode will mount tmpfs on /run, /tmp and /var/tmp, if those are not mountpoints
// Will also set the readonly flag in the OCI Runtime Spec. In this mode containers
// will only be able to write to volumes mounted into them
ReadOnly bool `toml:"read_only"`
// ConmonEnv is the environment variable list for conmon process.
// This option is currently deprecated, and will be replaced with RuntimeHandler.MonitorEnv.
ConmonEnv []string `toml:"conmon_env"`
// HooksDir holds paths to the directories containing hooks
// configuration files. When the same filename is present in
// multiple directories, the file in the directory listed last in
// this slice takes precedence.
HooksDir []string `toml:"hooks_dir"`
// Capabilities to add to all containers.
DefaultCapabilities capabilities.Capabilities `toml:"default_capabilities"`
// AddInheritableCapabilities can be set to add inheritable capabilities. They were pre-1.23 by default, and were dropped in 1.24.
// This can cause a regression with non-root users not getting capabilities as they previously did.
AddInheritableCapabilities bool `toml:"add_inheritable_capabilities"`
// Additional environment variables to set for all the
// containers. These are overridden if set in the
// container image spec or in the container runtime configuration.
DefaultEnv []string `toml:"default_env"`
// Sysctls to add to all containers.
DefaultSysctls []string `toml:"default_sysctls"`
// DefaultUlimits specifies the default ulimits to apply to containers
DefaultUlimits []string `toml:"default_ulimits"`
// Devices that are allowed to be configured.
AllowedDevices []string `toml:"allowed_devices"`
// Devices to add to containers
AdditionalDevices []string `toml:"additional_devices"`
// CDISpecDirs specifies the directories CRI-O/CDI will scan for CDI Spec files.
CDISpecDirs []string `toml:"cdi_spec_dirs"`
// DeviceOwnershipFromSecurityContext changes the default behavior of setting container devices uid/gid
// from CRI's SecurityContext (RunAsUser/RunAsGroup) instead of taking host's uid/gid. Defaults to false.
DeviceOwnershipFromSecurityContext bool `toml:"device_ownership_from_security_context"`
// DefaultRuntime is the _name_ of the OCI runtime to be used as the default.
// The name is matched against the Runtimes map below.
DefaultRuntime string `toml:"default_runtime"`
// DecryptionKeysPath is the path where keys for image decryption are stored.
DecryptionKeysPath string `toml:"decryption_keys_path"`
// Conmon is the path to conmon binary, used for managing the runtime.
// This option is currently deprecated, and will be replaced with RuntimeHandler.MonitorConfig.Path.
Conmon string `toml:"conmon"`
// ConmonCgroup is the cgroup setting used for conmon.
// This option is currently deprecated, and will be replaced with RuntimeHandler.MonitorConfig.Cgroup.
ConmonCgroup string `toml:"conmon_cgroup"`
// SeccompProfile is the seccomp.json profile path which is used as the
// default for the runtime.
SeccompProfile string `toml:"seccomp_profile"`
// ApparmorProfile is the apparmor profile name which is used as the
// default for the runtime.
ApparmorProfile string `toml:"apparmor_profile"`
// BlockIOConfigFile is the path to the blockio class configuration
// file for configuring the cgroup blockio controller.
BlockIOConfigFile string `toml:"blockio_config_file"`
// BlockIOReload instructs the runtime to reload blockio configuration
// rescan block devices in the system before assigning blockio parameters.
BlockIOReload bool `toml:"blockio_reload"`
// IrqBalanceConfigFile is the irqbalance service config file which is used
// for configuring irqbalance daemon.
IrqBalanceConfigFile string `toml:"irqbalance_config_file"`
// RdtConfigFile is the RDT config file used for configuring resctrl fs
RdtConfigFile string `toml:"rdt_config_file"`
// CgroupManagerName is the manager implementation name which is used to
// handle cgroups for containers.
CgroupManagerName string `toml:"cgroup_manager"`
// DefaultMountsFile is the file path for the default mounts to be mounted for the container
// Note, for testing purposes mainly
DefaultMountsFile string `toml:"default_mounts_file"`
// ContainerExitsDir is the directory in which container exit files are
// written to by conmon.
ContainerExitsDir string `toml:"container_exits_dir"`
// ContainerAttachSocketDir is the location for container attach sockets.
ContainerAttachSocketDir string `toml:"container_attach_socket_dir"`
// BindMountPrefix is the prefix to use for the source of the bind mounts.
BindMountPrefix string `toml:"bind_mount_prefix"`
// UIDMappings specifies the UID mappings to have in the user namespace.
// A range is specified in the form containerUID:HostUID:Size. Multiple
// ranges are separated by comma.
UIDMappings string `toml:"uid_mappings"`
// MinimumMappableUID specifies the minimum UID value which can be
// specified in a uid_mappings value, whether configured here or sent
// to us via CRI, for a pod that isn't to be run as UID 0.
MinimumMappableUID int64 `toml:"minimum_mappable_uid"`
// GIDMappings specifies the GID mappings to have in the user namespace.
// A range is specified in the form containerUID:HostUID:Size. Multiple
// ranges are separated by comma.
GIDMappings string `toml:"gid_mappings"`
// MinimumMappableGID specifies the minimum GID value which can be
// specified in a gid_mappings value, whether configured here or sent
// to us via CRI, for a pod that isn't to be run as UID 0.
MinimumMappableGID int64 `toml:"minimum_mappable_gid"`
// LogLevel determines the verbosity of the logs based on the level it is set to.
// Options are fatal, panic, error (default), warn, info, debug, and trace.
LogLevel string `toml:"log_level"`
// LogFilter specifies a regular expression to filter the log messages
LogFilter string `toml:"log_filter"`
// NamespacesDir is the directory where the state of the managed namespaces
// gets tracked
NamespacesDir string `toml:"namespaces_dir"`
// PinNSPath is the path to find the pinns binary, which is needed
// to manage namespace lifecycle
PinnsPath string `toml:"pinns_path"`
// CriuPath is the path to find the criu binary, which is needed
// to checkpoint and restore containers
EnableCriuSupport bool `toml:"enable_criu_support"`
// Runtimes defines a list of OCI compatible runtimes. The runtime to
// use is picked based on the runtime_handler provided by the CRI. If
// no runtime_handler is provided, the runtime will be picked based on
// the level of trust of the workload.
Runtimes Runtimes `toml:"runtimes"`
// Workloads defines a list of workloads types that are have grouped settings
// that will be applied to containers.
Workloads Workloads `toml:"workloads"`
// PidsLimit is the number of processes each container is restricted to
// by the cgroup process number controller.
PidsLimit int64 `toml:"pids_limit"`
// LogSizeMax is the maximum number of bytes after which the log file
// will be truncated. It can be expressed as a human-friendly string
// that is parsed to bytes.
// Negative values indicate that the log file won't be truncated.
LogSizeMax int64 `toml:"log_size_max"`
// CtrStopTimeout specifies the time to wait before to generate an
// error because the container state is still tagged as "running".
CtrStopTimeout int64 `toml:"ctr_stop_timeout"`
// SeparatePullCgroup specifies whether an image pull must be performed in a separate cgroup
SeparatePullCgroup string `toml:"separate_pull_cgroup"`
// InfraCtrCPUSet is the CPUs set that will be used to run infra containers
InfraCtrCPUSet string `toml:"infra_ctr_cpuset"`
// SharedCPUSet is the CPUs set that will be used for guaranteed containers that
// want access to shared cpus.
SharedCPUSet string `toml:"shared_cpuset"`
// AbsentMountSourcesToReject is a list of paths that, when absent from the host,
// will cause a container creation to fail (as opposed to the current behavior of creating a directory).
AbsentMountSourcesToReject []string `toml:"absent_mount_sources_to_reject"`
// EnablePodEvents specifies if the container pod-level events should be generated to optimize the PLEG at Kubelet.
EnablePodEvents bool `toml:"enable_pod_events"`
// IrqBalanceConfigRestoreFile is the irqbalance service banned CPU list to restore.
// If empty, no restoration attempt will be done.
IrqBalanceConfigRestoreFile string `toml:"irqbalance_config_restore_file"`
// seccompConfig is the internal seccomp configuration
seccompConfig *seccomp.Config
// apparmorConfig is the internal AppArmor configuration
apparmorConfig *apparmor.Config
// blockioConfig is the internal blockio configuration
blockioConfig *blockio.Config
// rdtConfig is the internal Rdt configuration
rdtConfig *rdt.Config
// ulimitConfig is the internal ulimit configuration
ulimitsConfig *ulimits.Config
// deviceConfig is the internal additional devices configuration
deviceConfig *device.Config
// cgroupManager is the internal CgroupManager configuration
cgroupManager cgmgr.CgroupManager
// conmonManager is the internal ConmonManager configuration
conmonManager *conmonmgr.ConmonManager
// namespaceManager is the internal NamespaceManager configuration
namespaceManager *nsmgr.NamespaceManager
// Whether SELinux should be disabled within a pod,
// when it is running in the host network namespace
// https://github.com/cri-o/cri-o/issues/5501
HostNetworkDisableSELinux bool `toml:"hostnetwork_disable_selinux"`
// Option to disable hostport mapping in CRI-O
// Default value is 'false'
DisableHostPortMapping bool `toml:"disable_hostport_mapping"`
}

  • Configuration reload handler

cri-o/server/server.go

Lines 583 to 600 in 7380d8f

func (s *Server) startReloadWatcher(ctx context.Context) {
// Setup the signal notifier
ch := make(chan os.Signal, 1)
signal.Notify(ch, signals.Hup)
go func() {
for {
// Block until the signal is received
<-ch
if err := s.config.Reload(); err != nil {
logrus.Errorf("Unable to reload configuration: %v", err)
continue
}
}
}()
log.Infof(ctx, "Registered SIGHUP reload watcher")
}

  • The Config type concrete Reload() function

cri-o/pkg/config/reload.go

Lines 17 to 77 in 7380d8f

func (c *Config) Reload() error {
logrus.Infof("Reloading configuration")
// Reload the config
newConfig, err := DefaultConfig()
if err != nil {
return fmt.Errorf("unable to create default config")
}
if _, err := os.Stat(c.singleConfigPath); !os.IsNotExist(err) {
logrus.Infof("Updating config from file %s", c.singleConfigPath)
if err := newConfig.UpdateFromFile(c.singleConfigPath); err != nil {
return err
}
} else {
logrus.Infof("Skipping not-existing config file %q", c.singleConfigPath)
}
if _, err := os.Stat(c.dropInConfigDir); !os.IsNotExist(err) {
logrus.Infof("Updating config from path %s", c.dropInConfigDir)
if err := newConfig.UpdateFromPath(c.dropInConfigDir); err != nil {
return err
}
} else {
logrus.Infof("Skipping not-existing config path %q", c.dropInConfigDir)
}
// Reload all available options
if err := c.ReloadLogLevel(newConfig); err != nil {
return err
}
if err := c.ReloadLogFilter(newConfig); err != nil {
return err
}
if err := c.ReloadPauseImage(newConfig); err != nil {
return err
}
c.ReloadPinnedImages(newConfig)
if err := c.ReloadRegistries(); err != nil {
return err
}
c.ReloadDecryptionKeyConfig(newConfig)
if err := c.ReloadSeccompProfile(newConfig); err != nil {
return err
}
if err := c.ReloadAppArmorProfile(newConfig); err != nil {
return err
}
if err := c.ReloadBlockIOConfig(newConfig); err != nil {
return err
}
if err := c.ReloadRdtConfig(newConfig); err != nil {
return err
}
if err := c.ReloadRuntimes(newConfig); err != nil {
return err
}
cdi.GetRegistry(cdi.WithSpecDirs(newConfig.CDISpecDirs...))
return nil
}

  • The function that handles log level reload and set-up

cri-o/pkg/config/reload.go

Lines 88 to 102 in 7380d8f

func (c *Config) ReloadLogLevel(newConfig *Config) error {
if c.LogLevel != newConfig.LogLevel {
level, err := logrus.ParseLevel(newConfig.LogLevel)
if err != nil {
return err
}
// Always log this message without considering the current
logrus.SetLevel(logrus.InfoLevel)
logConfig("log_level", newConfig.LogLevel)
logrus.SetLevel(level)
c.LogLevel = newConfig.LogLevel
}
return nil
}

CRI-O and Kubernetes version

$ crio --version
crio version 1.29.0
Version:        1.29.0
GitCommit:      7380d8ff553b9b2ead8bf3173e8c1db0bc9004db
GitCommitDate:  2023-12-12T19:22:16Z
GitTreeState:   clean
BuildDate:      2023-12-12T21:54:46Z
GoVersion:      go1.21.5
Compiler:       gc
Platform:       linux/amd64
Linkmode:       static
BuildTags:      
  containers_image_ostree_stub
  libdm_no_deferred_remove
  containers_image_openpgp
  seccomp
  selinux
  apparmor
  static
  netgo
  exclude_graphdriver_btrfs
LDFlags:          unknown
SeccompEnabled:   true
AppArmorEnabled:  true

OS version

$ cat /etc/os-release
PRETTY_NAME="Ubuntu 22.04.3 LTS"
NAME="Ubuntu"
VERSION_ID="22.04"
VERSION="22.04.3 LTS (Jammy Jellyfish)"
VERSION_CODENAME=jammy
ID=ubuntu
ID_LIKE=debian
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
UBUNTU_CODENAME=jammy
$ uname -a
Linux worker-node01 5.15.0-83-generic #92-Ubuntu SMP Mon Aug 14 09:30:42 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Additional environment details (AWS, VirtualBox, physical, etc.)

None.

Metadata

Metadata

Assignees

Labels

good first issueDenotes an issue ready for a new contributor, according to the "help wanted" guidelines.kind/bugCategorizes issue or PR as related to a bug.kind/featureCategorizes issue or PR as related to a new feature.

Type

No type

Projects

Feature Roadmap

Status

In Progress

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions
    点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载