Browse Source

cdi: use separate feature-flag

Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Bjorn Neergaard 1 year ago
parent
commit
d22c775e04
3 changed files with 32 additions and 44 deletions
  1. 4 4
      cmd/dockerd/daemon.go
  2. 10 17
      cmd/dockerd/daemon_test.go
  3. 18 23
      integration/container/cdi_test.go

+ 4 - 4
cmd/dockerd/daemon.go

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

+ 10 - 17
cmd/dockerd/daemon_test.go

@@ -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)

+ 18 - 23
integration/container/cdi_test.go

@@ -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