NAS-131326 / 25.04 / Simplify HA upgrade logic + NAS-131325: system.reboot.info and failover.reboot.info methods and events
#10744
Conversation
bugclerk
changed the title
system.reboot.info and failover.reboot.info methods and eventssystem.reboot.info and failover.reboot.info methods and events
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #10744 +/- ##
==========================================
+ Coverage 81.80% 81.94% +0.14%
==========================================
Files 1626 1627 +1
Lines 57324 57296 -28
Branches 5920 5923 +3
==========================================
+ Hits 46892 46954 +62
+ Misses 10432 10342 -90 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Code looks OK. I am getting the notification but a few points should be addressed
- I expected the dialog to be centered and follow the styles of webui's typical dialogs. However, it was stuck to the top right corner and looks weird.
- I expected there to be a "Reboot" button on the dialog itself that I can click and reboot right away. There's no action buttons on the notification dialog.
- The title can say "Reboot required" instead of "Reboot Info"
| @@ -39,8 +39,19 @@ | |||
|
|
|||
| <ix-truecommand-button></ix-truecommand-button> | |||
|
|
|||
| @if (hasRebootRequiredReasons) { | |||
There was a problem hiding this comment.
We can make hasRebootRequiredReasons a signal.
| export const rebootInfoLoaded = createAction( | ||
| '[Reboot Info API] Reboot Info Loaded', | ||
| props<{ | ||
| thisNodeInfo: SystemRebootInfo | null; |
There was a problem hiding this comment.
These keys should have reboot keyword in there somewhere. They are not node-info. They are specifically node-reboot-info. So, just calling them thisNodeInfo and otherNodeInfo can cause confusion in the future.
| (state) => state, | ||
| ); | ||
|
|
||
| export const selectThisNodeInfo = createSelector( |
There was a problem hiding this comment.
Here as well. Let's add reboot keyword in the variable name so it's clear it's only the node specific reboot info.
|
@RehanY147 middleware issue has been resolved, now the dialog is displayed |
There was a problem hiding this comment.
I merged with master because I was getting issues with the 2fa calls. Once merged, I can add a reason and the dialog shows up. But once I refresh the page, I see the following error in the browser console and the reboot dialog doesn't show up. I am not sure if this is because of some other merged commit or if it is because of this PR. It looks like it is a related problem. This is an HA system.
|
@RehanY147 Now everything works as expected! |
|
This PR has been merged and conversations have been locked. |
Changes:
This PR contains changes for 2 tickets
NAS-131325:
system.reboot.infoandfailover.reboot.infomethods and eventshere's what's implemented for this ticket:
system.reboot.infoandfailover.reboot.infobased on whether system is HA licensed,(if there are items in the
reboot_required_reasonsfield, the following dialog should appear)NAS-131326: Simplify HA upgrade logic
here's what's implemented for this ticket:
failover.upgrade_pendingandfailover.upgrade_finishmethods and events were removed, they are not necessary anymorefailover.upgrademethod and waits for middleware to upgrade both local and remote controllers, then reboots the remote controller and wait it to come online, then webUI will display a reboot reason (see pt "1" in the description of this PR)failover.become_passivewhen the user is ready to reboot the local controller, and thus the upgrade will be finished.Testing:
NAS-131325:
system.reboot.infoandfailover.reboot.infomethods and eventsTo test the
system.reboot.infoandfailover.reboot.infomethods and events, follow these steps:Login into the HA-enabled server, for example at M40 (10.220.16.82)
Expected result: When logging into the HA system, a call to
failover.reboot.infomust be made, and then a subscription tofailover.reboot.info. For non-HA - the same thing, onlysystem.reboot.info.If the call response or subscription contains items in
reboot_required_reasons, a dialog is shown with a proposal to restart the corresponding node.However, if the
failover.disabled.reasonsresponse contains reasons other thanMISMATCH_VERSIONS,LOC_FIPS_REBOOT_REQorREM_FIPS_REBOOT_REQ, the dialog is not shown.NAS-131326: Simplify HA upgrade logic
To test the
failover.upgradejob, follow these steps:Now you can see the Reboot Info notification, shown in pt "1" of the description of this PR.
Downstream