fix issue #3590 'Revert Ceph/RBD Snapshot'#3605
Conversation
|
@andrijapanicsb Can you please check if RBD snapshot rollback will work for you with this fix? Thanks! |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-291 |
|
Regression test is not necessary as change is very specific to a wrapper/handler related to RBD/ceph that trillian does not test now. Manual testing/confirmation needed. |
|
Pinging for review @andrijapanicsb @DaanHoogland @rhtyd @ustcweizhou @kiwiflyer @wido @svenvogel @rafaelweingartner @mike-tutkowski and others :-) |
|
I don't have ceph setup to test this, maybe @andrijapanicsb can review? |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-329 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-426)
|
There was a problem hiding this comment.
I can not comment though the code looks good. My experience with the rbd block device and roll backs is not good and though it looks like the roll back is on FS level, I refrain from judgement either way. (This is actually a quite positive comment, if i may say so myself ;)
|
I can test this at some point, have an env, which I can upgrade... |
|
@GabrielBrascher this seems to be a wrong branch to merge to (4.13) - please choose the master branch and ping me when done. |
|
Targeting 4.13 branch is fine as ultimately we forward merge the change to master @andrijapanicsb |
|
I can’t test upgrade since I already run “master” on this test env…
I can test clean install, which requires a manually deployed CEPH cluster – and I’m not doing that (due to time constraints).
Is it possible to rebase against master, so I can test?
Regards,
Andrija Panic
From: Rohit Yadav <notifications@github.com>
Sent: 06 November 2019 12:12
To: apache/cloudstack <cloudstack@noreply.github.com>
Cc: Andrija Panic <andrija.panic@shapeblue.com>; Mention <mention@noreply.github.com>
Subject: Re: [apache/cloudstack] fix issue #3590 'Revert Ceph/RBD Snapshot' (#3605)
Targeting 4.13 branch is fine as ultimately we forward merge the change to master @andrijapanicsb<https://github.com/andrijapanicsb>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#3605?email_source=notifications&email_token=AK5EN3OCPQUS2NYBBUCV6J3QSKQ6NA5CNFSM4IZ2YD42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDGFVUQ#issuecomment-550263506>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AK5EN3KRRC3XDKA6NF7X2NTQSKQ6NANCNFSM4IZ2YD4Q>.
Andrija Panic
Cloud Architect
s: +44 20 3603 0540 | m: +381 69 272 3690
e: andrija.panic@shapeblue.com | w: www.shapeblue.com | t: @shapeblue
a: Amadeus House, Floral Street, London WC2E 9DPUK
Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error.
Find out more about ShapeBlue and our range of CloudStack related services:
IaaS Cloud Design & Build | CSForge - rapid IaaS deployment framework
CloudStack Consulting | CloudStack Software Engineering
CloudStack Infrastructure Support | CloudStack Bootcamp Training Courses
|
|
I kept this PR aiming the branch 4.13 as it is a simple fix and it would be nice to have it released in 4.13.1.0. @andrijapanicsb I ported this changes on commit e8ec868687bf657e1d873bba229e403bf014d0bc, located in branch fix_revert_rbd_snapshot_on_master. The new branch/commit is based on CloudStack master branch and can be tested on a ACS environment running '4.14.0.0-SNAPSHOT'. Would that be ok for you? I can also generate the packages for a distro and make them available here. Thanks for the review/test :) |
|
OK, I get it now – makes sense.
I’m fine with either packages or simply a PR in the master, against which I can build packages.
Let me know – thx!
Regards,
Andrija Panic
From: Gabriel Beims Bräscher <notifications@github.com>
Sent: 06 November 2019 12:58
To: apache/cloudstack <cloudstack@noreply.github.com>
Cc: Andrija Panic <andrija.panic@shapeblue.com>; Mention <mention@noreply.github.com>
Subject: Re: [apache/cloudstack] fix issue #3590 'Revert Ceph/RBD Snapshot' (#3605)
I kept this PR aiming the branch 4.13 as it is a simple fix and it would be nice to have it released in 4.13.1.0.
@andrijapanicsb<https://github.com/andrijapanicsb> I ported this changes on commit e8ec868<CLDIN@e8ec868>, located in branch fix_revert_rbd_snapshot_on_master<https://github.com/PCextreme/cloudstack/tree/fix_revert_rbd_snapshot_on_master>. The new branch/commit is based on CloudStack master branch and can be tested on a ACS environment running '4.14.0.0-SNAPSHOT'.
Would that be ok for you? I can also generate the packages for a distro and make them available here.
Thanks for the review/test :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#3605?email_source=notifications&email_token=AK5EN3NKHUL4F6X3IQVXI5DQSKWNFA5CNFSM4IZ2YD42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDGJHOQ#issuecomment-550278074>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AK5EN3PMHV5AXWFYPSTK7UDQSKWNFANCNFSM4IZ2YD4Q>.
Andrija Panic
Cloud Architect
s: +44 20 3603 0540 | m: +381 69 272 3690
e: andrija.panic@shapeblue.com | w: www.shapeblue.com | t: @shapeblue
a: Amadeus House, Floral Street, London WC2E 9DPUK
Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error.
Find out more about ShapeBlue and our range of CloudStack related services:
IaaS Cloud Design & Build | CSForge - rapid IaaS deployment framework
CloudStack Consulting | CloudStack Software Engineering
CloudStack Infrastructure Support | CloudStack Bootcamp Training Courses
|
|
Cool, @andrijapanicsb! |
|
@andrijapanicsb you can test by using rpms on internal staging server and install/force-install only the cloudstack-agent rpm where the change is, against a 4.14-SNAPSHOT/master based env. |
|
LGTM tested manually restoring root and data volume snapshots - works fine! |
|
That is great @andrijapanicsb, thanks for the feedback :-) |
5 participants
Description
Fixes issue #3590 by using the last element on the array from the snapshot "path" String for retrieving the snapshot id. Additionally, it uses the
volumePathas the volume id which should always be the correct value. The error raised on issue #3590 was related to the wrong use of variable "path" where in some cases had a different set of substrings.The proposed change has been tested and evaluated. The values used for openning the RBD connection and executing the rollback were stable on the tests. Runned rollback on multiple snapshots and could start the VM with the content matching the ROOT reverted snapshot.
Types of changes