Skip to content

Commit 8d6e5cf

Browse files
cdesiniotiscdesiniotis
authored
Merge pull request #1311 from cdesiniotis/add-existing-runtimes-to-containerd-drop-in
Add CRI plugin config from source containerd config to drop-in file
2 parents 3592084 + 598c632 commit 8d6e5cf

File tree

4 files changed

+145
-5
lines changed

4 files changed

+145
-5
lines changed

‎cmd/nvidia-ctk-installer/container/container.go‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package container
1818

1919
import (
20+
"errors"
2021
"fmt"
2122
"os"
2223
"os/exec"
@@ -69,7 +70,23 @@ func (o Options) Unconfigure(cfg engine.Interface) error {
6970
if err != nil {
7071
return fmt.Errorf("unable to update config: %v", err)
7172
}
72-
return o.flush(cfg)
73+
74+
if err := o.flush(cfg); err != nil {
75+
return err
76+
}
77+
78+
if o.DropInConfig == "" {
79+
return nil
80+
}
81+
// When a drop-in config is used, we remove the drop-in file explicitly.
82+
// This is require for cases where we may have to include other contents
83+
// in the drop-in file and as such it may not be empty when we flush it.
84+
err = os.Remove(o.DropInConfig)
85+
if err != nil && !errors.Is(err, os.ErrNotExist) {
86+
return fmt.Errorf("failed to remove drop-in config file: %w", err)
87+
}
88+
89+
return nil
7390
}
7491

7592
// flush flushes the specified config to disk

‎cmd/nvidia-ctk-installer/container/runtime/containerd/config_test.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,7 @@ version = 2
789789
enable_cdi = true
790790
791791
[plugins."io.containerd.grpc.v1.cri".containerd]
792+
default_runtime_name = "runc"
792793
793794
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes]
794795
@@ -809,6 +810,12 @@ version = 2
809810
810811
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.nvidia-legacy.options]
811812
BinaryName = "/usr/bin/nvidia-container-runtime.legacy"
813+
814+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
815+
runtime_type = "io.containerd.runc.v2"
816+
817+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
818+
BinaryName = "/usr/bin/runc"
812819
`
813820
require.Equal(t, expectedDropIn, string(actualDropIn))
814821
return nil
@@ -954,6 +961,12 @@ version = 2
954961
955962
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.nvidia-legacy.options]
956963
BinaryName = "/usr/bin/nvidia-container-runtime.legacy"
964+
965+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
966+
runtime_type = "io.containerd.runc.v2"
967+
968+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
969+
BinaryName = "/usr/bin/runc"
957970
`
958971
require.Equal(t, expectedDropIn, string(actualDropIn))
959972
return nil
@@ -1106,9 +1119,17 @@ version = 2
11061119
enable_cdi = true
11071120
11081121
[plugins."io.containerd.grpc.v1.cri".containerd]
1122+
default_runtime_name = "runc"
1123+
snapshotter = "overlayfs"
11091124
11101125
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes]
11111126
1127+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.custom]
1128+
runtime_type = "io.containerd.custom.v1"
1129+
1130+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.custom.options]
1131+
TypeUrl = "custom.runtime/options"
1132+
11121133
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.nvidia]
11131134
container_annotations = ["cdi.k8s.io*"]
11141135
runtime_type = "io.containerd.runc.v2"
@@ -1132,6 +1153,20 @@ version = 2
11321153
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.nvidia-legacy.options]
11331154
BinaryName = "/usr/bin/nvidia-container-runtime.legacy"
11341155
SystemdCgroup = true
1156+
1157+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
1158+
runtime_type = "io.containerd.runc.v2"
1159+
1160+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
1161+
BinaryName = "/usr/bin/runc"
1162+
SystemdCgroup = true
1163+
1164+
[plugins."io.containerd.grpc.v1.cri".registry]
1165+
1166+
[plugins."io.containerd.grpc.v1.cri".registry.mirrors]
1167+
1168+
[plugins."io.containerd.grpc.v1.cri".registry.mirrors."docker.io"]
1169+
endpoint = ["https://registry-1.docker.io"]
11351170
`
11361171
require.Equal(t, expectedDropIn, string(actualDropIn))
11371172
return nil
@@ -1278,6 +1313,12 @@ version = 2
12781313
12791314
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.nvidia-legacy.options]
12801315
BinaryName = "/usr/bin/nvidia-container-runtime.legacy"
1316+
1317+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
1318+
runtime_type = "io.containerd.runc.v2"
1319+
1320+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
1321+
BinaryName = "/usr/bin/runc"
12811322
`
12821323

12831324
require.Equal(t, expectedDropIn, string(actualDropIn))
@@ -1407,6 +1448,12 @@ version = 2
14071448
14081449
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.nvidia-legacy.options]
14091450
BinaryName = "/usr/bin/nvidia-container-runtime.legacy"
1451+
1452+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
1453+
runtime_type = "io.containerd.runc.v2"
1454+
1455+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
1456+
BinaryName = "/usr/bin/runc"
14101457
`
14111458
require.Equal(t, expectedDropIn, string(actualDropIn))
14121459
return nil
@@ -1533,6 +1580,12 @@ version = 3
15331580
15341581
[plugins."io.containerd.cri.v1.runtime".containerd.runtimes.nvidia-legacy.options]
15351582
BinaryName = "/usr/bin/nvidia-container-runtime.legacy"
1583+
1584+
[plugins."io.containerd.cri.v1.runtime".containerd.runtimes.runc]
1585+
runtime_type = "io.containerd.runc.v2"
1586+
1587+
[plugins."io.containerd.cri.v1.runtime".containerd.runtimes.runc.options]
1588+
BinaryName = "/usr/bin/runc"
15361589
`
15371590
require.Equal(t, expectedDropIn, string(actualDropIn))
15381591

@@ -1678,6 +1731,15 @@ version = 3
16781731
NoPivotRoot = false
16791732
Root = "/run/containerd/runc"
16801733
SystemdCgroup = true
1734+
1735+
[plugins."io.containerd.cri.v1.runtime".containerd.runtimes.runc]
1736+
runtime_type = "io.containerd.runc.v2"
1737+
1738+
[plugins."io.containerd.cri.v1.runtime".containerd.runtimes.runc.options]
1739+
BinaryName = "/usr/bin/runc"
1740+
NoPivotRoot = false
1741+
Root = "/run/containerd/runc"
1742+
SystemdCgroup = true
16811743
`
16821744
require.Equal(t, expectedDropIn, string(actualDropIn))
16831745

‎pkg/config/engine/containerd/config_test.go‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,15 @@ func TestAddRuntime(t *testing.T) {
9494
[plugins]
9595
[plugins."io.containerd.grpc.v1.cri"]
9696
[plugins."io.containerd.grpc.v1.cri".containerd]
97-
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes]
97+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes]
98+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
99+
privileged_without_host_devices = true
100+
runtime_engine = "engine"
101+
runtime_root = "root"
102+
runtime_type = "type"
103+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
104+
BinaryName = "/usr/bin/runc"
105+
SystemdCgroup = true
98106
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test]
99107
privileged_without_host_devices = true
100108
runtime_engine = "engine"
@@ -128,7 +136,16 @@ func TestAddRuntime(t *testing.T) {
128136
[plugins]
129137
[plugins."io.containerd.grpc.v1.cri"]
130138
[plugins."io.containerd.grpc.v1.cri".containerd]
139+
default_runtime_name = "default"
131140
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes]
141+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.default]
142+
privileged_without_host_devices = true
143+
runtime_engine = "engine"
144+
runtime_root = "root"
145+
runtime_type = "type"
146+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.default.options]
147+
BinaryName = "/usr/bin/default"
148+
SystemdCgroup = true
132149
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test]
133150
privileged_without_host_devices = true
134151
runtime_engine = "engine"
@@ -170,7 +187,24 @@ func TestAddRuntime(t *testing.T) {
170187
[plugins]
171188
[plugins."io.containerd.grpc.v1.cri"]
172189
[plugins."io.containerd.grpc.v1.cri".containerd]
190+
default_runtime_name = "default"
173191
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes]
192+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
193+
privileged_without_host_devices = true
194+
runtime_engine = "engine"
195+
runtime_root = "root"
196+
runtime_type = "type"
197+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
198+
BinaryName = "/usr/bin/runc"
199+
SystemdCgroup = true
200+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.default]
201+
privileged_without_host_devices = false
202+
runtime_engine = "defaultengine"
203+
runtime_root = "defaultroot"
204+
runtime_type = "defaulttype"
205+
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.default.options]
206+
BinaryName = "/usr/bin/default"
207+
SystemdCgroup = false
174208
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test]
175209
privileged_without_host_devices = false
176210
runtime_engine = "defaultengine"
@@ -225,6 +259,14 @@ func TestAddRuntime(t *testing.T) {
225259
[plugins."io.containerd.cri.v1.runtime"]
226260
[plugins."io.containerd.cri.v1.runtime".containerd]
227261
[plugins."io.containerd.cri.v1.runtime".containerd.runtimes]
262+
[plugins."io.containerd.cri.v1.runtime".containerd.runtimes.runc]
263+
privileged_without_host_devices = true
264+
runtime_engine = "engine"
265+
runtime_root = "root"
266+
runtime_type = "type"
267+
[plugins."io.containerd.cri.v1.runtime".containerd.runtimes.runc.options]
268+
BinaryName = "/usr/bin/runc"
269+
SystemdCgroup = true
228270
[plugins."io.containerd.cri.v1.runtime".containerd.runtimes.test]
229271
privileged_without_host_devices = true
230272
runtime_engine = "engine"

‎pkg/config/engine/containerd/containerd.go‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,15 @@ func New(opts ...Option) (engine.Interface, error) {
133133
}
134134
dropInConfig := &engine.Config{
135135
Source: sourceConfig,
136-
// The destinationConfig is an empty config with the same options
137-
// as the source config.
136+
// The destinationConfig is a minimal config with the same options
137+
// as the source config. The starting content of the destinationConfig
138+
// is the entire plugins."io.containerd.grpc.v1.cri" section of the source
139+
// config. This is needed due to how containerd merges configuration from
140+
// multiple files. In particular, plugins are overridden by key and the
141+
// content is not merged. This issue is fixed starting in containerd 2.1
142+
// Reference: https://github.com/containerd/containerd/issues/5837
138143
Destination: &Config{
139-
Tree: toml.NewEmpty(),
144+
Tree: getBaseDropInConfigTree(sourceConfig),
140145
configOptions: sourceConfigOptions,
141146
},
142147
}
@@ -204,3 +209,17 @@ func chrootIfRequired(hostRoot string, commandLine ...string) []string {
204209

205210
return append([]string{"chroot", hostRoot}, commandLine...)
206211
}
212+
213+
// getBaseDropInConfigTree returns the base config to use for the drop-in config file.
214+
// For older containerd versions (e.g. 1.7) the plugins section of multiple
215+
// configs are not fully merged, but merged by key instead. This means that we
216+
// need to duplicate the entire plugin-specific config in the drop in file so as
217+
// to not override settings applied in the base config.
218+
func getBaseDropInConfigTree(sourceConfig *Config) *toml.Tree {
219+
baseDropInConfigTree := toml.NewEmpty()
220+
criPlugin := sourceConfig.GetSubtreeByPath([]string{"plugins", sourceConfig.CRIRuntimePluginName})
221+
if criPlugin != nil {
222+
baseDropInConfigTree.SetPath([]string{"plugins", sourceConfig.CRIRuntimePluginName}, criPlugin)
223+
}
224+
return baseDropInConfigTree
225+
}

0 commit comments

Comments
 (0)