New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mon: include device class in tree view; hide shadow hierarchy #16016

Merged
merged 6 commits into from Jul 10, 2017

Conversation

Projects
None yet
2 participants
@liewegas
Member

liewegas commented Jun 29, 2017

  • Do not show each per-class variation of the hieararchy
  • Do include a CLASS column in the tree view
@liewegas

This comment has been minimized.

Member

liewegas commented Jul 5, 2017

retest this please

@liewegas liewegas modified the milestone: luminous Jul 6, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 6, 2017

retest this please

liewegas added some commits Jun 29, 2017

crush: hide device class shadow roots from tree dumper
Signed-off-by: Sage Weil <sage@redhat.com>
mon: include class in 'osd df [tree]' output
Signed-off-by: Sage Weil <sage@redhat.com>
mon: include device class in 'osd tree' output
Signed-off-by: Sage Weil <sage@redhat.com>
mon: shorten 'PRIMARY-AFFINITY' -> 'PRI-AFF' column header
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas

This comment has been minimized.

Member

liewegas commented Jul 9, 2017

retest this please

@liewegas liewegas requested a review from tchaikov Jul 10, 2017

@@ -4,16 +4,16 @@
$ osdmaptool --tree=plain om
osdmaptool: osdmap file 'om'
ID WEIGHT TYPE NAME UP/DOWN REWEIGHT PRIMARY-AFFINITY
-1 3.00000 root default

This comment has been minimized.

@tchaikov

tchaikov Jul 10, 2017

Contributor
/home/jenkins-build/build/workspace/ceph-pull-requests/src/test/cli/osdmaptool/tree.t: failed
--- /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/cli/osdmaptool/tree.t
+++ /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/cli/osdmaptool/tree.t.err
@@ -4,13 +4,13 @@
 
   $ osdmaptool --tree=plain om
   osdmaptool: osdmap file 'om'
-  ID CLASS WEIGHT  TYPE NAME              UP/DOWN REWEIGHT PRI-AFF
-  -1       3.00000 root default
-  -3       3.00000     rack localrack
-  -2       3.00000         host localhost
-   0       1.00000             osd.0          DNE        0
-   1       1.00000             osd.1          DNE        0
-   2       1.00000             osd.2          DNE        0
+  ID CLASS WEIGHT  TYPE NAME              UP/DOWN REWEIGHT PRI-AFF 
+  -1       3.00000 root default                                    
+  -3       3.00000     rack localrack                              
+  -2       3.00000         host localhost                          
+   0       1.00000             osd.0          DNE        0         
+   1       1.00000             osd.1          DNE        0         
+   2       1.00000             osd.2          DNE        0         
 
   $ osdmaptool --tree=json om
   osdmaptool: osdmap file 'om'

i don't think we can remove the trailing spaces here.

const char *c = crush->get_item_class(qi.id);
if (!c)
c = "";
f->dump_string("device_class", c);

This comment has been minimized.

@tchaikov

tchaikov Jul 10, 2017

Contributor
/home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mon/osd-crush.sh:229: TEST_crush_tree:  ceph osd crush tree --format=xml
/home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mon/osd-crush.sh:230: TEST_crush_tree:  xmlstarlet val -e -r /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mon/osd-crush-tree.rng -
-:1.166: Did not expect element device_class there
-:1.166: Did not expect element name there
-:1.166: Did not expect text in element  TODO  content
-:1.166: Did not expect element type there
-:1.166: Did not expect text in element  TODO  content
-:1.166: Did not expect element type_id there
-:1.166: Did not expect text in element  TODO  content
-:1.166: Did not expect element items there
- - invalid
/home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mon/osd-crush.sh:230: TEST_crush_tree:  return 1

we also need to update the rng file accordingly

diff --git a/src/test/mon/osd-crush-tree.rng b/src/test/mon/osd-crush-tree.rng
index d050ed2052..14ea5f35a0 100644
--- a/src/test/mon/osd-crush-tree.rng
+++ b/src/test/mon/osd-crush-tree.rng
@@ -14,6 +14,9 @@
       <element name="id">
         <text/>
       </element>
+      <element name="device_class">
+        <text/>
+      </element>
       <element name="name">
         <text/>
       </element>
@@ -36,6 +39,9 @@
       <element name="id">
         <text/>
       </element>
+      <element name="device_class">
+        <text/>
+      </element>
       <element name="name">
         <text/>
       </element>
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 10, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 10, 2017

once the "make check" failures are fixed, lgtm.

liewegas added some commits Jul 10, 2017

test/cli/osdmaptool/tree.t: fix
Signed-off-by: Sage Weil <sage@redhat.com>
test/mon/osd-crush-tree.rng: fix expected xml schema for osd tree
Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas merged commit 1247872 into ceph:master Jul 10, 2017

3 of 4 checks passed

make check make check failed
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check (arm64) make check succeeded
Details

@liewegas liewegas deleted the liewegas:wip-osd-df branch Jul 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment