Skip to content

Commit

Permalink
Fix page crash on invalid domain startup times
Browse files Browse the repository at this point in the history
When the log file contains a "starting up" timestamp which `new Date()`
cannot parse, the page oopsed with a `RangeError` from
`distanceToNow()`.

Change `domainGetStartTime()` to return a Date object instead of a raw
string, to keep the knowledge of the date format/parsing entirely within
the function. Check for invalid dates and return `null` in the error
cases, which the confirmation dialog already handles.

Also, fix the `grep` to look for the same pattern that the `.split()`
uses, to avoid mismatches.

In the confirmation dialog, rename `uptime` to `startTime`, as that's
what it is -- uptime is a duration, which is only computed inside of the
render function, but that state is an absolute time stamp.

https://issues.redhat.com/browse/RHEL-19878
  • Loading branch information
martinpitt committed Jan 5, 2024
1 parent 8e04059 commit a7e93e8
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 8 deletions.
8 changes: 4 additions & 4 deletions src/components/vm/confirmDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ const _ = cockpit.gettext;

export const ConfirmDialog = ({ idPrefix, actionsList, title, titleIcon, vm }) => {
const Dialogs = useDialogs();
const [uptime, setUptime] = useState();
const [startTime, setStartTime] = useState();

useEffect(() => {
return domainGetStartTime({ connectionName: vm.connectionName, vmName: vm.name })
.then(res => setUptime(res))
.then(res => setStartTime(res))
.catch(e => console.error(JSON.stringify(e)));
}, [vm]);

Expand Down Expand Up @@ -64,11 +64,11 @@ export const ConfirmDialog = ({ idPrefix, actionsList, title, titleIcon, vm }) =
titleIconVariant={titleIcon}
isOpen
footer={actions}>
{uptime
{startTime
? <DescriptionList isHorizontal isFluid>
<DescriptionListGroup>
<DescriptionListTerm>{_("Uptime")}</DescriptionListTerm>
<DescriptionListDescription id="uptime">{distanceToNow(new Date(uptime))}</DescriptionListDescription>
<DescriptionListDescription id="uptime">{distanceToNow(startTime)}</DescriptionListDescription>
</DescriptionListGroup>
</DescriptionList>
/* for tests */
Expand Down
12 changes: 8 additions & 4 deletions src/libvirtApi/domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -790,16 +790,20 @@ export function domainGetStartTime({

// Use libvirt APIs for getting VM start up time when it's implemented:
// https://gitlab.com/libvirt/libvirt/-/issues/481
return cockpit.script(`grep 'starting up' '${logFile}' | tail -1`, options);
return cockpit.script(`grep ': starting up' '${logFile}' | tail -1`, options);
})
.then(line => {
// Line from a log with a start up time is expected to look like this:
// 2023-05-05 11:22:03.043+0000: starting up libvirt version: 8.6.0, package: 3.fc37 (Fedora Project, 2022-08-09-13:54:03, ), qemu version: 7.0.0qemu-7.0.0-9.fc37, kernel: 6.0.6-300.fc37.x86_64, hostname: fedora

// Alternatively regex line.match(/\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}/g) can be used
const timeStr = line.split(': starting')[0];

return Promise.resolve(timeStr);
const timeStr = line.split(': starting up')[0];
const date = new Date(timeStr);
return isNaN(date) ? null : date;
})
.catch(ex => {
console.log("Unable to detect domain start time:", ex);
return null;
});
}

Expand Down
16 changes: 16 additions & 0 deletions test/check-machines-lifecycle
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,22 @@ class TestMachinesLifecycle(VirtualMachinesCase):
b.click(".pf-v5-c-modal-box__footer button:contains(Cancel)")
b.wait_not_present("#vm-subVmTest2-system-confirm-action-modal")

# uptime parsing robustness: unparseable format
self.machine.execute(f"sed -i '/: starting up/ s/^/invalidtime/' {args2['qemulog']}")
b.click("#vm-subVmTest2-system-shutdown-button")
b.wait_visible("#vm-subVmTest2-system-confirm-action-modal")
b._wait_present(".uptime-not-available")
b.click(".pf-v5-c-modal-box__footer button:contains(Cancel)")
b.wait_not_present("#vm-subVmTest2-system-confirm-action-modal")

# uptime parsing robustness: no "starting up" line
self.machine.execute(f"sed -i '/: starting up/d' {args2['qemulog']}")
b.click("#vm-subVmTest2-system-shutdown-button")
b.wait_visible("#vm-subVmTest2-system-confirm-action-modal")
b._wait_present(".uptime-not-available")
b.click(".pf-v5-c-modal-box__footer button:contains(Cancel)")
b.wait_not_present("#vm-subVmTest2-system-confirm-action-modal")

b.set_input_text("#text-search", "subVmTest2")
self.waitVmRow("subVmTest2")
self.waitVmRow("subVmTest1", "system", False)
Expand Down

0 comments on commit a7e93e8

Please sign in to comment.