Merge pull request #51711 from robmry/nri-config-reload

NRI: config reload

daemon/internal/nri/nri.go

@@ -47,10 +47,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
 }
 
@@ -123,6 +123,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.
 //

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
+}

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"
 )
 
@@ -250,3 +252,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("")
+}

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)
+	}
+}