Merge pull request from neersighted/cdi_feature

cdi: use separate feature-flag
This commit is contained in:
Sebastiaan van Stijn 2024-01-17 14:11:41 +01:00 committed by GitHub
commit d3e08fe3cf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 32 additions and 44 deletions
cmd/dockerd
integration/container

View file

@ -270,13 +270,13 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
return errors.Wrap(err, "failed to validate authorization plugin")
}
// Note that CDI is not inherrently linux-specific, there are some linux-specific assumptions / implementations in the code that
// Note that CDI is not inherently linux-specific, there are some linux-specific assumptions / implementations in the code that
// queries the properties of device on the host as wel as performs the injection of device nodes and their access permissions into the OCI spec.
//
// In order to lift this restriction the following would have to be addressed:
// - Support needs to be added to the cdi package for injecting Windows devices: https://tags.cncf.io/container-device-interface/issues/28
// - The DeviceRequests API must be extended to non-linux platforms.
if runtime.GOOS == "linux" && cli.Config.Experimental {
if runtime.GOOS == "linux" && cli.Config.Features["cdi"] {
daemon.RegisterCDIDriver(cli.Config.CDISpecDirs...)
}
@ -611,8 +611,8 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) {
// 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.
if !conf.Features["cdi"] {
// If the CDI feature is not enabled, we clear the CDISpecDirs to ensure that CDI is disabled.
conf.CDISpecDirs = nil
}

View file

@ -227,44 +227,40 @@ 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",
description: "CDI enabled and no spec dirs specified returns default",
specDirs: nil,
experimental: true,
configContent: `{"features": {"cdi": true}}`,
expectedCDISpecDirs: []string{"/etc/cdi", "/var/run/cdi"},
},
{
description: "experimental and specified spec dirs are returned",
description: "CDI enabled and specified spec dirs are returned",
specDirs: []string{"/foo/bar", "/baz/qux"},
experimental: true,
configContent: `{"features": {"cdi": true}}`,
expectedCDISpecDirs: []string{"/foo/bar", "/baz/qux"},
},
{
description: "experimental and empty string as spec dir returns empty slice",
description: "CDI enabled and empty string as spec dir returns empty slice",
specDirs: []string{""},
experimental: true,
configContent: `{"features": {"cdi": true}}`,
expectedCDISpecDirs: []string{},
},
{
description: "experimental and empty config option returns empty slice",
configContent: `{"cdi-spec-dirs": []}`,
experimental: true,
description: "CDI enabled and empty config option returns empty slice",
configContent: `{"cdi-spec-dirs": [], "features": {"cdi": true}}`,
expectedCDISpecDirs: []string{},
},
{
description: "non-experimental and no spec dirs specified returns no cdi spec dirs",
description: "CDI disabled 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",
description: "CDI disabled and specified spec dirs returns no cdi spec dirs",
specDirs: []string{"/foo/bar", "/baz/qux"},
experimental: false,
expectedCDISpecDirs: nil,
},
}
@ -280,9 +276,6 @@ func TestCDISpecDirs(t *testing.T) {
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)

View file

@ -27,8 +27,12 @@ func TestCreateWithCDIDevices(t *testing.T) {
cwd, err := os.Getwd()
assert.NilError(t, err)
d := daemon.New(t, daemon.WithExperimental())
d.StartWithBusybox(ctx, t, "--cdi-spec-dir="+filepath.Join(cwd, "testdata", "cdi"))
configPath := filepath.Join(cwd, "daemon.json")
err = os.WriteFile(configPath, []byte(`{"features": {"cdi": true}}`), 0o644)
defer os.Remove(configPath)
assert.NilError(t, err)
d := daemon.New(t)
d.StartWithBusybox(ctx, t, "--config-file", configPath, "--cdi-spec-dir="+filepath.Join(cwd, "testdata", "cdi"))
defer d.Stop(t)
apiClient := d.NewClientT(t)
@ -72,55 +76,49 @@ func TestCDISpecDirsAreInSystemInfo(t *testing.T) {
testCases := []struct {
description string
config map[string]interface{}
experimental bool
specDirs []string
expectedInfoCDISpecDirs []string
}{
{
description: "experimental no spec dirs specified returns default",
experimental: true,
description: "CDI enabled with no spec dirs specified returns default",
config: map[string]interface{}{"features": map[string]bool{"cdi": true}},
specDirs: nil,
expectedInfoCDISpecDirs: []string{"/etc/cdi", "/var/run/cdi"},
},
{
description: "experimental specified spec dirs are returned",
experimental: true,
description: "CDI enabled with specified spec dirs are returned",
config: map[string]interface{}{"features": map[string]bool{"cdi": 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,
description: "CDI enabled with empty string as spec dir returns empty slice",
config: map[string]interface{}{"features": map[string]bool{"cdi": true}},
specDirs: []string{""},
expectedInfoCDISpecDirs: []string{},
},
{
description: "experimental empty config option returns empty slice",
experimental: true,
config: map[string]interface{}{"cdi-spec-dirs": []string{}},
description: "CDI enabled with empty config option returns empty slice",
config: map[string]interface{}{"features": map[string]bool{"cdi": true}, "cdi-spec-dirs": []string{}},
expectedInfoCDISpecDirs: []string{},
},
{
description: "non-experimental no spec dirs specified returns empty slice",
experimental: false,
description: "CDI disabled with no spec dirs specified returns empty slice",
specDirs: nil,
expectedInfoCDISpecDirs: []string{},
},
{
description: "non-experimental specified spec dirs returns empty slice",
experimental: false,
description: "CDI disabled with specified spec dirs returns empty slice",
specDirs: []string{"/foo/bar", "/baz/qux"},
expectedInfoCDISpecDirs: []string{},
},
{
description: "non-experimental empty string as spec dir returns empty slice",
experimental: false,
description: "CDI disabled with empty string as spec dir returns empty slice",
specDirs: []string{""},
expectedInfoCDISpecDirs: []string{},
},
{
description: "non-experimental empty config option returns empty slice",
experimental: false,
description: "CDI disabled with empty config option returns empty slice",
config: map[string]interface{}{"cdi-spec-dirs": []string{}},
expectedInfoCDISpecDirs: []string{},
},
@ -129,9 +127,6 @@ func TestCDISpecDirsAreInSystemInfo(t *testing.T) {
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