Skip to content

Commit dda4b93

Browse files
cdesiniotiscdesiniotis
authored
Merge pull request #1360 from cdesiniotis/do-not-cleanup-crio-drop-in
[nvidia-ctk-installer] do not revert cri-o config on shutdown
2 parents 4fd94d7 + 9fab81c commit dda4b93

File tree

11 files changed

+285
-18
lines changed

11 files changed

+285
-18
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (o Options) Configure(cfg engine.Interface) error {
6565
if err != nil {
6666
return fmt.Errorf("unable to update config: %v", err)
6767
}
68-
return o.flush(cfg)
68+
return o.Flush(cfg)
6969
}
7070

7171
// Unconfigure removes the options from the specified config
@@ -75,7 +75,7 @@ func (o Options) Unconfigure(cfg engine.Interface) error {
7575
return fmt.Errorf("unable to update config: %v", err)
7676
}
7777

78-
if err := o.flush(cfg); err != nil {
78+
if err := o.Flush(cfg); err != nil {
7979
return err
8080
}
8181

@@ -93,8 +93,8 @@ func (o Options) Unconfigure(cfg engine.Interface) error {
9393
return nil
9494
}
9595

96-
// flush flushes the specified config to disk
97-
func (o Options) flush(cfg engine.Interface) error {
96+
// Flush flushes the specified config to disk
97+
func (o Options) Flush(cfg engine.Interface) error {
9898
filepath := o.DropInConfig
9999
if filepath == "" {
100100
filepath = o.TopLevelConfigPath

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

Lines changed: 112 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,30 @@ func TestCrioConfigLifecycle(t *testing.T) {
8888
},
8989
assertCleanupPostConditions: func(t *testing.T, co *container.Options, _ *Options) error {
9090
require.NoFileExists(t, co.TopLevelConfigPath)
91-
require.NoFileExists(t, co.DropInConfig)
91+
// drop-in file not removed on cleanup
92+
actual, err := os.ReadFile(co.DropInConfig)
93+
require.NoError(t, err)
94+
95+
expected := `
96+
[crio]
97+
98+
[crio.runtime]
99+
100+
[crio.runtime.runtimes]
101+
102+
[crio.runtime.runtimes.nvidia]
103+
runtime_path = "/usr/bin/nvidia-container-runtime"
104+
runtime_type = "oci"
105+
106+
[crio.runtime.runtimes.nvidia-cdi]
107+
runtime_path = "/usr/bin/nvidia-container-runtime.cdi"
108+
runtime_type = "oci"
109+
110+
[crio.runtime.runtimes.nvidia-legacy]
111+
runtime_path = "/usr/bin/nvidia-container-runtime.legacy"
112+
runtime_type = "oci"
113+
`
114+
require.Equal(t, expected, string(actual))
92115
return nil
93116
},
94117
},
@@ -182,8 +205,6 @@ signature_policy = "/etc/crio/policy.json"
182205
assertCleanupPostConditions: func(t *testing.T, co *container.Options, o *Options) error {
183206
require.FileExists(t, co.TopLevelConfigPath)
184207

185-
require.NoFileExists(t, co.DropInConfig)
186-
187208
actualTopLevel, err := os.ReadFile(co.TopLevelConfigPath)
188209
require.NoError(t, err)
189210

@@ -203,6 +224,37 @@ signature_policy = "/etc/crio/policy.json"
203224
`
204225
require.Equal(t, expectedTopLevel, string(actualTopLevel))
205226

227+
// drop-in file not removed on cleanup
228+
require.FileExists(t, co.DropInConfig)
229+
actual, err := os.ReadFile(co.DropInConfig)
230+
require.NoError(t, err)
231+
232+
expected := `
233+
[crio]
234+
235+
[crio.runtime]
236+
237+
[crio.runtime.runtimes]
238+
239+
[crio.runtime.runtimes.nvidia]
240+
monitor_path = "/usr/libexec/crio/conmon"
241+
runtime_path = "/usr/bin/nvidia-container-runtime"
242+
runtime_root = "/run/crun"
243+
runtime_type = "oci"
244+
245+
[crio.runtime.runtimes.nvidia-cdi]
246+
monitor_path = "/usr/libexec/crio/conmon"
247+
runtime_path = "/usr/bin/nvidia-container-runtime.cdi"
248+
runtime_root = "/run/crun"
249+
runtime_type = "oci"
250+
251+
[crio.runtime.runtimes.nvidia-legacy]
252+
monitor_path = "/usr/libexec/crio/conmon"
253+
runtime_path = "/usr/bin/nvidia-container-runtime.legacy"
254+
runtime_root = "/run/crun"
255+
runtime_type = "oci"
256+
`
257+
require.Equal(t, expected, string(actual))
206258
return nil
207259
},
208260
},
@@ -312,8 +364,33 @@ runtime_type = "oci"
312364

313365
require.Equal(t, expectedTopLevel, string(actualTopLevel))
314366

315-
require.NoFileExists(t, co.DropInConfig)
367+
// drop-in file not removed on cleanup
368+
// default_runtime setting removed from drop-in
369+
require.FileExists(t, co.DropInConfig)
370+
actual, err := os.ReadFile(co.DropInConfig)
371+
require.NoError(t, err)
372+
373+
expected := `
374+
[crio]
375+
376+
[crio.runtime]
377+
378+
[crio.runtime.runtimes]
379+
380+
[crio.runtime.runtimes.nvidia]
381+
runtime_path = "/usr/bin/nvidia-container-runtime"
382+
runtime_type = "oci"
316383
384+
[crio.runtime.runtimes.nvidia-cdi]
385+
runtime_path = "/usr/bin/nvidia-container-runtime.cdi"
386+
runtime_type = "oci"
387+
388+
[crio.runtime.runtimes.nvidia-legacy]
389+
runtime_path = "/usr/bin/nvidia-container-runtime.legacy"
390+
runtime_type = "oci"
391+
`
392+
393+
require.Equal(t, expected, string(actual))
317394
return nil
318395
},
319396
},
@@ -480,7 +557,37 @@ plugin_dirs = [
480557
`
481558
require.Equal(t, expected, string(actual))
482559

483-
require.NoFileExists(t, co.DropInConfig)
560+
// drop-in file not removed on cleanup
561+
require.FileExists(t, co.DropInConfig)
562+
actualDropIn, err := os.ReadFile(co.DropInConfig)
563+
require.NoError(t, err)
564+
565+
expectedDropIn := `
566+
[crio]
567+
568+
[crio.runtime]
569+
570+
[crio.runtime.runtimes]
571+
572+
[crio.runtime.runtimes.nvidia]
573+
monitor_path = "/usr/libexec/crio/conmon"
574+
runtime_path = "/usr/bin/nvidia-container-runtime"
575+
runtime_root = "/run/crun"
576+
runtime_type = "oci"
577+
578+
[crio.runtime.runtimes.nvidia-cdi]
579+
monitor_path = "/usr/libexec/crio/conmon"
580+
runtime_path = "/usr/bin/nvidia-container-runtime.cdi"
581+
runtime_root = "/run/crun"
582+
runtime_type = "oci"
583+
584+
[crio.runtime.runtimes.nvidia-legacy]
585+
monitor_path = "/usr/libexec/crio/conmon"
586+
runtime_path = "/usr/bin/nvidia-container-runtime.legacy"
587+
runtime_root = "/run/crun"
588+
runtime_type = "oci"
589+
`
590+
require.Equal(t, expectedDropIn, string(actualDropIn))
484591

485592
return nil
486593
},

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

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func setupHook(o *container.Options, co *Options) error {
133133
func setupConfig(o *container.Options) error {
134134
log.Infof("Updating config file")
135135

136-
cfg, err := getRuntimeConfig(o)
136+
cfg, err := getRuntimeConfig(o, false)
137137
if err != nil {
138138
return fmt.Errorf("unable to load config: %v", err)
139139
}
@@ -180,16 +180,24 @@ func cleanupHook(co *Options) error {
180180

181181
// cleanupConfig removes the NVIDIA container runtime from the cri-o config
182182
func cleanupConfig(o *container.Options) error {
183+
if !o.SetAsDefault {
184+
return nil
185+
}
186+
183187
log.Infof("Reverting config file modifications")
184188

185-
cfg, err := getRuntimeConfig(o)
189+
cfg, err := getRuntimeConfig(o, true)
186190
if err != nil {
187191
return fmt.Errorf("unable to load config: %v", err)
188192
}
189193

190-
err = o.Unconfigure(cfg)
194+
err = cfg.UpdateDefaultRuntime(o.RuntimeName, engine.UpdateActionUnset)
191195
if err != nil {
192-
return fmt.Errorf("unable to unconfigure cri-o: %v", err)
196+
return fmt.Errorf("failed to unset %q as the default runtime: %w", o.RuntimeName, err)
197+
}
198+
199+
if err := o.Flush(cfg); err != nil {
200+
return err
193201
}
194202

195203
err = RestartCrio(o)
@@ -206,24 +214,35 @@ func RestartCrio(o *container.Options) error {
206214
}
207215

208216
func GetLowlevelRuntimePaths(o *container.Options) ([]string, error) {
209-
cfg, err := getRuntimeConfig(o)
217+
cfg, err := getRuntimeConfig(o, false)
210218
if err != nil {
211219
return nil, fmt.Errorf("unable to load crio config: %w", err)
212220
}
213221
return engine.GetBinaryPathsForRuntimes(cfg), nil
214222
}
215223

216-
func getRuntimeConfig(o *container.Options) (engine.Interface, error) {
224+
func getRuntimeConfig(o *container.Options, loadDestinationConfig bool) (engine.Interface, error) {
217225
loaders, err := o.GetConfigLoaders(crio.CommandLineSource)
218226
if err != nil {
219227
return nil, err
220228
}
221-
return crio.New(
229+
230+
options := []crio.Option{
222231
crio.WithTopLevelConfigPath(o.TopLevelConfigPath),
223232
crio.WithConfigSource(
224233
toml.LoadFirst(
225234
loaders...,
226235
),
227236
),
228-
)
237+
}
238+
239+
if loadDestinationConfig {
240+
destinationConfigPath := o.TopLevelConfigPath
241+
if o.DropInConfig != "" {
242+
destinationConfigPath = o.DropInConfig
243+
}
244+
options = append(options, crio.WithConfigDestination(toml.FromFile(destinationConfigPath)))
245+
}
246+
247+
return crio.New(options...)
229248
}

‎pkg/config/engine/api.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ const (
2020
// SaveToSTDOUT is used to write the specified config to stdout instead of
2121
// to a file on disk.
2222
SaveToSTDOUT = ""
23+
// UpdateActionSet is used as an argument to UpdateDefaultRuntime
24+
// when setting a runtime handler as the default in the config
25+
UpdateActionSet = "set"
26+
// UpdateActionUnset is used as an argument to UpdateDefaultRuntime
27+
// when unsetting a runtime handler as the default in the config
28+
UpdateActionUnset = "unset"
2329
)
2430

2531
// Interface defines the API for a runtime config updater.
@@ -29,6 +35,7 @@ type Interface interface {
2935
EnableCDI()
3036
GetRuntimeConfig(string) (RuntimeConfig, error)
3137
RemoveRuntime(string) error
38+
UpdateDefaultRuntime(string, string) error
3239
Save(string) (int64, error)
3340
String() string
3441
}

‎pkg/config/engine/config.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ type RuntimeConfigDestination interface {
4343
AddRuntimeWithOptions(string, string, bool, interface{}) error
4444
EnableCDI()
4545
RemoveRuntime(string) error
46+
UpdateDefaultRuntime(string, string) error
4647
Save(string) (int64, error)
4748
String() string
4849
}
@@ -60,6 +61,14 @@ func (c *Config) RemoveRuntime(runtime string) error {
6061
return c.Destination.RemoveRuntime(runtime)
6162
}
6263

64+
// UpdateDefaultRuntime updates the default runtime setting in the destination config.
65+
// When action is 'set' the provided runtime name is set as the default.
66+
// When action is 'unset' we make sure the provided runtime name is not
67+
// the default.
68+
func (c *Config) UpdateDefaultRuntime(runtime string, action string) error {
69+
return c.Destination.UpdateDefaultRuntime(runtime, action)
70+
}
71+
6372
// EnableCDI enables CDI in the destination config.
6473
func (c *Config) EnableCDI() {
6574
c.Destination.EnableCDI()

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,3 +154,33 @@ func (c *Config) RemoveRuntime(name string) error {
154154
*c.Tree = config
155155
return nil
156156
}
157+
158+
// UpdateDefaultRuntime updates the default runtime setting in the config.
159+
// When action is 'set' the provided runtime name is set as the default.
160+
// When action is 'unset' we make sure the provided runtime name is not
161+
// the default.
162+
func (c *Config) UpdateDefaultRuntime(name string, action string) error {
163+
if action != engine.UpdateActionSet && action != engine.UpdateActionUnset {
164+
return fmt.Errorf("invalid action %q, valid actions are %q and %q", action, engine.UpdateActionSet, engine.UpdateActionUnset)
165+
}
166+
167+
if c == nil || c.Tree == nil {
168+
if action == engine.UpdateActionSet {
169+
return fmt.Errorf("config toml is nil")
170+
}
171+
return nil
172+
}
173+
174+
config := *c.Tree
175+
if action == engine.UpdateActionSet {
176+
config.SetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "default_runtime_name"}, name)
177+
} else {
178+
defaultRuntime, ok := config.GetPath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "default_runtime_name"}).(string)
179+
if ok && defaultRuntime == name {
180+
config.DeletePath([]string{"plugins", c.CRIRuntimePluginName, "containerd", "default_runtime_name"})
181+
}
182+
}
183+
184+
*c.Tree = config
185+
return nil
186+
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,14 @@ func (c *ConfigWithDropIn) RemoveRuntime(name string) error {
9696
return c.Interface.RemoveRuntime(name)
9797
}
9898

99+
// UpdateDefaultRuntime updates the default runtime setting in the drop-in config.
100+
// When action is 'set' the provided runtime name is set as the default.
101+
// When action is 'unset' we make sure the provided runtime name is not
102+
// the default.
103+
func (c *ConfigWithDropIn) UpdateDefaultRuntime(name string, action string) error {
104+
return c.Interface.UpdateDefaultRuntime(name, action)
105+
}
106+
99107
// flush saves the top-level config to its path.
100108
// If the config is empty, the file will be deleted.
101109
func (c *topLevelConfig) Save(dropInPath string) (int64, error) {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,10 @@ func (c *ConfigV1) RemoveRuntime(name string) error {
120120
return nil
121121
}
122122

123+
func (c *ConfigV1) UpdateDefaultRuntime(name string, action string) error {
124+
return fmt.Errorf("this method is not implemented")
125+
}
126+
123127
// Save writes the config to a file
124128
func (c ConfigV1) Save(path string) (int64, error) {
125129
return (Config)(c).Save(path)

0 commit comments

Comments
 (0)