Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

server: detach data disks before destroying vms#5356

Merged
nvazquez merged 1 commit intoapache:4.15apache/cloudstack:4.15from
shapeblue:4.15-detach-volumes-before-destroy-vmshapeblue/cloudstack:4.15-detach-volumes-before-destroy-vmCopy head branch name to clipboard
Aug 28, 2021
Merged

server: detach data disks before destroying vms#5356
nvazquez merged 1 commit intoapache:4.15apache/cloudstack:4.15from
shapeblue:4.15-detach-volumes-before-destroy-vmshapeblue/cloudstack:4.15-detach-volumes-before-destroy-vmCopy head branch name to clipboard

Conversation

@weizhouapache
Copy link
Contributor

@weizhouapache weizhouapache commented Aug 23, 2021

Description

This PR fixes #4902

The testing results with this change:

  • When destroy VM without expunge and when no data disk is selected for deletion
    destroy vm: VM is stopped, data disk is detached from vm (in Ready state)
    recover vm: vm is ready only with root disk.

  • When destroy VM without expunge and when one or more data disks are selected for deletion
    destroy vm: VM is stopped, Data disks marked for deletion are detached and in Destroy state . the remaining disks are detached from VM
    recover vm: vm is ready only with root disk.
    recover data disk in Destroy: volume is ready, and not attached to vm.

  • When destroy VM with expunge and when no data disk is selected for deletion
    VM is destroyed and expunged completely.
    data disks are detached from vm.

  • When destroy VM with expunge and few data disks are selected for deletion
    VM is destroyed and expunged completely.
    Data disks marked for deletion are deleted. the remaining disks are detached from VM

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@weizhouapache
Copy link
Contributor Author

weizhouapache commented Aug 23, 2021

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Aug 23, 2021

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

@blueorangutan
Copy link

blueorangutan commented Aug 23, 2021

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 978

@nvazquez nvazquez added this to the 4.16.0.0 milestone Aug 24, 2021
nvazquez
nvazquez approved these changes Aug 24, 2021
View reviewed changes
Copy link
Contributor

@nvazquez nvazquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - needs testing

@nvazquez
Copy link
Contributor

nvazquez commented Aug 24, 2021

@weizhouapache can you target this PR to the main branch?

@yadvr yadvr changed the base branch from 4.15 to main August 24, 2021 07:11
@yadvr
Copy link
Member

yadvr commented Aug 24, 2021

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Aug 24, 2021

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

@blueorangutan
Copy link

blueorangutan commented Aug 24, 2021

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 993

@sureshanaparti
Copy link
Contributor

sureshanaparti commented Aug 24, 2021

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Aug 24, 2021

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

@weizhouapache weizhouapache reopened this Aug 24, 2021
@blueorangutan
Copy link

blueorangutan commented Aug 24, 2021

Trillian test result (tid-1761)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33898 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5356-t1761-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

davidjumani
davidjumani approved these changes Aug 25, 2021
View reviewed changes
Copy link
Contributor

@davidjumani davidjumani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with multiple DataDisks, all 4 scenarios work as expected

@weizhouapache weizhouapache reopened this Aug 26, 2021
@weizhouapache
Copy link
Contributor Author

weizhouapache commented Aug 26, 2021

Tested with multiple DataDisks, all 4 scenarios work as expected

great, thanks for testing @davidjumani

@rhtyd @PaulAngus @DaanHoogland @nvazquez
are the expected results correct to you?

@DaanHoogland
Copy link
Contributor

DaanHoogland commented Aug 27, 2021

clgtm, tests done. I've given the failed travis tests another spin.

@yadvr
Copy link
Member

yadvr commented Aug 27, 2021

@weizhouapache does this affect 4.15 too? If so, pl raise PR against 4.15 branch if it's a bug fix. Thanks.

@weizhouapache weizhouapache changed the base branch from main to 4.15 August 27, 2021 11:03
@weizhouapache
Copy link
Contributor Author

weizhouapache commented Aug 27, 2021

@weizhouapache does this affect 4.15 too? If so, pl raise PR against 4.15 branch if it's a bug fix. Thanks.

@rhtyd @nvazquez changed base branch back to 4.15

@nvazquez
Copy link
Contributor

nvazquez commented Aug 27, 2021

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Aug 27, 2021

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

@blueorangutan
Copy link

blueorangutan commented Aug 27, 2021

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 1043

@DaanHoogland
Copy link
Contributor

DaanHoogland commented Aug 27, 2021

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Aug 27, 2021

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

@nvazquez nvazquez modified the milestones: 4.16.0.0, 4.15.2.0 Aug 27, 2021
@blueorangutan
Copy link

blueorangutan commented Aug 28, 2021

Trillian test result (tid-1814)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35434 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5356-t1814-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
Smoke tests completed. 87 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

@nvazquez nvazquez merged commit e968f3b into apache:4.15 Aug 28, 2021
@weizhouapache weizhouapache mentioned this pull request Sep 21, 2021
Closed
@weizhouapache weizhouapache deleted the 4.15-detach-volumes-before-destroy-vm branch December 9, 2022 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DaanHoogland DaanHoogland DaanHoogland approved these changes

@nvazquez nvazquez nvazquez approved these changes

@davidjumani davidjumani davidjumani approved these changes

@PaulAngus PaulAngus Awaiting requested review from PaulAngus

@yadvr yadvr Awaiting requested review from yadvr

Assignees

@weizhouapache weizhouapache

Labels

type:improvement

Projects

None yet

Milestone

4.15.2.0

Development

Successfully merging this pull request may close these issues.

Destroy VM should detach the volumes from VM and recover VM should not try to attach the volumes again

7 participants

@weizhouapache @blueorangutan @nvazquez @yadvr @sureshanaparti @DaanHoogland @davidjumani
Morty Proxy This is a proxified and sanitized view of the page, visit original site.