Skip to content
This repository was archived by the owner on Jan 20, 2021. It is now read-only.

Conversation

@utchoang
Copy link

Fixes #807

@utchoang
Copy link
Author

image
image

@utchoang
Copy link
Author

@rhtyd @svenvogel Please review it. Thanks.

@utchoang utchoang marked this pull request as ready for review October 15, 2020 10:05
@yadvr
Copy link
Member

yadvr commented Oct 15, 2020

@blueorangutan package

@blueorangutan
Copy link

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

@blueorangutan
Copy link

Packaging result: ✔️centos ✔️debian ✔️archive.
QA: http://primate-qa.cloudstack.cloud:8080/client/pr/808 (JID-3591)

@davidjumani
Copy link
Contributor

@utchoang There also needs to be a check for quiescevm on the volume
https://github.com/apache/cloudstack/blob/master/ui/scripts/instances.js#L1255

@utchoang
Copy link
Author

@blueorangutan package

@blueorangutan
Copy link

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

@blueorangutan
Copy link

Packaging result: ✔️centos ✔️debian ✔️archive.
QA: http://primate-qa.cloudstack.cloud:8080/client/pr/808 (JID-3605)

@yadvr yadvr requested a review from davidjumani October 28, 2020 09:42
@svenvogel
Copy link

@rhtyd @davidjumani is this ready to go? 😄

@svenvogel
Copy link

svenvogel commented Nov 13, 2020

@utchoang There also needs to be a check for quiescevm on the volume
https://github.com/apache/cloudstack/blob/master/ui/scripts/instances.js#L1255

@davidjumani so what i see is that there is no squeezing in managed storage. we dont need at the moment an quiesed button.

@utchoang
Copy link
Author

@blueorangutan package

@blueorangutan
Copy link

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

@utchoang
Copy link
Author

@davidjumani Please review again. Thank you.

@blueorangutan
Copy link

Packaging result: ✔️centos ✔️debian ✔️archive.
QA: http://primate-qa.cloudstack.cloud:8080/client/pr/808 (JID-3664)

@svenvogel
Copy link

@davidjumani @utchoang maybe I am wrong but there is NO qiescevm on a managed storage volume. I don’t know if there is something in VMware but I think the mechanism is another one. Please refer #3724 in cloudstack.
apache/cloudstack#3724 (comment)
If this so we can remove the quiescing and it’s at the Moment for volume snapshots not important.
@rhtyd can you clarify?

@davidjumani
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

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

@blueorangutan
Copy link

Packaging result: ✔️centos ✔️debian ✔️archive.
QA: http://primate-qa.cloudstack.cloud:8080/client/pr/808 (JID-3678)

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.

LGTM. However, this needs to be tested on VMware

@yadvr yadvr modified the milestone: 1.0-GA Nov 23, 2020
@yadvr yadvr added this to the 1.1 milestone Nov 23, 2020
@svenvogel
Copy link

svenvogel commented Nov 26, 2020

@davidjumani can you clarify where you use "quiescevm" in VMware and which feature you want to test? there are normal VM Snapshots and i only know volume snapshot like VVOLs. Do you speak about VVOLs (virtual volumes)? Can you link or clarify the test?

Maybe we dont need an VMware test because there is no functionality they use the volume snapshot and qiescing.

@davidjumani
Copy link
Contributor

@svenvogel Not too sure about it but I do see it as a parameter in volume snapshot as well as while creating VMware snapshots

@svenvogel
Copy link

@davidjumani how can we proceed? i see there is a parameter but which time it is used?
if there is no test scenario then why are we testing it? normally functionality is not changed to old UI.

take a look here. its included in the old UI.
image
image

@rhtyd @utchoang @davidjumani is the master frozen? i think this is ready to merge.

@utchoang
Copy link
Author

utchoang commented Dec 1, 2020

@davidjumani @rhtyd Do you test OK and ready to merge?

@GabrielBrascher
Copy link
Member

Well, from what I understand, what it matters here is not the hypervisor (VMware). But the volume itself with quiescevm enabled. One can test it by mocking such situation.

Also, the API returns such information on the list volumes call which enabled to implement the UI on same approach as the old UI. I don't see any problem here.

@davidjumani
Copy link
Contributor

davidjumani commented Dec 1, 2020

I too don't see any blocker here since it works as expected but would like the snapshot API call tested on a volume with quiescevm enabled (mocked also works fine)

@svenvogel
Copy link

svenvogel commented Jan 11, 2021

@rhtyd @davidjumani @GabrielBrascher can we merge? @utchoang could do it.

@utchoang
Copy link
Author

@rhtyd @davidjumani cc @svenvogel Is it possible to merge this PR?

@davidjumani
Copy link
Contributor

I'm okay with having this merged

Copy link
Member

@GabrielBrascher GabrielBrascher left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @utchoang! LGTM

@yadvr yadvr merged commit 004d765 into apache:master Jan 13, 2021
@utchoang utchoang deleted the feature/fix-issue-807 branch January 14, 2021 09:53
qrry added a commit to qrry/cloudstack-primate that referenced this pull request Jan 14, 2021
* master:
  Using post for uploadSslCert api  (apache#842)
  Allow enabling network/vpc offering at creation (apache#911)
  Display all data volumes for vm while destroying (apache#915)
  tools/docker: Change directory to docker.sh's directory (apache#916)
  FIX - compute - Create snapshot from virtual machine with managed storage (apache#808)
  migratewizard: Fix fetching jobid from api response (apache#913)
  component: remove Primate name from the footer, fix bug report link
  migratewizard: Display error and unfreeze form when api call fails (apache#912)
  continue with the Zone deployment without shared primary storage (apache#908)

# Conflicts:
#	src/components/page/GlobalFooter.vue
weizhouapache pushed a commit that referenced this pull request Jan 19, 2021
…rage (#808)

* compute: add button and modal `take VM volume snapshot`

* add awesome camera retro plugins

* modified to using component

* fix for quiescevm

* add quiescevm to api params
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[IIMPROVEMENT] create snapshot from virtual machine with managed storage

6 participants