-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Prevent starting a VM in destroyed state (or any state but Stopped) #5165
Conversation
@blueorangutan package |
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔️ centos7 ✖️ centos8 ✔️ debian. SL-JID 422 |
@blueorangutan package |
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 424 |
@blueorangutan test |
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
@blueorangutan package |
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 425 |
Trillian Build Failed (tid-1159) |
@blueorangutan test |
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
s_logger.warn("VM " + vm + " is not in a state to be started: " + state); | ||
throw new CloudRuntimeException(String.format("Cannot start VM: %s in %s state", vm, state)); |
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 unify these messages in a String var (with String.format).
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.
s_logger.warn("VM " + vm + " is not in a state to be started: " + state); | |
throw new CloudRuntimeException(String.format("Cannot start VM: %s in %s state", vm, state)); | |
String msg = String.format("Cannot start VM: %s in %s state", vm, state) | |
s_logger.warn(msg); | |
throw new CloudRuntimeException(msg)); |
Trillian test result (tid-1160)
|
@blueorangutan package |
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 438 |
@blueorangutan test |
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-1179)
|
@blueorangutan test |
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-1190)
|
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.
cltgm
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.
CLGTM; I raised a last point about toString
.
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 475 |
@blueorangutan test |
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-1205)
|
return null; | ||
String msg = String.format("Cannot start %s in %s state", vm, state); | ||
s_logger.warn(msg); | ||
throw new CloudRuntimeException(msg); |
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.
code LGTM
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.
Functionality tested - LGTM.
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.
Code LGTM
Merging this based on LGTMs, manual and smoke tests. |
Description
A destroyed VM when attempted to be started (using the bulk action support in the UI), results in a successful operation, however, the VM doesn't actually start. This PR fixes the false positive result shown in the above mentioned condition
VM continues to remain in destroyed state - but a success response is returned
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Tried starting an instance in destroyed state: