Merge pull request #46004 from elezar/add-cdi-spec-dirs-to-info

Add CDISpecDirs to Info output

api/swagger.yaml

@@ -5301,7 +5301,25 @@ definitions:
           - "WARNING: No memory limit support"
           - "WARNING: bridge-nf-call-iptables is disabled"
           - "WARNING: bridge-nf-call-ip6tables is disabled"
+      CDISpecDirs:
+        description: |
+          List of directories where (Container Device Interface) CDI
+          specifications are located.
 
+          These specifications define vendor-specific modifications to an OCI
+          runtime specification for a container being created.
+
+          An empty list indicates that CDI device injection is disabled.
+
+          Note that since using CDI device injection requires the daemon to have
+          experimental enabled. For non-experimental daemons an empty list will
+          always be returned.
+        type: "array"
+        items:
+          type: "string"
+        example:
+          - "/etc/cdi"
+          - "/var/run/cdi"
 
   # PluginsInfo is a temp struct holding Plugins name
   # registered with docker daemon. It is used by Info struct

api/types/system/info.go

@@ -73,6 +73,7 @@ type Info struct {
 	SecurityOptions     []string
 	ProductLicense      string               `json:",omitempty"`
 	DefaultAddressPools []NetworkAddressPool `json:",omitempty"`
+	CDISpecDirs         []string
 
 	// Legacy API fields for older API versions.
 	legacyFields

cmd/dockerd/daemon.go

@@ -251,9 +251,7 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
 	// - Support needs to be added to the cdi package for injecting Windows devices: https://github.com/container-orchestrated-devices/container-device-interface/issues/28
 	// - The DeviceRequests API must be extended to non-linux platforms.
 	if runtime.GOOS == "linux" && cli.Config.Experimental {
-		daemon.RegisterCDIDriver(
-			cdi.WithSpecDirs(cli.Config.CDISpecDirs...),
-		)
+		daemon.RegisterCDIDriver(cli.Config.CDISpecDirs...)
 	}
 
 	cli.d = d
@@ -544,6 +542,18 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) {
 		return nil, err
 	}
 
+	if conf.CDISpecDirs == nil {
+		// If the CDISpecDirs is not set at this stage, we set it to the default.
+		conf.CDISpecDirs = append([]string(nil), cdi.DefaultSpecDirs...)
+	} else if len(conf.CDISpecDirs) == 1 && conf.CDISpecDirs[0] == "" {
+		// If CDISpecDirs is set to an empty string, we clear it to ensure that CDI is disabled.
+		conf.CDISpecDirs = nil
+	}
+	if !conf.Experimental {
+		// If experimental mode is not set, we clear the CDISpecDirs to ensure that CDI is disabled.
+		conf.CDISpecDirs = nil
+	}
+
 	return conf, nil
 }
 

cmd/dockerd/daemon_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/containerd/containerd/log"
 	"github.com/docker/docker/daemon/config"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
 	"gotest.tools/v3/assert"
@@ -223,3 +224,72 @@ func TestConfigureDaemonLogs(t *testing.T) {
 	// TODO (thaJeztah): add more aliases in log package
 	assert.Check(t, is.Equal(logrus.WarnLevel, log.GetLevel()))
 }
+
+func TestCDISpecDirs(t *testing.T) {
+	testCases := []struct {
+		description         string
+		configContent       string
+		experimental        bool
+		specDirs            []string
+		expectedCDISpecDirs []string
+	}{
+		{
+			description:         "experimental and no spec dirs specified returns default",
+			specDirs:            nil,
+			experimental:        true,
+			expectedCDISpecDirs: []string{"/etc/cdi", "/var/run/cdi"},
+		},
+		{
+			description:         "experimental and specified spec dirs are returned",
+			specDirs:            []string{"/foo/bar", "/baz/qux"},
+			experimental:        true,
+			expectedCDISpecDirs: []string{"/foo/bar", "/baz/qux"},
+		},
+		{
+			description:         "experimental and empty string as spec dir returns empty slice",
+			specDirs:            []string{""},
+			experimental:        true,
+			expectedCDISpecDirs: []string{},
+		},
+		{
+			description:         "experimental and empty config option returns empty slice",
+			configContent:       `{"cdi-spec-dirs": []}`,
+			experimental:        true,
+			expectedCDISpecDirs: []string{},
+		},
+		{
+			description:         "non-experimental and no spec dirs specified returns no cdi spec dirs",
+			specDirs:            nil,
+			experimental:        false,
+			expectedCDISpecDirs: nil,
+		},
+		{
+			description:         "non-experimental and specified spec dirs returns no cdi spec dirs",
+			specDirs:            []string{"/foo/bar", "/baz/qux"},
+			experimental:        false,
+			expectedCDISpecDirs: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			tempFile := fs.NewFile(t, "config", fs.WithContent(tc.configContent))
+			defer tempFile.Remove()
+
+			opts := defaultOptions(t, tempFile.Path())
+
+			flags := opts.flags
+			for _, specDir := range tc.specDirs {
+				assert.Check(t, flags.Set("cdi-spec-dir", specDir))
+			}
+			if tc.experimental {
+				assert.Check(t, flags.Set("experimental", "true"))
+			}
+
+			loadedConfig, err := loadDaemonCliConfig(opts)
+			assert.NilError(t, err)
+
+			assert.Check(t, is.DeepEqual(tc.expectedCDISpecDirs, loadedConfig.CDISpecDirs, cmpopts.EquateEmpty()))
+		})
+	}
+}

daemon/cdi.go

@@ -18,21 +18,29 @@ type cdiHandler struct {
 
 // RegisterCDIDriver registers the CDI device driver.
 // The driver injects CDI devices into an incoming OCI spec and is called for DeviceRequests associated with CDI devices.
-func RegisterCDIDriver(opts ...cdi.Option) {
-	cache, err := cdi.NewCache(opts...)
+// If the list of CDI spec directories is empty, the driver is not registered.
+func RegisterCDIDriver(cdiSpecDirs ...string) {
+	driver := newCDIDeviceDriver(cdiSpecDirs...)
+
+	registerDeviceDriver("cdi", driver)
+}
+
+// newCDIDeviceDriver creates a new CDI device driver.
+// If the creation of the CDI cache fails, a driver is returned that will return an error on an injection request.
+func newCDIDeviceDriver(cdiSpecDirs ...string) *deviceDriver {
+	cache, err := createCDICache(cdiSpecDirs...)
 	if err != nil {
-		log.G(context.TODO()).WithError(err).Error("CDI registry initialization failed")
+		log.G(context.TODO()).WithError(err)
 		// We create a spec updater that always returns an error.
 		// This error will be returned only when a CDI device is requested.
-		// This ensures that daemon startup is not blocked by a CDI registry initialization failure.
+		// This ensures that daemon startup is not blocked by a CDI registry initialization failure or being disabled
+		// by configuratrion.
 		errorOnUpdateSpec := func(s *specs.Spec, dev *deviceInstance) error {
-			return fmt.Errorf("CDI device injection failed due to registry initialization failure: %w", err)
+			return fmt.Errorf("CDI device injection failed: %w", err)
 		}
-		driver := &deviceDriver{
+		return &deviceDriver{
 			updateSpec: errorOnUpdateSpec,
 		}
-		registerDeviceDriver("cdi", driver)
-		return
 	}
 
 	// We construct a spec updates that injects CDI devices into the OCI spec using the initialized registry.
@@ -40,11 +48,24 @@ func RegisterCDIDriver(opts ...cdi.Option) {
 		registry: cache,
 	}
 
-	driver := &deviceDriver{
+	return &deviceDriver{
 		updateSpec: c.injectCDIDevices,
 	}
+}
 
-	registerDeviceDriver("cdi", driver)
+// createCDICache creates a CDI cache for the specified CDI specification directories.
+// If the list of CDI specification directories is empty or the creation of the CDI cache fails, an error is returned.
+func createCDICache(cdiSpecDirs ...string) (*cdi.Cache, error) {
+	if len(cdiSpecDirs) == 0 {
+		return nil, fmt.Errorf("No CDI specification directories specified")
+	}
+
+	cache, err := cdi.NewCache(cdi.WithSpecDirs(cdiSpecDirs...))
+	if err != nil {
+		return nil, fmt.Errorf("CDI registry initialization failure: %w", err)
+	}
+
+	return cache, nil
 }
 
 // injectCDIDevices injects a set of CDI devices into the specified OCI specification.

daemon/config/config.go

@@ -14,7 +14,6 @@ import (
 	"golang.org/x/text/encoding/unicode"
 	"golang.org/x/text/transform"
 
-	"github.com/container-orchestrated-devices/container-device-interface/pkg/cdi"
 	"github.com/containerd/containerd/log"
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/registry"
@@ -288,7 +287,6 @@ func New() (*Config, error) {
 			ContainerdNamespace:       DefaultContainersNamespace,
 			ContainerdPluginNamespace: DefaultPluginNamespace,
 			DefaultRuntime:            StockRuntimeName,
-			CDISpecDirs:               append([]string(nil), cdi.DefaultSpecDirs...),
 		},
 	}
 

daemon/info.go

@@ -62,6 +62,7 @@ func (daemon *Daemon) SystemInfo() *system.Info {
 		NoProxy:            getConfigOrEnv(cfg.NoProxy, "NO_PROXY", "no_proxy"),
 		LiveRestoreEnabled: cfg.LiveRestoreEnabled,
 		Isolation:          daemon.defaultIsolation,
+		CDISpecDirs:        promoteNil(cfg.CDISpecDirs),
 	}
 
 	daemon.fillContainerStates(v)
@@ -309,3 +310,12 @@ func getConfigOrEnv(config string, env ...string) string {
 	}
 	return getEnvAny(env...)
 }
+
+// promoteNil converts a nil slice to an empty slice of that type.
+// A non-nil slice is returned as is.
+func promoteNil[S ~[]E, E any](s S) S {
+	if s == nil {
+		return S{}
+	}
+	return s
+}

docs/api/version-history.md

@@ -34,6 +34,10 @@ keywords: "API, Docker, rcli, REST, documentation"
   `BindOptions.ReadOnlyNonRecursive` and `BindOptions.ReadOnlyForceRecursive` to customize the behavior.
 * `POST /containers/create` now accepts a `HealthConfig.StartInterval` to set the
   interval for health checks during the start period.
+* `GET /info` now includes a `CDISpecDirs` field indicating the configured CDI
+  specifications directories. The use of the applied setting requires the daemon
+  to have expermental enabled, and for non-experimental daemons an empty list is
+  always returned.
 
 ## v1.43 API changes
 
@@ -103,7 +107,7 @@ keywords: "API, Docker, rcli, REST, documentation"
   a default.
 
   This change is not versioned, and affects all API versions if the daemon has
-  this patch. 
+  this patch.
 * `GET /_ping` and `HEAD /_ping` now return a `Swarm` header, which allows a
   client to detect if Swarm is enabled on the daemon, without having to call
   additional endpoints.
@@ -126,7 +130,7 @@ keywords: "API, Docker, rcli, REST, documentation"
   versioned, and affects all API versions if the daemon has this patch.
 * `GET /containers/{id}/attach`, `GET /exec/{id}/start`, `GET /containers/{id}/logs`
   `GET /services/{id}/logs` and `GET /tasks/{id}/logs` now set Content-Type header
-  to `application/vnd.docker.multiplexed-stream` when a multiplexed stdout/stderr 
+  to `application/vnd.docker.multiplexed-stream` when a multiplexed stdout/stderr
   stream is sent to client, `application/vnd.docker.raw-stream` otherwise.
 * `POST /volumes/create` now accepts a new `ClusterVolumeSpec` to create a cluster
   volume (CNI). This option can only be used if the daemon is a Swarm manager.
@@ -139,7 +143,7 @@ keywords: "API, Docker, rcli, REST, documentation"
 * Volume information returned by `GET /volumes/{name}`, `GET /volumes` and
   `GET /system/df` can now contain a `ClusterVolume` if the volume is a cluster
   volume (requires the daemon to be a Swarm manager).
-* The `Volume` type, as returned by `Added new `ClusterVolume` fields 
+* The `Volume` type, as returned by `Added new `ClusterVolume` fields
 * Added a new `PUT /volumes{name}` endpoint to update cluster volumes (CNI).
   Cluster volumes are only supported if the daemon is a Swarm manager.
 * `GET /containers/{name}/attach/ws` endpoint now accepts `stdin`, `stdout` and
@@ -355,7 +359,7 @@ keywords: "API, Docker, rcli, REST, documentation"
 
 [Docker Engine API v1.36](https://docs.docker.com/engine/api/v1.36/) documentation
 
-* `Get /events` now return `exec_die` event when an exec process terminates.  
+* `Get /events` now return `exec_die` event when an exec process terminates.
 
 
 ## v1.35 API changes
@@ -563,7 +567,7 @@ keywords: "API, Docker, rcli, REST, documentation"
 * `POST /services/create` and `POST /services/(id or name)/update` now accept the `TTY` parameter, which allocate a pseudo-TTY in container.
 * `POST /services/create` and `POST /services/(id or name)/update` now accept the `DNSConfig` parameter, which specifies DNS related configurations in resolver configuration file (resolv.conf) through `Nameservers`, `Search`, and `Options`.
 * `POST /services/create` and `POST /services/(id or name)/update` now support
-  `node.platform.arch` and `node.platform.os` constraints in the services 
+  `node.platform.arch` and `node.platform.os` constraints in the services
   `TaskSpec.Placement.Constraints` field.
 * `GET /networks/(id or name)` now includes IP and name of all peers nodes for swarm mode overlay networks.
 * `GET /plugins` list plugins.

integration/container/cdi_test.go

@@ -3,6 +3,7 @@ package container // import "github.com/docker/docker/integration/container"
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"io"
 	"os"
 	"path/filepath"
@@ -62,3 +63,99 @@ func TestCreateWithCDIDevices(t *testing.T) {
 	outlines := strings.Split(actualStdout.String(), "\n")
 	assert.Assert(t, is.Contains(outlines, "FOO=injected"))
 }
+
+func TestCDISpecDirsAreInSystemInfo(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows") // d.Start fails on Windows with `protocol not available`
+	// TODO: This restriction can be relaxed with https://github.com/moby/moby/pull/46158
+	skip.If(t, testEnv.IsRootless, "the t.TempDir test creates a folder with incorrect permissions for rootless")
+
+	testCases := []struct {
+		description             string
+		config                  map[string]interface{}
+		experimental            bool
+		specDirs                []string
+		expectedInfoCDISpecDirs []string
+	}{
+		{
+			description:             "experimental no spec dirs specified returns default",
+			experimental:            true,
+			specDirs:                nil,
+			expectedInfoCDISpecDirs: []string{"/etc/cdi", "/var/run/cdi"},
+		},
+		{
+			description:             "experimental specified spec dirs are returned",
+			experimental:            true,
+			specDirs:                []string{"/foo/bar", "/baz/qux"},
+			expectedInfoCDISpecDirs: []string{"/foo/bar", "/baz/qux"},
+		},
+		{
+			description:             "experimental empty string as spec dir returns empty slice",
+			experimental:            true,
+			specDirs:                []string{""},
+			expectedInfoCDISpecDirs: []string{},
+		},
+		{
+			description:             "experimental empty config option returns empty slice",
+			experimental:            true,
+			config:                  map[string]interface{}{"cdi-spec-dirs": []string{}},
+			expectedInfoCDISpecDirs: []string{},
+		},
+		{
+			description:             "non-experimental no spec dirs specified returns empty slice",
+			experimental:            false,
+			specDirs:                nil,
+			expectedInfoCDISpecDirs: []string{},
+		},
+		{
+			description:             "non-experimental specified spec dirs returns empty slice",
+			experimental:            false,
+			specDirs:                []string{"/foo/bar", "/baz/qux"},
+			expectedInfoCDISpecDirs: []string{},
+		},
+		{
+			description:             "non-experimental empty string as spec dir returns empty slice",
+			experimental:            false,
+			specDirs:                []string{""},
+			expectedInfoCDISpecDirs: []string{},
+		},
+		{
+			description:             "non-experimental empty config option returns empty slice",
+			experimental:            false,
+			config:                  map[string]interface{}{"cdi-spec-dirs": []string{}},
+			expectedInfoCDISpecDirs: []string{},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			var opts []daemon.Option
+			if tc.experimental {
+				opts = append(opts, daemon.WithExperimental())
+			}
+			d := daemon.New(t, opts...)
+
+			var args []string
+			for _, specDir := range tc.specDirs {
+				args = append(args, "--cdi-spec-dir="+specDir)
+			}
+			if tc.config != nil {
+				configPath := filepath.Join(t.TempDir(), "daemon.json")
+
+				configFile, err := os.Create(configPath)
+				assert.NilError(t, err)
+				defer configFile.Close()
+
+				err = json.NewEncoder(configFile).Encode(tc.config)
+				assert.NilError(t, err)
+
+				args = append(args, "--config-file="+configPath)
+			}
+			d.Start(t, args...)
+			defer d.Stop(t)
+
+			info := d.Info(t)
+
+			assert.Check(t, is.DeepEqual(tc.expectedInfoCDISpecDirs, info.CDISpecDirs))
+		})
+	}
+}