Skip to content
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

server: fix for inactive service offering for VM #3320

Merged
merged 2 commits into from May 27, 2019

Conversation

Projects
None yet
5 participants
@shwstppr
Copy link
Contributor

commented May 9, 2019

Description

Fixes #3315
Currently, the code was allowed to change service offering for VM to a deleted or inactive service offering. Added check for it to throw an exception.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

Using cmk

server: fix for inactive service offering for VM (#3315)
Fixes #3315
Currently code was allowing to change service offering for VM to a deleted or inactive service offering. Added check for it to throw an exception.

Signed-off-by: Abhishek Kumar <abhishek.kumar@shapeblue.com>
@@ -965,6 +965,9 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE
// Check resource limits for CPU and Memory.
Map<String, String> customParameters = cmd.getDetails();
ServiceOfferingVO newServiceOffering = _offeringDao.findById(svcOffId);
if (newServiceOffering.getState() == DiskOffering.State.Inactive) {
throw new InvalidParameterValueException(String.format("Unable to upgrade virtual machine %s with an inactive service offering %s", vmInstance.toString(), newServiceOffering.getUuid()));

This comment has been minimized.

Copy link
@anuragaw

anuragaw May 10, 2019

Contributor

nit - vmInstance.name?

This comment has been minimized.

Copy link
@shwstppr

shwstppr May 10, 2019

Author Contributor

@anuragaw instead vmInstance.getUuid()? As we show UUIDs mostly.

show VM UUID in exception error message
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@anuragaw

This comment has been minimized.

Copy link
Contributor

commented May 20, 2019

LGTM

@rhtyd

This comment has been minimized.

Copy link
Member

commented May 23, 2019

@blueorangutan package

@rhtyd rhtyd added this to the 4.13.0.0 milestone May 23, 2019

@rhtyd

rhtyd approved these changes May 23, 2019

@blueorangutan

This comment has been minimized.

Copy link

commented May 23, 2019

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@rhtyd rhtyd added the type:bug label May 23, 2019

@blueorangutan

This comment has been minimized.

Copy link

commented May 23, 2019

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-2750

@shwstppr

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2019

@blueorangutan package

@blueorangutan

This comment has been minimized.

Copy link

commented May 23, 2019

@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

This comment has been minimized.

Copy link

commented May 23, 2019

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2753

@DaanHoogland

This comment has been minimized.

Copy link
Contributor

commented May 23, 2019

@blueorangutan

This comment has been minimized.

Copy link

commented May 23, 2019

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@DaanHoogland
Copy link
Contributor

left a comment

test suite running

@@ -965,6 +965,9 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE
// Check resource limits for CPU and Memory.
Map<String, String> customParameters = cmd.getDetails();
ServiceOfferingVO newServiceOffering = _offeringDao.findById(svcOffId);
if (newServiceOffering.getState() == DiskOffering.State.Inactive) {
throw new InvalidParameterValueException(String.format("Unable to upgrade virtual machine %s with an inactive service offering %s", vmInstance.getUuid(), newServiceOffering.getUuid()));

This comment has been minimized.

Copy link
@DaanHoogland

DaanHoogland May 23, 2019

Contributor

👍

@rhtyd

This comment has been minimized.

Copy link
Member

commented May 23, 2019

@blueorangutan package

@blueorangutan

This comment has been minimized.

Copy link

commented May 23, 2019

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

This comment has been minimized.

Copy link

commented May 23, 2019

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2769

@blueorangutan

This comment has been minimized.

Copy link

commented May 23, 2019

Trillian test result (tid-3571)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32146 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3320-t3571-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 69 look OK, 1 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_02_cancel_host_maintenace_with_migration_jobs Error 3.52 test_host_maintenance.py

@rhtyd rhtyd requested a review from PaulAngus May 24, 2019

@rhtyd rhtyd merged commit d1090c0 into apache:master May 27, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Jenkins This pull request looks good
Details

gmueller-ewerk added a commit to gmueller-ewerk/cloudstack that referenced this pull request May 28, 2019

server: fix for inactive service offering for VM (apache#3320)
Fixes apache#3315
Currently, the code was allowed to change service offering for VM to a deleted or inactive service offering. Added check for it to throw an exception.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.