Use BinaryName for v1 containerd runtime config

This fixes a bug where the runtime path for v1 containerd configs
was specified in the options.Runtime setting (which is used
for the default runtime) instead of options.BinaryName.

Signed-off-by: Evan Lezar <elezar@nvidia.com>

tools/container/containerd/config.go

@@ -28,9 +28,8 @@ type UpdateReverter interface {
 
 type config struct {
 	*toml.Tree
-	version   int64
-	cri       string
-	binaryKey string
+	version int64
+	cri     string
 }
 
 // update adds the specified runtime class to the the containerd config.
@@ -48,7 +47,7 @@ func (config *config) update(runtimeClass string, runtimeType string, runtimeBin
 		config.SetPath(runtimeClassPath, runc)
 	}
 
-	config.initRuntime(runtimeClassPath, runtimeType, runtimeBinary)
+	config.initRuntime(runtimeClassPath, runtimeType, "BinaryName", runtimeBinary)
 
 	if setAsDefault {
 		defaultRuntimeNamePath := config.defaultRuntimeNamePath()
@@ -83,7 +82,7 @@ func (config *config) revert(runtimeClass string) {
 
 // initRuntime creates a runtime config if it does not exist and ensures that the
 // runtimes binary path is specified.
-func (config *config) initRuntime(path []string, runtimeType string, binary string) {
+func (config *config) initRuntime(path []string, runtimeType string, binaryKey string, binary string) {
 	if config.GetPath(path) == nil {
 		config.SetPath(append(path, "runtime_type"), runtimeType)
 		config.SetPath(append(path, "runtime_root"), "")
@@ -91,7 +90,7 @@ func (config *config) initRuntime(path []string, runtimeType string, binary stri
 		config.SetPath(append(path, "privileged_without_host_devices"), false)
 	}
 
-	binaryPath := append(path, "options", config.binaryKey)
+	binaryPath := append(path, "options", binaryKey)
 	config.SetPath(binaryPath, binary)
 }
 
@@ -99,10 +98,6 @@ func (config config) runcPath() []string {
 	return config.runtimeClassPath("runc")
 }
 
-func (config config) runtimeClassBinaryPath(runtimeClass string) []string {
-	return append(config.runtimeClassPath(runtimeClass), "options", config.binaryKey)
-}
-
 func (config config) runtimeClassPath(runtimeClass string) []string {
 	return append(config.containerdPath(), "runtimes", runtimeClass)
 }

tools/container/containerd/config_v1.go

@@ -31,10 +31,9 @@ type configV1 struct {
 func newConfigV1(cfg *toml.Tree) UpdateReverter {
 	c := configV1{
 		config: config{
-			Tree:      cfg,
-			version:   1,
-			cri:       "cri",
-			binaryKey: "Runtime",
+			Tree:    cfg,
+			version: 1,
+			cri:     "cri",
 		},
 	}
 
@@ -68,7 +67,7 @@ func (config *configV1) Update(o *options) error {
 
 		log.Warnf("Setting default_runtime is deprecated")
 		defaultRuntimePath := append(config.containerdPath(), "default_runtime")
-		config.initRuntime(defaultRuntimePath, o.runtimeType, runtimeBinary)
+		config.initRuntime(defaultRuntimePath, o.runtimeType, "Runtime", runtimeBinary)
 	}
 	return nil
 }

tools/container/containerd/config_v1_test.go

@@ -110,7 +110,7 @@ func TestUpdateV1ConfigDefaultRuntime(t *testing.T) {
 		if tc.expectedDefaultRuntimeBinary == nil {
 			require.Nil(t, defaultRuntime, "%d: %v", i, tc)
 		} else {
-			expected, err := runtimeTomlConfigV1(tc.expectedDefaultRuntimeBinary.(string))
+			expected, err := defaultRuntimeTomlConfigV1(tc.expectedDefaultRuntimeBinary.(string))
 			require.NoError(t, err, "%d: %v", i, tc)
 
 			configContents, _ := toml.Marshal(defaultRuntime.(*toml.Tree))
@@ -291,7 +291,7 @@ func TestRevertV1Config(t *testing.T) {
 								"nvidia":              runtimeMapV1("/test/runtime/dir/nvidia-container-runtime"),
 								"nvidia-experimental": runtimeMapV1("/test/runtime/dir/nvidia-container-runtime-experimental"),
 							},
-							"default_runtime":      runtimeMapV1("/test/runtime/dir/nvidia-container-runtime"),
+							"default_runtime":      defaultRuntimeV1("/test/runtime/dir/nvidia-container-runtime"),
 							"default_runtime_name": "nvidia",
 						},
 					},
@@ -325,7 +325,11 @@ func runtimeTomlConfigV1(binary string) (*toml.Tree, error) {
 	return toml.TreeFromMap(runtimeMapV1(binary))
 }
 
-func runtimeMapV1(binary string) map[string]interface{} {
+func defaultRuntimeTomlConfigV1(binary string) (*toml.Tree, error) {
+	return toml.TreeFromMap(defaultRuntimeV1(binary))
+}
+
+func defaultRuntimeV1(binary string) map[string]interface{} {
 	return map[string]interface{}{
 		"runtime_type":                    runtimeType,
 		"runtime_root":                    "",
@@ -337,6 +341,18 @@ func runtimeMapV1(binary string) map[string]interface{} {
 	}
 }
 
+func runtimeMapV1(binary string) map[string]interface{} {
+	return map[string]interface{}{
+		"runtime_type":                    runtimeType,
+		"runtime_root":                    "",
+		"runtime_engine":                  "",
+		"privileged_without_host_devices": false,
+		"options": map[string]interface{}{
+			"BinaryName": binary,
+		},
+	}
+}
+
 func runcConfigMapV1(binary string) map[string]interface{} {
 	return map[string]interface{}{
 		"plugins": map[string]interface{}{
@@ -359,7 +375,7 @@ func runcRuntimeConfigMapV1(binary string) map[string]interface{} {
 		"privileged_without_host_devices": true,
 		"options": map[string]interface{}{
 			"runc-option": "value",
-			"Runtime":     binary,
+			"BinaryName":  binary,
 		},
 	}
 }

tools/container/containerd/config_v2.go

@@ -28,10 +28,9 @@ type configV2 struct {
 func newConfigV2(cfg *toml.Tree) UpdateReverter {
 	c := configV2{
 		config: config{
-			Tree:      cfg,
-			version:   2,
-			cri:       "io.containerd.grpc.v1.cri",
-			binaryKey: "BinaryName",
+			Tree:    cfg,
+			version: 2,
+			cri:     "io.containerd.grpc.v1.cri",
 		},
 	}