Bug 1873402: Add deleting status #6565
Conversation
openshift-ci-robot
added
bugzilla/severity-high
|
@yaacov: This pull request references Bugzilla bug 1873402, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
DetailsIn response to this:
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. |
openshift-ci-robot
added
the
approved
yaacov
force-pushed
the
add-vm-deleting-status
branch
from
a0f0b2e to
65ae9b4
Compare
yaacov
force-pushed
the
add-vm-deleting-status
branch
from
65ae9b4 to
c42c449
Compare
| isVMIPaused(vmi) ? { status: VMStatus.PAUSED, message: PAUSED_VM_MODAL_MESSAGE } : null; | ||
|
|
||
| const isDeleting = (vm: VMKind, vmi: VMIKind): VMStatusBundle => | ||
| (vm && !!getDeletetionTimestamp(vm)) || (!vm && vmi && !!getDeletetionTimestamp(vmi)) |
There was a problem hiding this comment.
is there a situation when you have a VMI object without a VM ?
There was a problem hiding this comment.
yes, it was a user request:
https://issues.redhat.com/browse/KNIP-1165
#3841
| static readonly STOPPING = new VMStatus('VMStatus_STOPPING', VMStatusSimpleLabel.Stopping, { | ||
| isInProgress: true, | ||
| }); | ||
| static readonly DELETING = new VMStatus('VMStatus_DELETING', VMStatusSimpleLabel.Deleting, { |
There was a problem hiding this comment.
has to be also mentioned in topology/components/nodes/VmNode.tsx
| Stopping = 'Stopping', | ||
| Running = 'Running', | ||
| Off = 'Off', | ||
| Deleting = 'Deleting', |
There was a problem hiding this comment.
has to be mentioned in getVMStatusGroups
| const isPaused = (vmi: VMIKind): VMStatusBundle => | ||
| isVMIPaused(vmi) ? { status: VMStatus.PAUSED, message: PAUSED_VM_MODAL_MESSAGE } : null; | ||
|
|
||
| const isDeleting = (vm: VMKind, vmi: VMIKind): VMStatusBundle => |
There was a problem hiding this comment.
+1 much better resolution than what we had before in vm.tsx
There was a problem hiding this comment.
please move the function to be in order they are called in the status
| isBeingMigrated(vm, vmi, migrations) || | ||
| isBeingImported(vm, pods, pvcs, dataVolumes) || | ||
| isDeleting(vm, vmi) || | ||
| isVMError(vm) || |
There was a problem hiding this comment.
IMO we should move it behind isVMError
There was a problem hiding this comment.
That is the bug, users see "vmi-error" while they need to see "deleting"
There was a problem hiding this comment.
this is not a vmi error but possible vm error - which should have a prefference IMO, as it could say a reason why something cannot be deleted even though it has deleteionTimestamp
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: suomiy, yaacov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds 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. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@yaacov: All pull requests linked via external trackers have merged: Bugzilla bug 1873402 has been moved to the MODIFIED state. DetailsIn response to this:
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. |
7 participants
When a running VM is deleted, it may take up to terminationGracePeriod for the VMI to be deleted. In that period of time the VM is removed from the UI and there's no indication for the user that the VMI is still running.
This PR adds a "Deleting" status for VMIs with deleting timestamp.
Screen shot:
