New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add unpause VM feature #3985
Add unpause VM feature #3985
Conversation
65bc648
to
5eafb60
Compare
frontend/packages/kubevirt-plugin/src/components/vms/menu-actions.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/kubevirt-plugin/src/constants/vm/constants.ts
Outdated
Show resolved
Hide resolved
frontend/packages/kubevirt-plugin/src/components/vms/vm-resource.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/kubevirt-plugin/src/selectors/vmi/selectors.ts
Outdated
Show resolved
Hide resolved
2161e61
to
328cd50
Compare
frontend/packages/kubevirt-plugin/src/components/modals/vm-status-modal/vm-status-modal.tsx
Outdated
Show resolved
Hide resolved
const VMStatusModalComponent = ({ | ||
vmi, | ||
isOpen, | ||
setOpen, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be better to pass close
which doesnt take any parameters. Same as I mentioned in Gilad's PR #3805 (comment) - he is going to do that in a follow up so this should be done too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I'll do this in a follow-up, as well.
frontend/packages/kubevirt-plugin/src/components/vm-status/vm-status.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/kubevirt-plugin/src/components/vm-status/vm-status.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/kubevirt-plugin/src/components/vms/menu-actions.tsx
Outdated
Show resolved
Hide resolved
|
||
const footer = ( | ||
<ModalFooter | ||
errorMessage={showPatchError && errorMessage} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really need showPatchError. Can't we just show errorMessage directly?
frontend/packages/kubevirt-plugin/src/k8s/requests/vmi/actions.ts
Outdated
Show resolved
Hide resolved
frontend/packages/kubevirt-plugin/src/components/vms/menu-actions.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/kubevirt-plugin/src/k8s/requests/vmi/actions.ts
Outdated
Show resolved
Hide resolved
url = `${url}/${action}`; | ||
|
||
const response = await coFetch(url, { method }); | ||
const text = await response.text(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's just return the text - please update the VMActionRequest as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could also parametrize this method by the model, but I am not sure we want to do that
@@ -102,6 +115,11 @@ export const VMStatus: React.FC<VMStatusProps> = ({ | |||
)}`; // to default tab | |||
const additionalText = verbose ? getAdditionalImportText(statusDetail.pod) : null; | |||
|
|||
const unpauseVMI = async (event) => { | |||
event.preventDefault(); | |||
await unpauseVM(vmi); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we do something here when this fails?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the error should be handled in some way. Lets do it as a follow up @pcbailey
@@ -237,6 +242,7 @@ export const getVMStatus = ({ | |||
}): VMStatus => { | |||
const launcherPod = findVMPod(vm, pods); | |||
return ( | |||
isPaused(vmi) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO importing should be first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok let's keep the priority for that one
I see now, that we do not want to expose this feature |
d32edd2
to
4d9691d
Compare
/lgtm |
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/hold |
/hold cancel |
/retest Please review the full test history for this PR and help us cut down flakes. |
4d9691d
to
2c3f1e6
Compare
/retest |
/lgtm |
/retest |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pcbailey, rawagner The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
@pcbailey: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
This PR adds the option to unpause a paused VM from the overview tab, list view, and actions menu.
List view:
Overview tab: