Add bootconfig to Activate. This allows the caller to provide #218
Conversation
a new bootconfig to the device during activate. This allows for the device to be provided a minimum configuration to gaurentee compatibility with the new OS or provide OS version specific configuration to allow the new OS to boot.
Pull Request Test Coverage Report for Build 11441760490Details
💛 - Coveralls |
| // Bootconfig provides the next boot configuration to load after OS upgrade. | ||
| // The device will take all artifacts provided and replace them just as if | ||
| // a gnoi.BootConfig.Set was performed on the device. | ||
| // The main use case is to facilitate the new OS to utilized a new | ||
| // configuration syntax which was not supported in previous versions. | ||
| gnoi.bootconfig.SetBootConfigRequest boot_config = 4; |
There was a problem hiding this comment.
While reviewing this, I had to reason through when a bootconfig change is supposed to take effect. If a client invokes gNOI.BootConfig.SetBootConfig, when is the boot_config loaded or activated? Is it
- immediate?
- on reboot?
- loaded immediately, but application of "configuration items" are implementation dependent? (ie: some items are activated immediately and others may require a reboot).
It took me about 20 minutes to reason through this (I am slow). I suspect it's option 3? I didn't find a strict definition. Probably this should be added.
Unless it's option 2, I am not sure this is really needed? Can't the boot config just be updated after the new OS is installed?
gNOI.OS.Install
gNOI.OS.Activate
(wait for reboot to complete)
gNOI.BootConfig.SetBootConfig
I guess maybe it's an optimization to prevent needing a second reboot if there are configuration items being changed which only take effect on a reboot?
There was a problem hiding this comment.
on reboot "next boot configuration to load after OS upgrade."
the example you gave fails for devices which when doing the actiate will try to load the old configuration to validate it still works with the new os and fails...
basically the example you gave is the exact reason this is required
There was a problem hiding this comment.
LGTM.
Did you purposely use SetBootConfigRequest, instead of Bootz.BootConfig, to cover possible g* API breaking changes?
a new bootconfig to the device during activate.
This allows for the device to be provided a minimum configuration to gaurentee compatibility with the new OS or provide OS version specific configuration to allow the new OS to boot.