Builder.cdiSetupDevicesInSpecdefConfig(): use configured CDI dirs

Use the directories configured in containers.conf, now that
containers/common can tell us what they are, and now that it provides a
place to configure defaults for container tools, always override the
library's default set, even if it's empty, which means we do nothing.

Switch to the default CDI registry instead of the recenty-deprecated
non-global one.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

go.mod

@@ -10,7 +10,7 @@ go 1.20 // *****  ATTENTION  WARNING  CAUTION  DANGER  ******
 //         different distros and distro-versions build from
 //         this code, golang version consistency is
 //         desireable.  After manually updating to 1.21, a
-//         `toolchain` specificication should be added to pin
+//         `toolchain` specification should be added to pin
 //         the version and block auto-updates.  This does not
 //         block any future changes to the `go` value.
 //         Ref: Upstream discussion:
@@ -55,7 +55,7 @@ require (
 	golang.org/x/sys v0.19.0
 	golang.org/x/term v0.19.0
 	sigs.k8s.io/yaml v1.4.0
-	tags.cncf.io/container-device-interface v0.7.1
+	tags.cncf.io/container-device-interface v0.7.2
 )
 
 require (

go.sum

@@ -656,7 +656,7 @@ k8s.io/klog v1.0.0 h1:Pt+yjF5aB1xDSVbau4VsWe+dQNzA0qv1LlXdC2dF6Q8=
 k8s.io/klog v1.0.0/go.mod h1:4Bi6QPql/J/LkTDqv7R/cd3hPo4k2DG6Ptcz060Ez5I=
 sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
 sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY=
-tags.cncf.io/container-device-interface v0.7.1 h1:MATNCbAD1su9U6zwQe5BrQ2vGGp1GBayD70bYaxYCNE=
-tags.cncf.io/container-device-interface v0.7.1/go.mod h1:h1JVuOqTQVORp8DziaWKUCDNzAmN+zeCbqbqD30D0ZQ=
+tags.cncf.io/container-device-interface v0.7.2 h1:MLqGnWfOr1wB7m08ieI4YJ3IoLKKozEnnNYBtacDPQU=
+tags.cncf.io/container-device-interface v0.7.2/go.mod h1:Xb1PvXv2BhfNb3tla4r9JL129ck1Lxv9KuU6eVOfKto=
 tags.cncf.io/container-device-interface/specs-go v0.7.0 h1:w/maMGVeLP6TIQJVYT5pbqTi8SCw/iHZ+n4ignuGHqg=
 tags.cncf.io/container-device-interface/specs-go v0.7.0/go.mod h1:hMAwAbMZyBLdmYqWgYcKH0F/yctNpV3P35f+/088A80=

run_linux.go

@@ -47,6 +47,7 @@ import (
 	"golang.org/x/exp/slices"
 	"golang.org/x/sys/unix"
 	"tags.cncf.io/container-device-interface/pkg/cdi"
+	"tags.cncf.io/container-device-interface/pkg/parser"
 )
 
 var (
@@ -69,40 +70,55 @@ func setChildProcess() error {
 }
 
 func (b *Builder) cdiSetupDevicesInSpec(deviceSpecs []string, configDir string, spec *specs.Spec) ([]string, error) {
-	leftoverDevices := deviceSpecs
-	registry, err := cdi.NewCache()
-	if err != nil {
-		return nil, fmt.Errorf("creating CDI registry: %w", err)
-	}
 	var configDirs []string
+	defConfig, err := config.Default()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get container config: %w", err)
+	}
+	// The CDI cache prioritizes entries from directories that are later in
+	// the list of ones it scans, so start with our general config, then
+	// append values passed to us through API layers.
+	configDirs = slices.Clone(defConfig.Engine.CdiSpecDirs.Get())
 	if b.CDIConfigDir != "" {
 		configDirs = append(configDirs, b.CDIConfigDir)
 	}
 	if configDir != "" {
 		configDirs = append(configDirs, configDir)
 	}
-	// TODO: CdiSpecDirs will be in containers/common v0.59.0 or later?
-	// defConfig, err := config.Default()
-	// if err != nil {
-	//	return nil, fmt.Errorf("failed to get container config: %w", err)
-	// }
-	// configDirs = append(configDirs, defConfig.Engine.CdiSpecDirs.Get()...)
-	if len(configDirs) > 0 {
-		if err := registry.Configure(cdi.WithSpecDirs(configDirs...)); err != nil {
-			return nil, fmt.Errorf("CDI registry ignored configured directories %v: %w", configDirs, err)
+	if len(configDirs) == 0 {
+		// No directories to scan for CDI configuration means that CDI
+		// won't have any details for setting up any devices, so we
+		// don't need to be doing anything here.
+		return deviceSpecs, nil
+	}
+	var qualifiedDeviceSpecs, unqualifiedDeviceSpecs []string
+	for _, deviceSpec := range deviceSpecs {
+		if parser.IsQualifiedName(deviceSpec) {
+			qualifiedDeviceSpecs = append(qualifiedDeviceSpecs, deviceSpec)
+		} else {
+			unqualifiedDeviceSpecs = append(unqualifiedDeviceSpecs, deviceSpec)
 		}
 	}
-	if err := registry.Refresh(); err != nil {
-		logrus.Warnf("CDI registry refresh: %v", err)
+	if len(qualifiedDeviceSpecs) == 0 {
+		// None of the specified devices were in the form that would be
+		// handled by CDI, so we don't need to do anything here.
+		return deviceSpecs, nil
+	}
+	if err := cdi.Configure(cdi.WithSpecDirs(configDirs...)); err != nil {
+		return nil, fmt.Errorf("CDI default registry ignored configured directories %v: %w", configDirs, err)
+	}
+	leftoverDevices := slices.Clone(deviceSpecs)
+	if err := cdi.Refresh(); err != nil {
+		logrus.Warnf("CDI default registry refresh: %v", err)
 	} else {
-		leftoverDevices, err = registry.InjectDevices(spec, deviceSpecs...)
+		leftoverDevices, err = cdi.InjectDevices(spec, qualifiedDeviceSpecs...)
 		if err != nil {
-			logrus.Debugf("CDI device injection: %v, unresolved list %v", err, leftoverDevices)
+			return nil, fmt.Errorf("CDI device injection (leftover devices: %v): %w", leftoverDevices, err)
 		}
 	}
 	removed := slices.DeleteFunc(slices.Clone(deviceSpecs), func(t string) bool { return slices.Contains(leftoverDevices, t) })
-	logrus.Debugf("CDI taking care of devices %v, leaving devices %v", removed, leftoverDevices)
-	return leftoverDevices, nil
+	logrus.Debugf("CDI taking care of devices %v, leaving devices %v, skipped %v", removed, leftoverDevices, unqualifiedDeviceSpecs)
+	return append(leftoverDevices, unqualifiedDeviceSpecs...), nil
 }
 
 // Extract the device list so that we can still try to make it work if

tests/bud/cdi/Dockerfile

@@ -1,3 +1,4 @@
 FROM busybox
 RUN mkdir /etc/cdi
 RUN cp /dev/containers-cdi.yaml /etc/cdi/containers-cdi.yaml
+RUN wc /dev/outsidenull

tests/bud/cdi/containers-cdi.yaml

@@ -1,5 +1,5 @@
 ---
-cdiVersion: 0.5.0
+cdiVersion: 0.7.0
 devices:
   - name: all
     containerEdits:

tests/cdi.bats

@@ -12,7 +12,7 @@ load helpers
   echo === Begin CDI configuration in $cdidir/containers-cdi.yaml ===
   cat $cdidir/containers-cdi.yaml
   echo === End CDI configuration ===
-  run_buildah build $WITH_POLICY_JSON --cdi-config-dir=$cdidir --security-opt label=disable --device=containers.github.io/sample=all $BUDFILES/cdi
+  run_buildah build $WITH_POLICY_JSON --cdi-config-dir=$cdidir --security-opt label=disable --device=containers.github.io/sample=all --device=/dev/null:/dev/outsidenull:rwm $BUDFILES/cdi
 }
 
 @test "from with CDI" {
@@ -25,9 +25,9 @@ load helpers
   echo === Begin CDI configuration in $cdidir/containers-cdi.yaml ===
   cat $cdidir/containers-cdi.yaml
   echo === End CDI configuration ===
-  run_buildah from $WITH_POLICY_JSON --security-opt label=disable --cdi-config-dir=$cdidir --device=containers.github.io/sample=all busybox
+  run_buildah from $WITH_POLICY_JSON --security-opt label=disable --cdi-config-dir=$cdidir --device=containers.github.io/sample=all --device=/dev/null:/dev/outsidenull:rwm busybox
   cid="$output"
-  run_buildah run "$cid" cat /dev/containers-cdi.yaml
+  run_buildah run "$cid" cat /dev/containers-cdi.yaml /dev/outsidenull
 }
 
 @test "run with CDI" {

vendor/modules.txt

@@ -998,7 +998,7 @@ k8s.io/klog
 ## explicit; go 1.12
 sigs.k8s.io/yaml
 sigs.k8s.io/yaml/goyaml.v2
-# tags.cncf.io/container-device-interface v0.7.1
+# tags.cncf.io/container-device-interface v0.7.2
 ## explicit; go 1.20
 tags.cncf.io/container-device-interface/internal/validation
 tags.cncf.io/container-device-interface/internal/validation/k8s