Support soft reboot - #151
Conversation
The daemon needs to know the pool's reboot policy to decide between a full reboot and a soft reboot. Add a RebootPolicy field to BootcNodeSpec, reusing the existing type from bootcnodepool_types.go. Assisted-by: AI Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Add Apply(ctx, softReboot) to the Executor interface. When softReboot is true, it runs bootc upgrade --from-downloaded --apply --soft-reboot=auto via nsenter, which performs a userspace-only restart when the kernel hasn't changed. Uses bootc upgrade (not switch) because current bink images predate bootc#2342 which adds --from-downloaded to switch. Assisted-by: AI Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Copy the pool's disruption.rebootPolicy to each BootcNode's spec on creation and on sync, following the same pattern as pullSecretRef propagation. Defaults to RebootOnly when the pool has no disruption spec. Assisted-by: AI Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
When the BootcNode's rebootPolicy is AllowSoftReboot, use Executor.Apply(ctx, true) instead of Executor.Reboot(ctx). This calls bootc with --soft-reboot=auto, which performs a userspace-only restart when the kernel hasn't changed, avoiding a full hardware reboot. Assisted-by: AI Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Verify the full soft reboot lifecycle: create a pool with AllowSoftReboot, trigger an update, and confirm the node comes back with the same boot ID (kernel stayed up, only userspace restarted). Assisted-by: AI Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
| Status(ctx context.Context) ([]byte, error) | ||
| Stage(ctx context.Context, image string) error | ||
| Reboot(ctx context.Context) error | ||
| Apply(ctx context.Context, softReboot bool) error |
There was a problem hiding this comment.
Should be called ApplyUpdate, just Apply is a bit ambiguous
There was a problem hiding this comment.
Yes, Thank you!
|
|
||
| args := []string{"bootc", "upgrade", "--from-downloaded", "--apply"} | ||
| if softReboot { | ||
| args = append(args, "--soft-reboot=auto") |
There was a problem hiding this comment.
We should probably have soft-reboot=required if it's true? I don't think this is incorrect as this would just perform a normal reboot is soft reboot isn't available
There was a problem hiding this comment.
From the bootc source (cli.rs):
- Required — "Require a soft reboot; fail if not possible"
- Auto — "Automatically use soft reboot if possible, otherwise use regular reboot"
If we want a RequireSoftReboot policy in the future that fails instead of falling back, we would add a new RebootPolicy enum value and use --soft-reboot=required for that.
There was a problem hiding this comment.
cc @alicefr should we support soft-reboot=required?
There was a problem hiding this comment.
Can bootc identify whether a soft-reboot is possible after staging an update? Could the controller potentially identify this condition before signalling to the daemon that it should stage the image? The reason I ask is because we should surface the error message via an API object whenever a soft-reboot is not possible and --soft-reboot=required is set. And ideally, it would be great to uncover this before staging the OS image.
There was a problem hiding this comment.
There was a problem hiding this comment.
Could the controller potentially identify this condition before signalling to the daemon that it should stage the image?
Generally no. Bootc needs the staged deployment to compare it with the running system. It cannot reliably know before downloading and preparing the target image.
There was a problem hiding this comment.
Yes it will be great, bootc determines this after staging and exposes it as status.staged.softRebootCapable, which the daemon already propagates to BootcNode.status. RequireSoftReboot policy, MCO or the bootc-operator controller could use that status as a gate before selecting and draining the node, surface SoftRebootUnavailable through an API condition, and still use --soft-reboot=required as final enforcement.
There was a problem hiding this comment.
Generally no. Bootc needs the staged deployment to compare it with the running system. It cannot reliably know before downloading and preparing the target image.
usually for upgrades, and upgrades only, I think we compute at image build time whether this particular image upgrade would be soft rebootable. It's just a matter of comparing the current and the upgrade image being built. That being said, I don't think there's a good requirement for this
| func (e *HostExecutor) Apply(ctx context.Context, softReboot bool) error { | ||
| log := logf.FromContext(ctx) | ||
|
|
||
| args := []string{"bootc", "upgrade", "--from-downloaded", "--apply"} |
There was a problem hiding this comment.
I really think we should have a centralized function/constants that have all the list of bootc commands and we just call that function to get the command we need. Not a huge fan of the current scattered approach we have
There was a problem hiding this comment.
Yes, Thank you!
| return ctrl.Result{}, fmt.Errorf("reboot: %w", err) | ||
| if res.softReboot { | ||
| log.Info("Applying update with soft reboot") | ||
| if err := r.Executor.Apply(ctx, true); err != nil { |
There was a problem hiding this comment.
Does this imply that soft reboot can only be performed with --download-only option? If yes, then that's not true for boot.c bootc update/switch both support --soft-reboot option
There was a problem hiding this comment.
No, it does not require --download-only. The --from-downloaded flag here just tells bootc to use the already-staged deployment without fetching from the registry again.
But it is not supported by operator It bypasses the drain. It does a full reboot in that case. cc @alicefr Can you please confirm this?
There was a problem hiding this comment.
it combines download + stage + reboot in one step in that case
Rename Apply to ApplyUpdate for clarity per review feedback. Extract bootc command construction into builder functions (bootcStatusArgs, bootcSwitchArgs, bootcApplyUpdateArgs, systemctlRebootArgs) so all bootc invocations are defined in one place. Assisted-by: AI Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
3 participants
More info below:
What I confirmed?
Also
Also
bn.spec.rebootPolicy: AllowSoftReboot was set on the BootcNode by the controller
Full reboot verification(different)
Also
Closes #117