mgr/dashboard: work with v1 RBD images by epuertat · Pull Request #35007 · ceph/ceph

epuertat · 2020-05-11T18:57:59Z

Add support for RBD Image Format v1:

This format lacks ID field, required for dashboard. Instead, RBD image block_name_prefix is used as unique ID (together with pool id)
Additionally, image_format is now exposed.
In the front-end side:
- Copy action on a v1 image will cause the image to be copied to v2 format.
- List doesn't allow Move to Trash on v1 images,
- Details section now shows image_format for images,
- Edit Form disables flags not supported for v1 (deep-flatten, layering, exclusive-lock).
- Protect does not work on v1 images or v2 images created from v1 ones.

After: #35025
Fixes: https://tracker.ceph.com/issues/36354
Signed-off-by: Ernesto Puerta epuertat@redhat.com

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard backend
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

dillaman

The other thing to potentially look-out for is that you (1) shouldn't be able to protect snapshots if the image is v1 (or doesn't have the LAYERING feature), (2) nor clone.

dillaman · 2020-05-11T20:08:34Z

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/rbd-list/rbd-list.component.ts

-        !selection.hasSingleSelection || selection.first().cdExecuting,
+        !selection.hasSingleSelection ||
+        selection.first().cdExecuting ||
+        selection.first().image_format === RBDImageFormat.V1,


Nit: you should be able to copy a v1 image (to a v2 image)

Then the error came from within dashboard implementation. Will check for exceptions there.

I think it fails because it tries to create the destination image as v1 format: if the format is not explicitly specified the source image format is used [1].

@dillman I suppose we should change this to use v2 format if it is not specified via image options?

Another problem with copy I see here is that although we allow to specify the destination image format via image options in C/C++ API, we do not provide such possibility neither for python nor for CLI. So, right now the dashboard can't force the destination image v2 format in any way. Do we want to improve this or it is not worth doing taking the fromat v1 is depricated? (I vote for the later)

[1] https://github.com/ceph/ceph/blob/master/src/librbd/internal.cc#L1126

Ouch -- that's no good. I am going to submit another commit to fix that copy defaulting to the source format since we really should never be creating v1 images any more.

Thanks for the hint @trociny. So if @dillaman is going to fix this, I'll re-enable copy (it was kinda-gracefully failing before).

@epuertat I've opened a PR here: #35025

dillaman · 2020-05-11T20:18:16Z

src/pybind/mgr/dashboard/services/rbd.py

            stat['name'] = image_name
-            stat['id'] = img.id()
+            stat['id'], stat['image_format'] = (img.id(), 2) if \
+                not img.old_format() else (stat['block_name_prefix'], 1)


Nit: Technically, v1 images don't have an id. So long as this isn't being used as a key for something and is mearly for display purposes, I'd say just drop the rb. prefix from the v1 block name prefix.

So, cannot that be expected to be a unique index? The Dashboard front-end table needs a unique id to work. If not the block_name_prefix, what else could do the trick?

It's guaranteed to be unique per pool -- it just is meaningless to the user. If you just need something for a unique index for internal structures, definitely can use it.

Great, thanks! For the ID v1 I'll concat pool_id and block_name_prefix then (Dashboard RBD images provides an aggregate view across pools, so IDs need to be globally unique).

That applies for v2 images as well -- ids are only guaranteed to be unique within the same pool + namespace.

Done, concatenating pool ID and image ID:

v1: id: "2.rb.0.104e.6b8b4567"

v2: id: "2.104c2fe7ebe3"

That applies for v2 images as well -- ids are only guaranteed to be unique within the same pool + namespace.

Do we want to insert the namespace string into the id string for v2 images as well?

Do we want to insert the namespace string into the id string for v2 images as well?

Indeed -- good catch.

Thanks @bk201 ! Done.

epuertat · 2020-06-01T17:24:48Z

@dillaman @trociny could you please check that the following operations with v1 and v2 & outcomes match your expectations from rbd CLI? Thanks!

Action	v1	v2
Create	❎ Only from CLI	🆗
Edit	🆗 Flags disabled	🆗
Copy	🆗 Migrates to v2	🆗
Flatten	❎ Disabled	🆗
Delete	🆗	🆗
Move to trash	❎ Disabled	🆗
Snapshot > Create	🆗	🆗
Snapshot > Rename	🆗	🆗
Snapshot > Delete	🆗	🆗
Snapshot > Protect	❎ Failure (similar to v2 without flags set)	🆗
Snapshot > Copy	🆗 Migrates a v1 snapshot to a v2 image	🆗
Snapshot > Rollback	🆗	🆗

Edit v1

All flags are disabled.

trociny

LGTM

dillaman

lgtm -- thx!

bk201 · 2020-06-02T03:56:48Z

jenkins test dashboard backend

epuertat · 2020-06-02T09:24:18Z

jenkins test dashboard backend

epuertat · 2020-06-02T09:27:21Z

jenkins test dashboard backend

Seems the trigger-phrase is no longer working. @djgalloway could this behaviour be related to ceph/ceph-build#1575?

djgalloway · 2020-06-02T11:53:41Z

jenkins test dashboard backend

Seems the trigger-phrase is no longer working. @djgalloway could this behaviour be related to ceph/ceph-build#1575?

ceph/ceph-build#1580 should fix this

epuertat · 2020-06-02T16:10:15Z

jenkins test dashboard backend

epuertat · 2020-06-02T19:37:52Z

API tests caught the following error: 🎉

RBD Trash ops don't use the regular image_spec (pool + namespace + image_name) but image_id_spec (pool + namespace + image_id).

Fixed now.

Add support for RBD Image Format v1: - This format lacks ID field, required for dashboard. Instead, RBD image `block_name_prefix` is used as unique ID (together with pool id and namespace) - Additionally, `image_format` is now exposed. - In the front-end side: - Copy action on a v1 image will cause the image to be copied to v2 format. - List doesn't allow Move to Trash on v1 images, - Details section now shows `image_format` for images, - Edit Form disables flags not supported for v1 (`deep-flatten`, `layering`, `exclusive-lock`). - Protect does not work on v1 images or v2 images created from v1 ones. Fixes: https://tracker.ceph.com/issues/36354 Signed-off-by: Ernesto Puerta <epuertat@redhat.com>

callithea · 2020-06-08T07:58:37Z

pkg build
QA run [9 / 9 passed]

callithea

lgtm

epuertat requested a review from a team as a code owner May 11, 2020 18:57

epuertat added dashboard bug-fix labels May 11, 2020

epuertat self-assigned this May 11, 2020

epuertat added the rbd label May 11, 2020

epuertat changed the title ~~mgr/dashboard: work with RBD images v1~~ mgr/dashboard: work with v1 RBD images May 11, 2020

epuertat requested a review from dillaman May 11, 2020 19:01

dillaman reviewed May 11, 2020

View reviewed changes

LenzGr requested a review from trociny May 12, 2020 09:05

epuertat force-pushed the fix-36354-master branch 4 times, most recently from b532a51 to 7dc33b4 Compare June 1, 2020 16:50

epuertat added the needs-review label Jun 1, 2020

trociny approved these changes Jun 1, 2020

View reviewed changes

dillaman approved these changes Jun 1, 2020

View reviewed changes

epuertat force-pushed the fix-36354-master branch from 7dc33b4 to 1b71888 Compare June 1, 2020 18:07

bk201 approved these changes Jun 2, 2020

View reviewed changes

bk201 requested a review from a team June 2, 2020 04:45

epuertat mentioned this pull request Jun 2, 2020

ceph-dashboard: Enable ansicolor plugin ceph/ceph-build#1577

Merged

epuertat added needs-qa and removed needs-review labels Jun 2, 2020

epuertat force-pushed the fix-36354-master branch from 1b71888 to 9dc154b Compare June 2, 2020 11:59

Devp00l approved these changes Jun 2, 2020

View reviewed changes

epuertat force-pushed the fix-36354-master branch from 9dc154b to 716fc0e Compare June 2, 2020 19:24

epuertat requested a review from a team June 2, 2020 19:24

epuertat force-pushed the fix-36354-master branch from 716fc0e to 2444cb5 Compare June 2, 2020 19:36

epuertat removed the needs-qa label Jun 3, 2020

epuertat force-pushed the fix-36354-master branch from 2444cb5 to 783b51a Compare June 3, 2020 18:43

epuertat added the needs-qa label Jun 3, 2020

epuertat force-pushed the fix-36354-master branch from 783b51a to 119851e Compare June 4, 2020 14:55

callithea added the wip-laura-testing label Jun 8, 2020

callithea added TESTED and removed needs-qa wip-laura-testing labels Jun 8, 2020

callithea approved these changes Jun 8, 2020

View reviewed changes

LenzGr merged commit 11ddc9c into ceph:master Jun 10, 2020

epuertat mentioned this pull request Jun 22, 2020

octopus: mgr/dashboard: work with v1 RBD images #35711

Merged

epuertat deleted the fix-36354-master branch June 22, 2020 19:13

epuertat mentioned this pull request Jun 22, 2020

nautilus: mgr/dashboard: work with v1 RBD images #35712

Merged

Conversation

epuertat commented May 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

dillaman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

epuertat commented Jun 1, 2020

Edit v1

Uh oh!

trociny left a comment

Choose a reason for hiding this comment

Uh oh!

dillaman left a comment

Choose a reason for hiding this comment

Uh oh!

bk201 commented Jun 2, 2020

Uh oh!

epuertat commented Jun 2, 2020

Uh oh!

epuertat commented Jun 2, 2020

Uh oh!

djgalloway commented Jun 2, 2020

Uh oh!

epuertat commented Jun 2, 2020

Uh oh!

epuertat commented Jun 2, 2020

Uh oh!

callithea commented Jun 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

callithea left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

epuertat commented May 11, 2020 •

edited

Loading

callithea commented Jun 8, 2020 •

edited

Loading