-
Notifications
You must be signed in to change notification settings - Fork 435
[nvidia-ctk-installer] introduce unconfigure mode to determine cleanu… #1362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…p behavior Signed-off-by: Christopher Desiniotis <[email protected]>
|
|
||
| // Unconfigure removes the options from the specified config | ||
| func (o Options) Unconfigure(cfg engine.Interface) error { | ||
| err := o.RevertConfig(cfg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about checking the mode here instead of adding another top-level function to the Options. This should mean that we don't need to make changes for each runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that should be possible...
| *c.Tree = config | ||
| } | ||
|
|
||
| func (c *Config) UnsetDefaultRuntime(name string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we invoke this from RemoveRuntime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was a copy-paste from the code currently in RemoveRuntime. I can look to reuse this new function there.
| } | ||
|
|
||
| func (c *ConfigWithDropIn) UnsetDefaultRuntime(name string) { | ||
| c.Interface.UnsetDefaultRuntime(name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this unsets it in the drop-in file and not in the top-level config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was intentional. When using drop-ins, we set the default_runtime_name field only in the drop-in file. The purpose of UnsetDefaultRuntime is to revert that setting (in cases where nvidia is set as the default runtime).
| } | ||
| if runtimePath != "" && defaultRuntimePath != "" && runtimePath == defaultRuntimePath { | ||
| config.DeletePath([]string{"plugins", "cri", "containerd", "default_runtime"}) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other implementations we have *c.Tree = config at the end of modifying functions. Do we not need those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably missed this...
|
Closing in favor of #1360 |
…p behavior