From ff553c5069dbc1c2f52e34a33548583587d3a92b Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Thu, 27 Nov 2025 19:49:37 +0000 Subject: [PATCH] NRI: make config reloadable Signed-off-by: Rob Murray --- daemon/internal/nri/nri.go | 43 ++++++++- daemon/reload.go | 28 +++++- integration/daemon/nri/nri_test.go | 79 +++++++++++++++ .../daemon/nri/testdata/test_plugin.go | 95 +++++++++++++++++++ 4 files changed, 241 insertions(+), 4 deletions(-) create mode 100644 integration/daemon/nri/testdata/test_plugin.go diff --git a/daemon/internal/nri/nri.go b/daemon/internal/nri/nri.go index 48d26c28fb..8e3f3284f8 100644 --- a/daemon/internal/nri/nri.go +++ b/daemon/internal/nri/nri.go @@ -46,10 +46,10 @@ const ( ) type NRI struct { - cfg Config - - // mu protects nri - read lock for container operations, write lock for sync and shutdown. + // mu protects cfg and adap + // Read lock for container operations, write lock for sync, config update and shutdown. mu sync.RWMutex + cfg Config adap *adaptation.Adaptation } @@ -103,6 +103,43 @@ func (n *NRI) Shutdown(ctx context.Context) { n.adap = nil } +// PrepareReload validates and prepares for a configuration reload. It returns +// a function to perform the actual reload when called. +func (n *NRI) PrepareReload(nriCfg opts.NRIOpts) (func() error, error) { + var newNRI *adaptation.Adaptation + newCfg := n.cfg + newCfg.DaemonConfig = nriCfg + if err := setDefaultPaths(&newCfg.DaemonConfig); err != nil { + return nil, err + } + + if nriCfg.Enable { + var err error + newNRI, err = adaptation.New("docker", dockerversion.Version, n.syncFn, n.updateFn, nriOptions(newCfg.DaemonConfig)...) + if err != nil { + return nil, err + } + } + + return func() error { + n.mu.Lock() + if n.adap != nil { + log.G(context.TODO()).Info("Shutting down old NRI instance") + n.adap.Stop() + } + n.cfg = newCfg + n.adap = newNRI + // Release the lock before starting newNRI, because it'll call back to syncFn + // which will acquire the lock. + n.mu.Unlock() + + if newNRI == nil { + return nil + } + return newNRI.Start() + }, nil +} + // CreateContainer notifies plugins of a "creation" NRI-lifecycle event for a container, // allowing the plugin to adjust settings before the container is created. // diff --git a/daemon/reload.go b/daemon/reload.go index cc5dd5b170..6d674e24ad 100644 --- a/daemon/reload.go +++ b/daemon/reload.go @@ -10,8 +10,8 @@ import ( "github.com/containerd/log" "github.com/mitchellh/copystructure" "github.com/moby/moby/api/types/events" - "github.com/moby/moby/v2/daemon/config" + "github.com/moby/moby/v2/daemon/pkg/opts" ) // reloadTxn is used to defer side effects of a config reload. @@ -71,6 +71,7 @@ func (tx *reloadTxn) Rollback() error { // - Insecure registries // - Registry mirrors // - Daemon live restore +// - NRI enable and filesystem locations func (daemon *Daemon) Reload(conf *config.Config) error { daemon.configReload.Lock() defer daemon.configReload.Unlock() @@ -107,6 +108,7 @@ func (daemon *Daemon) Reload(conf *config.Config) error { daemon.reloadRegistryConfig, daemon.reloadLiveRestore, daemon.reloadNetworkDiagnosticPort, + daemon.reloadNRI, } { if err := reload(&txn, newCfg, conf, attributes); err != nil { return errors.Join(err, txn.Rollback()) @@ -276,3 +278,27 @@ func (daemon *Daemon) reloadFeatures(txn *reloadTxn, newCfg *configStore, conf * attributes["features"] = fmt.Sprintf("%v", newCfg.Features) return nil } + +// reloadNRI updates NRI configuration +func (daemon *Daemon) reloadNRI(txn *reloadTxn, newCfg *configStore, conf *config.Config, attributes map[string]string) error { + if daemon.nri == nil { + // Daemon not initialised. + return nil + } + if conf.IsValueSet("nri-opts") { + newCfg.Config.NRIOpts = conf.NRIOpts + } else { + newCfg.Config.NRIOpts = opts.NRIOpts{} + } + + commit, err := daemon.nri.PrepareReload(newCfg.NRIOpts) + if err != nil { + return err + } + if commit != nil { + txn.OnCommit(commit) + } + + attributes["nri-opts"] = fmt.Sprintf("%v", newCfg.NRIOpts) + return nil +} diff --git a/integration/daemon/nri/nri_test.go b/integration/daemon/nri/nri_test.go index 254c71805f..23efa1e896 100644 --- a/integration/daemon/nri/nri_test.go +++ b/integration/daemon/nri/nri_test.go @@ -3,6 +3,7 @@ package nri import ( "os" "path/filepath" + "strings" "testing" "github.com/containerd/nri/pkg/api" @@ -13,6 +14,7 @@ import ( "github.com/moby/moby/v2/internal/testutil/daemon" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" + "gotest.tools/v3/icmd" "gotest.tools/v3/skip" ) @@ -190,3 +192,80 @@ func TestNRIContainerCreateAddMount(t *testing.T) { }) } } + +func TestNRIReload(t *testing.T) { + skip.If(t, testEnv.IsRemoteDaemon, "cannot run daemon when remote daemon") + skip.If(t, testEnv.DaemonInfo.OSType == "windows", "cannot start a separate daemon with NRI enabled on Windows") + skip.If(t, testEnv.IsRootless) + + ctx := testutil.StartSpan(baseContext, t) + + const pluginName = "00-nri-test-plugin" + const envVar = "NRI_TEST" + + // Build and install a plugin. + pluginDir := t.TempDir() + res := icmd.RunCommand("go", "build", "-o", filepath.Join(pluginDir, pluginName), "./testdata/test_plugin.go") + res.Assert(t, icmd.Success) + + // Location and update function for the plugin config file. + pluginConfigDir := t.TempDir() + configurePlugin := func(envVal string) { + t.Helper() + err := os.WriteFile(filepath.Join(pluginConfigDir, pluginName+".conf"), + []byte(`{"env-var": "`+envVar+`", "env-val": "`+envVal+`"}`), 0o644) + assert.NilError(t, err) + } + + // Location and update function for the daemon config file, with empty initial config. + daemonConfigDir := t.TempDir() + daemonConfigFile := filepath.Join(daemonConfigDir, "daemon.json") + configureDaemon := func(json string) { + t.Helper() + err := os.WriteFile(daemonConfigFile, []byte(json), 0o644) + assert.NilError(t, err) + } + configureDaemon("{}") + + d := daemon.New(t) + d.StartWithBusybox(ctx, t, "--config-file", daemonConfigFile, "--iptables=false", "--ip6tables=false") + defer d.Stop(t) + c := d.NewClientT(t) + + // Function to check envVar in a container has value expEnvVal, or is absent if expEnvVal is "". + checkEnvVar := func(expEnvVal string) { + t.Helper() + res := container.RunAttach(ctx, t, c, container.WithAutoRemove, container.WithCmd("env")) + if expEnvVal == "" { + assert.Check(t, !strings.Contains(res.Stdout.String(), envVar), + "unexpected %q in env:\n%s", envVar, res.Stdout.String()) + } else { + assert.Check(t, is.Contains(res.Stdout.String(), envVar+"="+expEnvVal)) + } + } + + // Without NRI enabled, the env var should not be set. + checkEnvVar("") + + // Enable NRI in the daemon config and reload. + configureDaemon(`{"nri-opts": {"enable": true, "plugin-path": "` + pluginDir + `", "plugin-config-path": "` + pluginConfigDir + `"}}`) + configurePlugin("1") + err := d.ReloadConfig() + assert.NilError(t, err) + + // Now the env var should be set by the plugin. + checkEnvVar("1") + + // Reconfigure the plugin, check the new config takes effect after reload. + configurePlugin("2") + checkEnvVar("1") + err = d.ReloadConfig() + assert.NilError(t, err) + checkEnvVar("2") + + // Disable NRI by clearing the config and reloading. + configureDaemon("{}") + err = d.ReloadConfig() + assert.NilError(t, err) + checkEnvVar("") +} diff --git a/integration/daemon/nri/testdata/test_plugin.go b/integration/daemon/nri/testdata/test_plugin.go new file mode 100644 index 0000000000..bdb65e2368 --- /dev/null +++ b/integration/daemon/nri/testdata/test_plugin.go @@ -0,0 +1,95 @@ +/* + Based on https://github.com/containerd/nri/blob/main/plugins/template/ - which is ... + + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package main + +import ( + "context" + "encoding/json" + "flag" + "fmt" + "os" + + "github.com/containerd/nri/pkg/api" + "github.com/containerd/nri/pkg/stub" +) + +type config struct { + EnvVar string `json:"env-var"` + EnvVal string `json:"env-val"` +} + +type plugin struct { + stub stub.Stub + cfg config +} + +func (p *plugin) Configure(_ context.Context, config, runtime, version string) (stub.EventMask, error) { + if config == "" { + return 0, nil + } + + err := json.Unmarshal([]byte(config), &p.cfg) + if err != nil { + return 0, fmt.Errorf("failed to parse configuration: %w", err) + } + + return api.MustParseEventMask("CreateContainer"), nil +} + +func (p *plugin) CreateContainer(_ context.Context, pod *api.PodSandbox, ctr *api.Container) (*api.ContainerAdjustment, []*api.ContainerUpdate, error) { + env := []*api.KeyValue{{Key: "NRI_TEST_PLUGIN", Value: "wozere"}} + if p.cfg.EnvVar != "" { + env = append(env, &api.KeyValue{Key: p.cfg.EnvVar, Value: p.cfg.EnvVal}) + } + + adjustment := &api.ContainerAdjustment{Env: env} + updates := []*api.ContainerUpdate{} + + return adjustment, updates, nil +} + +func main() { + var ( + pluginName string + pluginIdx string + err error + ) + + flag.StringVar(&pluginName, "name", "", "plugin name to register to NRI") + flag.StringVar(&pluginIdx, "idx", "", "plugin index to register to NRI") + flag.Parse() + + p := &plugin{} + opts := []stub.Option{ + stub.WithOnClose(func() { os.Exit(0) }), + } + if pluginName != "" { + opts = append(opts, stub.WithPluginName(pluginName)) + } + if pluginIdx != "" { + opts = append(opts, stub.WithPluginIdx(pluginIdx)) + } + + if p.stub, err = stub.New(p, opts...); err != nil { + os.Exit(1) + } + if err = p.stub.Run(context.Background()); err != nil { + os.Exit(1) + } +}