From a1afa58e2769c72e427ffb728d25d4a2b13d54e9 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 12 Feb 2026 19:07:59 +0100 Subject: [PATCH] system service: remove config reload functionallity As I outlined in the design docs this is broken, there are several data races here because we write to the config files that can be read by other goroutines in parallel which violates the go memory model and thus can lead to runtime panics and undefined behavior. One could fix with a mutex but that would make the whole code base much more ugly and there is still the risk that something would access this field without the mutex held. I am not sure we have any users using this, it never worked for the storage side and since the service is a not a daemon any user could just stop and start it again to re-read the files without having to stop running containers. Signed-off-by: Paul Holzinger --- cmd/podman/system/service_abi.go | 1 - libpod/runtime.go | 37 ------------------------------ pkg/domain/infra/runtime_libpod.go | 23 ------------------- 3 files changed, 61 deletions(-) diff --git a/cmd/podman/system/service_abi.go b/cmd/podman/system/service_abi.go index 07f622c0ac..1e9ef6eff6 100644 --- a/cmd/podman/system/service_abi.go +++ b/cmd/podman/system/service_abi.go @@ -126,7 +126,6 @@ func restService(flags *pflag.FlagSet, cfg *entities.PodmanConfig, opts entities maybeMoveToSubCgroup() maybeStartServiceReaper() - infra.StartWatcher(libpodRuntime) server, err := api.NewServerWithSettings(libpodRuntime, listener, opts) if err != nil { return err diff --git a/libpod/runtime.go b/libpod/runtime.go index 4347053ed1..f84a89042b 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -38,7 +38,6 @@ import ( artStore "go.podman.io/common/pkg/libartifact" "go.podman.io/common/pkg/secrets" systemdCommon "go.podman.io/common/pkg/systemd" - "go.podman.io/image/v5/pkg/sysregistriesv2" is "go.podman.io/image/v5/storage" "go.podman.io/image/v5/types" "go.podman.io/storage" @@ -1049,42 +1048,6 @@ func (r *Runtime) EnableLabeling() bool { return r.config.Containers.EnableLabeling } -// Reload reloads the configurations files -func (r *Runtime) Reload() error { - if err := r.reloadContainersConf(); err != nil { - return err - } - if err := r.reloadStorageConf(); err != nil { - return err - } - // Invalidate the registries.conf cache. The next invocation will - // reload all data. - sysregistriesv2.InvalidateCache() - return nil -} - -// reloadContainersConf reloads the containers.conf -func (r *Runtime) reloadContainersConf() error { - config, err := config.Reload() - if err != nil { - return err - } - r.config = config - logrus.Infof("Applied new containers configuration: %v", config) - return nil -} - -// reloadStorageConf reloads the storage.conf -func (r *Runtime) reloadStorageConf() error { - configFile, err := storage.DefaultConfigFile() - if err != nil { - return err - } - storage.ReloadConfigurationFile(configFile, &r.storageConfig) - logrus.Infof("Applied new storage configuration: %v", r.storageConfig) - return nil -} - // getVolumePlugin gets a specific volume plugin. func (r *Runtime) getVolumePlugin(volConfig *VolumeConfig) (*plugin.VolumePlugin, error) { // There is no plugin for local. diff --git a/pkg/domain/infra/runtime_libpod.go b/pkg/domain/infra/runtime_libpod.go index 2deb63f182..c26b184b19 100644 --- a/pkg/domain/infra/runtime_libpod.go +++ b/pkg/domain/infra/runtime_libpod.go @@ -8,10 +8,8 @@ import ( "fmt" "io/fs" "os" - "os/signal" "strings" "sync" - "syscall" "github.com/containers/podman/v6/libpod" "github.com/containers/podman/v6/pkg/domain/entities" @@ -287,24 +285,3 @@ func ParseIDMapping(mode namespaces.UsernsMode, uidMapSlice, gidMapSlice []strin } return &options, nil } - -// StartWatcher starts a new SIGHUP go routine for the current config. -func StartWatcher(rt *libpod.Runtime) { - // Set up the signal notifier - ch := make(chan os.Signal, 1) - signal.Notify(ch, syscall.SIGHUP) - - go func() { - for { - // Block until the signal is received - logrus.Debugf("waiting for SIGHUP to reload configuration") - <-ch - if err := rt.Reload(); err != nil { - logrus.Errorf("Unable to reload configuration: %v", err) - continue - } - } - }() - - logrus.Debugf("registered SIGHUP watcher for config") -}