HPE3PAR: Failing to clone a volume having children

[Rajat Dhasmana]

[Impact]

The customer faced issue when they are using nova with 3par storage.
they can't delete volume if there is children and it is attached to vm.

it makes sense they can't delete it as the children has attachment but nova should expose proper error when trying deletion.

[Test Case]

The customer has the required hardware for testing this.

Volume->Snapshot->Volume2
Volume2 is attached to some VM.
and can't delete Volume without proper error msg.

[Where problems could occur]
This is related to hpe3par storage.
snapshot handling could be issue with this patch.
deleting volume could be issue with this patch.

[Others]

Original Desc

When we try to delete a snapshot, we flatten it's dependent volumes by copying them to a new volume and deleting the original one.
We fail to copy the volume when it has children and it is not handled in the code.

: hpe3parclient.exceptions.HTTPConflict: Conflict (HTTP 409) 32 - volume has a child

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/cinder/+/862679

[Raghavendra Tilay (raghavendrat)]

Customer created volume and snapshots as below:

v1
|
`-- s1
    |
    `-- v2
        |
        `-- s2

User sends command to delete snapshot s1.
The 3par cinder code tries to convert volume v2 to base volume.
In the process, it renames from osv-<id> to omv-<id>.

Although v2 has a child s2, client.copyVolume() is able to convert v2 to new base volume omv-<id>.
And s2 remains child of old volume osv-<id>.
So, the 3parclient returns [1] as warning and does not throw as exception during copy of v2.

[1] Conflict (HTTP 409) 32 - volume has a child

Later during delete of v2 osv-<id> volume, it is observed that v2 has a child s2.
At this point, the above [1] is thrown as exception by client.deleteVolume().

Updated the code to catch exception [1] and delete snapshot s2 of old volume v2 osv-<id>.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/cinder/+/862679
Committed: https://opendev.org/openstack/cinder/commit/dfd8f99743a29220ca3face5fdf00a1a6071cf48
Submitter: "Zuul (22348)"
Branch: master

commit dfd8f99743a29220ca3face5fdf00a1a6071cf48
Author: Rajat Dhasmana <email address hidden>
Date: Wed Oct 26 09:55:43 2022 +0000

    3PAR: Error out if vol cannot be converted to base

    Consider volume and snapshots as below:

    v1
    |
    `-- s1
        |
        `-- v2
            |
            `-- s2

    User initiated deletion of snapshot s1.
    It failed with some vague message.

    Initially, it was suspected that ...
    While copying volume v2 (sometimes an intermediate step to break
    volume dependency), we send a request to clone the volume v2 to new
    base volume; and the exception [1] isn't handled properly.

    [1] Conflict (HTTP 409) 32 - volume has a child

    However, on further investigation it was found that ...
    after a new volume v2 (omv-<id>) is created and
    when we try to delete old volume v2 (osv-<id>),
    at this point the exception [1] is thrown as error.

    This is now handled gracefully. Appropriate error is thrown
    if the volume (v2) has snapshot (s2).

    Co-Authored-By: raghavendrat <email address hidden>
    Closes-Bug: #1994521
    Change-Id: I5e7fb425c92cdf8c16d5a86a58ca1a52421543d7

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/2023.1
Review: https://review.opendev.org/c/openstack/cinder/+/882782

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 23.0.0.0rc1 release candidate.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/zed
Review: https://review.opendev.org/c/openstack/cinder/+/907261

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/cinder/+/907258

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/yoga
Review: https://review.opendev.org/c/openstack/cinder/+/907258
Reason: stable/yoga branch of openstack/cinder is about to be deleted. To be able to do that, all open patches need to be abandoned. Please cherry pick the patch to unmaintained/yoga if you want to further work on this patch.

[Dariusz Gadomski (dgadomski)]

hey Seyeong. Can you add the SRU template to the bug please? Thanks!

[Seyeong Kim (seyeongkim)]

Hello @dgadomski, sorry for making confusion.
For now, upstream review is in progress for zed, 2023.1 ( for yoga is rejected )
I wasn't sure if I can go forward with this situation so I didn't updated description yet.

But do you think it is possible to SRU in this situation?

Either way, I'll update description today for future work.

Thanks a lot.

[Mauricio Faria de Oliveira (mfo)]

Seyeong, Dariusz (subscribing you as you're the uploader),

Could you please confirm whether the Openstack Engineering team
is OK with the proposed changes, and that they are not upstream?

From comment #12 it seems unclear whether the situation could
move forward, and the SRU template was updated in case it was;
but there is no other comment indicating how things turned out,
before the upload happened.

Thanks!

[Seyeong Kim (seyeongkim)]

@mfo
Thanks I had a discussion with dariusz and he mentioned it could be possible and he uploaded.

After that, he found out below comment.

https://bugs.launchpad.net/cinder/+bug/1988942/comments/23

Do I need to contact openstack team directly about this?

Thanks.

[Dariusz Gadomski (dgadomski)]

My understanding is as follows: it has been rejected upstream for yoga, because it was too close to EOL. But according to [1] it has been merged upstream for later releases. IMO the presence in later release is a good justification to backport the fix to jammy.

What's your take on this Mauricio?

[1] https://review.opendev.org/c/openstack/cinder/+/882782

[Mauricio Faria de Oliveira (mfo)]

Hey Seyeong, Dariusz,

Thanks for the clarifications!

Yes, this needs coordination with the Openstack Engineering team
not only for the other changes they plan to combine in an upload,
but also regarding the upstream acceptance situation for this in
the different branches.

Apparently, they still haven't confirmed that, so I have reached
out to James (as I'm involved in one of the three bugs) to offer
help and ask the question whether they are OK with these changes.

Thanks!

[Seyeong Kim (seyeongkim)]

@mfo @james-page

Thanks all!

[Mauricio Faria de Oliveira (mfo)]

Hi Seyeong,

Thanks for the patches!

In the future, could you please add DEP-3 headers [1] to them?
(usually just 'Bug-Ubuntu:'/'Origin:' are needed on git patches)

Also, it's a good practice to name the patch files with the bug
prefix (this is style, not requirement, but it helps when there
is multiple bugs per upload, and with maintenance over time).

I have handled these in this upload, as the changes are minor.
Thanks again!

[1] https://dep-team.pages.debian.net/deps/dep3/

$ head -n2 debian/patches/lp1994521-*

==> debian/patches/lp1994521-0001-HPE-3PAR-Fix-umanaged-volumes-snapshots-missing.patch <==
Bug-Ubuntu: http://bugs.launchpad.net/bugs/1994521
Origin: https://review.opendev.org/c/openstack/cinder/+/907257

==> debian/patches/lp1994521-0002-3PAR-Error-out-if-vol-cannot-be-converted-to-base.patch <==
Bug-Ubuntu: http://bugs.launchpad.net/bugs/1994521
Origin: https://review.opendev.org/c/openstack/cinder/+/907258

[Mauricio Faria de Oliveira (mfo)]

Uploaded to Jammy.

This upload has been coordinated with the Openstack Engineering team (~james-page) for bugs 1987663, 1988942, and 1994521.
Test PPA and build-time unit test summary in https://bugs.launchpad.net/ubuntu/+source/cinder/+bug/1987663/comments/16

[Mauricio Faria de Oliveira (mfo)]

Cloud Archive:

Caracal is addressed with Noble
Bobcat is addressed with Mantic
* Antelope is _not_ addressed with Lunar (no longer supported), needs patching in UCA [Triaged]
Zed is no longer supported (2024/04)
Yoga is addressed with Jammy

[Timo Aaltonen (tjaalton)]

Hello Rajat, or anyone else affected,

Accepted cinder into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cinder/2:20.3.1-0ubuntu1.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/zed
Review: https://review.opendev.org/c/openstack/cinder/+/907261
Reason: stable/zed branch of openstack/cinder is about to be deleted. To be able to do that, all open patches need to be abandoned. Please cherry pick the patch to unmaintained/zed if you want to further work on this patch.