dockerfile: support optional cdi devices

Signed-off-by: CrazyMax <[email protected]>

Author: crazy-max

frontend/dockerfile/dockerfile2llb/convert_rundevice.go

@@ -9,9 +9,14 @@ import (
 
 func dispatchRunDevices(c *instructions.RunCommand) ([]llb.RunOption, error) {
 	var out []llb.RunOption
-	devices := instructions.GetDevices(c)
-	for _, device := range devices {
-		out = append(out, llb.AddCDIDevice(llb.CDIDeviceName(device), llb.CDIDeviceOptional))
+	for _, device := range instructions.GetDevices(c) {
+		deviceOpts := []llb.CDIDeviceOption{
+			llb.CDIDeviceName(device.Name),
+		}
+		if !device.Required {
+			deviceOpts = append(deviceOpts, llb.CDIDeviceOptional)
+		}
+		out = append(out, llb.AddCDIDevice(deviceOpts...))
 	}
 	return out, nil
 }

frontend/dockerfile/dockerfile_rundevice_test.go

@@ -60,7 +60,8 @@ COPY --from=base /foo.env /
 
 	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
 		FrontendAttrs: map[string]string{
-			"device": "vendor1.com/device=foo",
+			"device:0": "vendor1.com/device=foo,required",
+			"device:1": "vendor2.com/device=bar",
 		},
 		LocalMounts: map[string]fsutil.FS{
 			dockerui.DefaultLocalNameDockerfile: dir,
@@ -100,7 +101,9 @@ devices:
 
 	dockerfile := []byte(`
 FROM busybox AS base
-RUN --device=vendor1.com/device=foo env|sort | tee foo.env
+RUN --device=vendor1.com/device=foo,required \
+    --device=vendor2.com/device=bar \
+    env|sort | tee foo.env
 FROM scratch
 COPY --from=base /foo.env /
 `)

frontend/dockerfile/instructions/commands_rundevice.go

@@ -1,10 +1,16 @@
 package instructions
 
 import (
+	"strconv"
+	"strings"
+
+	"github.com/moby/buildkit/util/suggest"
 	"github.com/pkg/errors"
+	"github.com/tonistiigi/go-csvvalue"
+	"tags.cncf.io/container-device-interface/pkg/parser"
 )
 
-var deviceKey = "dockerfile/run/device"
+var devicesKey = "dockerfile/run/devices"
 
 func init() {
 	parseRunPreHooks = append(parseRunPreHooks, runDevicePreHook)
@@ -14,7 +20,7 @@ func init() {
 func runDevicePreHook(cmd *RunCommand, req parseRequest) error {
 	st := &deviceState{}
 	st.flag = req.flags.AddStrings("device")
-	cmd.setExternalValue(deviceKey, st)
+	cmd.setExternalValue(devicesKey, st)
 	return nil
 }
 
@@ -27,23 +33,95 @@ func setDeviceState(cmd *RunCommand) error {
 	if st == nil {
 		return errors.Errorf("no device state")
 	}
-	st.names = st.flag.StringValues
+	devices := make([]*Device, len(st.flag.StringValues))
+	for i, str := range st.flag.StringValues {
+		d, err := ParseDevice(str)
+		if err != nil {
+			return err
+		}
+		devices[i] = d
+	}
+	st.devices = devices
 	return nil
 }
 
 func getDeviceState(cmd *RunCommand) *deviceState {
-	v := cmd.getExternalValue(deviceKey)
+	v := cmd.getExternalValue(devicesKey)
 	if v == nil {
 		return nil
 	}
 	return v.(*deviceState)
 }
 
-func GetDevices(cmd *RunCommand) []string {
-	return getDeviceState(cmd).names
+func GetDevices(cmd *RunCommand) []*Device {
+	return getDeviceState(cmd).devices
 }
 
 type deviceState struct {
-	flag  *Flag
-	names []string
+	flag    *Flag
+	devices []*Device
+}
+
+type Device struct {
+	Name     string
+	Required bool
+}
+
+func ParseDevice(val string) (*Device, error) {
+	fields, err := csvvalue.Fields(val, nil)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to parse csv devices")
+	}
+
+	d := &Device{}
+
+	for i, field := range fields {
+		// check if the first field is a valid device name
+		var firstFieldErr error
+		if i == 0 {
+			if _, _, _, firstFieldErr = parser.ParseQualifiedName(field); firstFieldErr == nil {
+				d.Name = field
+				continue
+			}
+		}
+
+		key, value, ok := strings.Cut(field, "=")
+		key = strings.ToLower(key)
+
+		if !ok {
+			if len(fields) == 1 && firstFieldErr != nil {
+				return nil, errors.Wrapf(firstFieldErr, "invalid device name %s", field)
+			}
+			switch key {
+			case "required":
+				d.Required = true
+				continue
+			default:
+				// any other option requires a value.
+				return nil, errors.Errorf("invalid field '%s' must be a key=value pair", field)
+			}
+		}
+
+		switch key {
+		case "name":
+			if d.Name != "" {
+				return nil, errors.Errorf("device name already set to %s", d.Name)
+			}
+			d.Name = value
+		case "required":
+			d.Required, err = strconv.ParseBool(value)
+			if err != nil {
+				return nil, errors.Errorf("invalid value for %s: %s", key, value)
+			}
+		default:
+			allKeys := []string{"name", "required"}
+			return nil, suggest.WrapError(errors.Errorf("unexpected key '%s' in '%s'", key, field), key, allKeys, true)
+		}
+	}
+
+	if _, _, _, err := parser.ParseQualifiedName(d.Name); err != nil {
+		return nil, errors.Wrapf(err, "invalid device name %s", d.Name)
+	}
+
+	return d, nil
 }

frontend/dockerfile/instructions/commands_rundevice_test.go

@@ -0,0 +1,74 @@
+package instructions
+
+import (
+	"testing"
+
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseDevice(t *testing.T) {
+	cases := []struct {
+		input       string
+		expected    *Device
+		expectedErr error
+	}{
+		{
+			input:       "vendor1.com/device=foo",
+			expected:    &Device{Name: "vendor1.com/device=foo", Required: false},
+			expectedErr: nil,
+		},
+		{
+			input:       "vendor1.com/device=foo,required",
+			expected:    &Device{Name: "vendor1.com/device=foo", Required: true},
+			expectedErr: nil,
+		},
+		{
+			input:       "vendor1.com/device=foo,required=true",
+			expected:    &Device{Name: "vendor1.com/device=foo", Required: true},
+			expectedErr: nil,
+		},
+		{
+			input:       "vendor1.com/device=foo,required=false",
+			expected:    &Device{Name: "vendor1.com/device=foo", Required: false},
+			expectedErr: nil,
+		},
+		{
+			input:       "name=vendor1.com/device=foo",
+			expected:    &Device{Name: "vendor1.com/device=foo", Required: false},
+			expectedErr: nil,
+		},
+		{
+			input:       "name=vendor1.com/device=foo,required",
+			expected:    &Device{Name: "vendor1.com/device=foo", Required: true},
+			expectedErr: nil,
+		},
+		{
+			input:       "vendor1.com/device=foo,name=vendor2.com/device=bar",
+			expected:    nil,
+			expectedErr: errors.New("device name already set to vendor1.com/device=foo"),
+		},
+		{
+			input:       "invalid-device-name",
+			expected:    nil,
+			expectedErr: errors.New(`invalid device name invalid-device-name: unqualified device "invalid-device-name", missing vendor`),
+		},
+		{
+			input:       "name=invalid-device-name",
+			expected:    nil,
+			expectedErr: errors.New(`invalid device name invalid-device-name: unqualified device "invalid-device-name", missing vendor`),
+		},
+	}
+	for _, tt := range cases {
+		t.Run(tt.input, func(t *testing.T) {
+			result, err := ParseDevice(tt.input)
+			if tt.expectedErr != nil {
+				require.Error(t, err)
+				require.EqualError(t, err, tt.expectedErr.Error())
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}

frontend/dockerui/attr.go

@@ -1,7 +1,6 @@
 package dockerui
 
 import (
-	"encoding/csv"
 	"net"
 	"strconv"
 	"strings"
@@ -10,11 +9,11 @@ import (
 	"github.com/containerd/platforms"
 	"github.com/docker/go-units"
 	"github.com/moby/buildkit/client/llb"
+	"github.com/moby/buildkit/frontend/dockerfile/instructions"
 	"github.com/moby/buildkit/solver/pb"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/tonistiigi/go-csvvalue"
-	"tags.cncf.io/container-device-interface/pkg/parser"
 )
 
 func parsePlatforms(v string) ([]ocispecs.Platform, error) {
@@ -99,22 +98,19 @@ func parseUlimits(v string) ([]*pb.Ulimit, error) {
 	return out, nil
 }
 
-func parseDevices(v string) ([]*pb.CDIDevice, error) {
-	if v == "" {
+func parseDevices(v map[string]string) ([]*pb.CDIDevice, error) {
+	if v == nil {
 		return nil, nil
 	}
 	out := make([]*pb.CDIDevice, 0)
-	csvReader := csv.NewReader(strings.NewReader(v))
-	names, err := csvReader.Read()
-	if err != nil {
-		return nil, err
-	}
-	for _, name := range names {
-		if _, _, _, err := parser.ParseQualifiedName(name); err != nil {
-			return nil, errors.Wrapf(err, "invalid CDI device name %q", name)
+	for _, attrs := range v {
+		device, err := instructions.ParseDevice(attrs)
+		if err != nil {
+			return nil, err
 		}
 		out = append(out, &pb.CDIDevice{
-			Name: name,
+			Name:     device.Name,
+			Optional: !device.Required,
 		})
 	}
 	return out, nil

frontend/dockerui/config.go

@@ -28,6 +28,7 @@ const (
 	buildArgPrefix       = "build-arg:"
 	labelPrefix          = "label:"
 	localSessionIDPrefix = "local-sessionid:"
+	devicePrefix         = "device:"
 
 	keyTarget           = "target"
 	keyCgroupParent     = "cgroup-parent"
@@ -40,7 +41,6 @@ const (
 	keyShmSize          = "shm-size"
 	keyTargetPlatform   = "platform"
 	keyUlimit           = "ulimit"
-	keyDevice           = "device"
 	keyCacheFrom        = "cache-from"    // for registry only. deprecated in favor of keyCacheImports
 	keyCacheImports     = "cache-imports" // JSON representation of []CacheOptionsEntry
 
@@ -189,9 +189,9 @@ func (bc *Client) init() error {
 	}
 	bc.Ulimits = ulimits
 
-	devices, err := parseDevices(opts[keyDevice])
+	devices, err := parseDevices(filter(opts, devicePrefix))
 	if err != nil {
-		return errors.Wrap(err, "failed to parse device")
+		return errors.Wrap(err, "failed to parse devices")
 	}
 	bc.Devices = devices