Re-order boot order to avoid booting into disk during maintenance reboots #488
Conversation
nagadeesh-nagaraja
requested review from
Nuckal777,
afritzler,
defo89 and
stefanhipfel
nagadeesh-nagaraja
changed the title
github-actions
bot
added
the
documentation
hardikdr
left a comment
hardikdr
left a comment
There was a problem hiding this comment.
Had a first quick iteration, have left few comments inline.
Thanks for the PR @nagadeesh-nagaraja
nagadeesh-nagaraja
force-pushed
the
RebootSkipDisk
branch
from
8d597f7 to
4f7a461
Compare
nagadeesh-nagaraja
force-pushed
the
RebootSkipDisk
branch
from
4f7a461 to
2e83a09
Compare
| return ctrl.Result{RequeueAfter: r.ResyncInterval}, nil | ||
| } | ||
| // if no config found or if it has changed, we need to create or path it | ||
| config, err = r.applyServerBootConfiguration(ctx, log, serverMaintenance, server) |
There was a problem hiding this comment.
Does this PR cover cover the use case to change bios settings, which don't need a reboot? In that case the boot config dance isn't strictly needed.
There was a problem hiding this comment.
@Nuckal777 workflow is as follows:
For operations which needs reboots:
The creator of serverMaintenance needs to provide the bootConfig Spec. if provided, it will be handled
For Operations which does not need server reboots: (however, as of now, almost all maintenance needs it)
The creator of serverMaintenance will not provide the bootConfig Spec. if skipped, the server bootConfig will not be created/changed.
| // Note: this is to check skip of BootConfiguration | ||
| // implication of skipping is that the server will still continue to run the OS during Maintenance | ||
| // might be subjected to reboots | ||
| val, found := serverMaintenance.GetAnnotations()[metalv1alpha1.OperationSkipBootConfiguration] |
There was a problem hiding this comment.
The annotation feels a bit redundant to .spec.serverBootConfigurationTemplate: null on the ServerMaintenance. Am I missing something?
There was a problem hiding this comment.
from what I have seen until now. All of the operations related to maintenance needs server reboot. Idea is to force servermaintenance creator to provide the bootCondifguration spec.
if this is not needed for some extra ordinary conditions, we can skip providing the spec using annotation.
api/v1alpha1/constants.go
Outdated
| OperationSkipBootConfiguration = "metal.ironcore.dev/operation-skip-boot-configuration" | ||
| // SkipBootConfiguration skips the boot configuration if set to this value. | ||
| SkipBootConfiguration = "skip" |
There was a problem hiding this comment.
Would it make sense to simply delete an unused bootconfig?
There was a problem hiding this comment.
I dont understand this
|
I think this PR will be made obsolete by ironcore-dev/boot-operator#232 and the fact that we will use |
I dont think this is true. |
|
parking it as we work on a centralized solution to change the boot order of a server |
Fixes #472 #469