Skip to content

Commit

Permalink
Add confirmation dialog for shutdown and reboot operation
Browse files Browse the repository at this point in the history 
Fixes #994
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2180281
Fixes https://issues.redhat.com/browse/RHELPLAN-152520
  • Loading branch information
@skobyda
skobyda committed Jun 26, 2023
1 parent b6a1977 commit ca21440
Show file tree
Hide file tree
Showing 5 changed files with 204 additions and 9 deletions.
76 changes: 76 additions & 0 deletions src/components/vm/confirmDialog.jsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* This file is part of Cockpit.
*
* Copyright (C) 2023 Red Hat, Inc.
*
* Cockpit is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation; either version 2.1 of the License, or
* (at your option) any later version.
*
* Cockpit is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with Cockpit; If not, see <http://www.gnu.org/licenses/>.
*/
import cockpit from 'cockpit';
import React, { useEffect, useState } from 'react';
import { Button } from "@patternfly/react-core/dist/esm/components/Button";
import { DescriptionList, DescriptionListDescription, DescriptionListGroup, DescriptionListTerm } from "@patternfly/react-core/dist/esm/components/DescriptionList";
import { Modal } from "@patternfly/react-core/dist/esm/components/Modal";
import { useDialogs } from 'dialogs.jsx';
import { distanceToNow } from 'timeformat.js';

import { domainGetStartTime } from '../../libvirtApi/domain.js';

const _ = cockpit.gettext;

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

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

const actions = actionsList.map(action =>
<Button variant={action.variant}
key={action.id}
id={`${idPrefix}-${action.id}`}
onClick={() => {
action.handler();
Dialogs.close();
}}>
{action.name}
</Button>
);
actions.push(
<Button variant="link" key="cancel" onClick={Dialogs.close}>
{_("Cancel")}
</Button>
);

return (
<Modal id={`${idPrefix}-confirm-action-modal`}
position="top"
variant="small"
onClose={Dialogs.close}
title={title}
titleIconVariant={titleIcon}
isOpen
footer={actions}>
{uptime &&
<DescriptionList isHorizontal isFluid>
<DescriptionListGroup>
<DescriptionListTerm>{_("Uptime")}</DescriptionListTerm>
<DescriptionListDescription id="uptime">{distanceToNow(new Date(uptime))}</DescriptionListDescription>
</DescriptionListGroup>
</DescriptionList>}
</Modal>
);
};
104 changes: 98 additions & 6 deletions src/components/vm/vmActions.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@ import PropTypes from 'prop-types';
import { Button } from "@patternfly/react-core/dist/esm/components/Button";
import { Dropdown, DropdownItem, DropdownSeparator, KebabToggle } from "@patternfly/react-core/dist/esm/deprecated/components/Dropdown";
import { Tooltip } from "@patternfly/react-core/dist/esm/components/Tooltip";
import { PowerOffIcon, RedoIcon } from '@patternfly/react-icons';
import { useDialogs } from 'dialogs.jsx';
import { fmt_to_fragments } from 'utils.jsx';

import { updateVm } from '../../actions/store-actions.js';
import {
vmId,
} from "../../helpers.js";

import { CloneDialog } from './vmCloneDialog.jsx';
import { ConfirmDialog } from './confirmDialog.jsx';
import { DeleteDialog } from "./deleteDialog.jsx";
import { MigrateDialog } from './vmMigrateDialog.jsx';
import { RenameDialog } from './vmRenameDialog.jsx';
Expand Down Expand Up @@ -227,29 +230,92 @@ const VmActions = ({ vm, onAddErrorNotification, isDetailsPage }) => {
variant={isDetailsPage ? 'primary' : 'secondary'}
isLoading={operationInProgress}
isDisabled={operationInProgress}
onClick={() => { setOperationInProgress(true); onShutdown(vm, setOperationInProgress) }} id={`${id}-shutdown-button`}>
id={`${id}-shutdown-button`}
onClick={() => Dialogs.show(
<ConfirmDialog idPrefix={id}
title={fmt_to_fragments(_("Shut down $0?"), <b>{vm.name}</b>)}
titleIcon={PowerOffIcon}
vm={vm}
actionsList={[
{
variant: "primary",
handler: () => {
setOperationInProgress(true);
onShutdown(vm, setOperationInProgress);
},
name: _("Shut down"),
id: "off",
},
{
variant: "secondary",
handler: () => {
onReboot(vm);
},
name: _("Reboot"),
id: "reboot",
},
]} />
)}>
{_("Shut down")}
</Button>
);
dropdownItems.push(
<DropdownItem key={`${id}-off`}
id={`${id}-off`}
onClick={() => onShutdown(vm)}>
onClick={() => Dialogs.show(
<ConfirmDialog idPrefix={id}
title={fmt_to_fragments(_("Shut down $0?"), <b>{vm.name}</b>)}
titleIcon={PowerOffIcon}
vm={vm}
actionsList={[
{
variant: "primary",
handler: () => onShutdown(vm),
name: _("Shut down"),
id: "off",
},
]} />
)}>
{_("Shut down")}
</DropdownItem>
);
dropdownItems.push(
<DropdownItem key={`${id}-forceOff`}
id={`${id}-forceOff`}
onClick={() => onForceoff(vm)}>
onClick={() => Dialogs.show(
<ConfirmDialog idPrefix={id}
title={fmt_to_fragments(_("Force shut down $0?"), <b>{vm.name}</b>)}
titleIcon={PowerOffIcon}
vm={vm}
actionsList={[
{
variant: "primary",
handler: () => onForceoff(vm),
name: _("Force shut down"),
id: "forceOff",
},
]} />
)}>
{_("Force shut down")}
</DropdownItem>
);
dropdownItems.push(<DropdownSeparator key="separator-shutdown" />);
dropdownItems.push(
<DropdownItem key={`${id}-sendNMI`}
id={`${id}-sendNMI`}
onClick={() => onSendNMI(vm)}>
onClick={() => Dialogs.show(
<ConfirmDialog idPrefix={id}
title={fmt_to_fragments(_("Send non-maskable interrupt to $0?"), <b>{vm.name}</b>)}
vm={vm}
actionsList={[
{
variant: "primary",
handler: () => onSendNMI(vm),
name: _("Send non-maskable interrupt"),
id: "sendNMI",
},
]} />
)}>
{_("Send non-maskable interrupt")}
</DropdownItem>
);
Expand All @@ -260,14 +326,40 @@ const VmActions = ({ vm, onAddErrorNotification, isDetailsPage }) => {
dropdownItems.push(
<DropdownItem key={`${id}-reboot`}
id={`${id}-reboot`}
onClick={() => onReboot(vm)}>
onClick={() => Dialogs.show(
<ConfirmDialog idPrefix={id}
title={fmt_to_fragments(_("Reboot $0?"), <b>{vm.name}</b>)}
titleIcon={RedoIcon}
vm={vm}
actionsList={[
{
variant: "primary",
handler: onReboot(vm),
name: _("Reboot"),
id: "reboot",
},
]} />
)}>
{_("Reboot")}
</DropdownItem>
);
dropdownItems.push(
<DropdownItem key={`${id}-forceReboot`}
id={`${id}-forceReboot`}
onClick={() => onForceReboot(vm)}>
onClick={() => Dialogs.show(
<ConfirmDialog idPrefix={id}
title={fmt_to_fragments(_("Force reboot $0?"), <b>{vm.name}</b>)}
vm={vm}
titleIcon={RedoIcon}
actionsList={[
{
variant: "primary",
handler: onForceReboot(vm),
name: _("Force reboot"),
id: "forceReboot",
},
]} />
)}>
{_("Force reboot")}
</DropdownItem>
);
Expand Down
25 changes: 23 additions & 2 deletions test/check-machines-lifecycle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ class TestMachinesLifecycle(VirtualMachinesCase):
# the shut off button beside the VM name
self.waitCirrOSBooted(args['logfile'])
b.click("#vm-subVmTest1-system-shutdown-button")
b.wait_visible("#vm-subVmTest1-system-confirm-action-modal")
b.click(".pf-c-modal-box__footer #vm-subVmTest1-system-off")
b.wait_in_text("#vm-subVmTest1-system-state", "Shut off")
b.wait_visible("#vm-subVmTest1-system-run")

Expand Down Expand Up @@ -211,7 +213,7 @@ class TestMachinesLifecycle(VirtualMachinesCase):
m.execute("virsh define --file /tmp/subVmTest1.xml")

# start another one, should appear automatically
self.createVm("subVmTest2")
args2 = self.createVm("subVmTest2")
b.wait_in_text("#vm-subVmTest2-system-state", "Running")
self.goToVmPage("subVmTest2")
b.wait_in_text("#vm-subVmTest2-cpu", "1 vCPU")
Expand All @@ -231,6 +233,23 @@ class TestMachinesLifecycle(VirtualMachinesCase):

b.click("#vm-subVmTest2-system-run")

# reboot through shutdown dialog
self.machine.execute(f"echo '' > {args2['logfile']}")
b.click("#vm-subVmTest2-system-shutdown-button")
b.wait_visible("#vm-subVmTest2-system-confirm-action-modal")
b.click(".pf-c-modal-box__footer button:contains(Reboot)")
b.wait_not_present("#vm-subVmTest2-system-confirm-action-modal")
b.wait_in_text("#vm-subVmTest2-system-state", "Running")
self.waitCirrOSBooted(args2['logfile'])

# Check uptime
b.click("#vm-subVmTest2-system-shutdown-button")
b.wait_visible("#vm-subVmTest2-system-confirm-action-modal")
if run_pixel_tests:
b.assert_pixels("#vm-subVmTest2-system-confirm-action-modal", "shutdown-confirm-dialog")
b.click(".pf-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 Expand Up @@ -472,8 +491,10 @@ class TestMachinesLifecycle(VirtualMachinesCase):
m.execute(f"virsh undefine {name}")
b.wait_visible(f"tr[data-row-id=vm-{name}-system][data-vm-transient=true]")
b.click(f"#vm-{name}-system-action-kebab button")
b.wait_visible(f"#vm-{name}-system-delete a.pf-m-disabled")
b.wait_visible(f"#vm-{name}-system-delete a.pf-m-disabled") # delete buton should be disabled
b.click(f"#vm-{name}-system-forceOff")
b.wait_visible(f"#vm-{name}-system-confirm-action-modal")
b.click(f".pf-c-modal-box__footer #vm-{name}-system-forceOff")
self.waitVmRow(name, 'system', False)
b.wait_not_present(f'#vm-{name}-system-state button:contains("view more")')

Expand Down
6 changes: 6 additions & 0 deletions test/machineslib.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ def performAction(self, vmName, action, checkExpectedState=True, connectionName=
b.wait_visible("#vm-{0}-{1}-action-kebab > .pf-c-dropdown__menu".format(vmName, connectionName))
b.click("#vm-{0}-{1}-{2} a".format(vmName, connectionName, action))

# Some actions, which can cause expensive downtime when clicked accidentally, have confirmation dialog
if action in ["off", "forceOff", "reboot", "forceReboot", "sendNMI"]:
b.wait_visible("#vm-{0}-{1}-confirm-action-modal".format(vmName, connectionName))
b.click(".pf-c-modal-box__footer #vm-{0}-{1}-{2}".format(vmName, connectionName, action))
b.wait_not_present("#vm-{0}-{1}-confirm-action-modal".format(vmName, connectionName))

if not checkExpectedState:
return

Expand Down
2 changes: 1 addition & 1 deletion test/reference
Submodule reference updated 5 files
+ − TestMachinesLifecycle-testBasic-shutdown-confirm-dialog-dark-pixels.png
+ − TestMachinesLifecycle-testBasic-shutdown-confirm-dialog-medium-pixels.png
+ − TestMachinesLifecycle-testBasic-shutdown-confirm-dialog-mobile-pixels.png
+ − TestMachinesLifecycle-testBasic-shutdown-confirm-dialog-pixels.png
+ − TestMachinesLifecycle-testBasic-shutdown-confirm-dialog-rtl-pixels.png

0 comments on commit ca21440

Please sign in to comment.