Support soft reboot - #151
Conversation
Sorry, something went wrong.
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>
Sorry, something went wrong.
| 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
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, Thank you!
Sorry, something went wrong.
|
|
||
| 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
Sorry, something went wrong.
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.
Sorry, something went wrong.
There was a problem hiding this comment.
cc @alicefr should we support soft-reboot=required?
Sorry, something went wrong.
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.
Sorry, something went wrong.
| 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
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, Thank you!
Sorry, something went wrong.
| 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
Sorry, something went wrong.
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?
Sorry, something went wrong.
There was a problem hiding this comment.
it combines download + stage + reboot in one step in that case
Sorry, something went wrong.
Sorry, something went wrong.
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>
Reviewers
At least 1 approving review is required to merge this pull request.
More info below:
What I confirmed?
Also
Also
bn.spec.rebootPolicy: AllowSoftReboot was set on the BootcNode by the controller
Full reboot verification(different)
Aug 25 12:36:54 worker2 systemd[1]: Reload requested from client PID 1044 ('systemctl') (unit cloud-init-main.service)... Aug 25 12:37:03 worker2 systemd[1]: Reload requested from client PID 1239 ('systemctl') (unit session-4.scope)... Aug 25 12:38:11 worker2 systemd[1]: Shutting down. Aug 25 12:38:20 worker2 kernel: ISO 9660 Extensions: Microsoft Joliet Level 3Also
Closes #117