Merge pull request #23754 from p-na/down-osd-raises-error

mgr/dashboard: Fix OSD down error display

Reviewed-by: Volker Theile <vtheile@suse.com>
Reviewed-by: Stephan Müller <smueller@suse.com>
Reviewed-by: Lenz Grimmer <lgrimmer@suse.com>

src/pybind/mgr/dashboard/controllers/osd.py

@@ -12,20 +12,24 @@ from ..tools import str_to_bool
 class Osd(RESTController):
     def list(self):
         osds = self.get_osd_map()
+
         # Extending by osd stats information
         for s in mgr.get('osd_stats')['osd_stats']:
             osds[str(s['osd'])].update({'osd_stats': s})
+
         # Extending by osd node information
         nodes = mgr.get('osd_map_tree')['nodes']
         osd_tree = [(str(o['id']), o) for o in nodes if o['id'] >= 0]
         for o in osd_tree:
             osds[o[0]].update({'tree': o[1]})
+
         # Extending by osd parent node information
         hosts = [(h['name'], h) for h in nodes if h['id'] < 0]
         for h in hosts:
             for o_id in h[1]['children']:
                 if o_id >= 0:
                     osds[str(o_id)]['host'] = h[1]
+
         # Extending by osd histogram data
         for o_id in osds:
             o = osds[o_id]
@@ -39,6 +43,7 @@ class Osd(RESTController):
             # Gauge stats
             for s in ['osd.numpg', 'osd.stat_bytes', 'osd.stat_bytes_used']:
                 o['stats'][s.split('.')[1]] = mgr.get_latest('osd', osd_spec, s)
+
         return list(osds.values())
 
     def get_osd_map(self):
@@ -50,7 +55,21 @@ class Osd(RESTController):
 
     @handle_send_command_error('osd')
     def get(self, svc_id):
-        histogram = CephService.send_command('osd', srv_spec=svc_id, prefix='perf histogram dump')
+        """
+        Returns collected data about an OSD.
+
+        :return: Returns the requested data. The `histogram` key man contain a
+                 string with an error that occurred when the OSD is down.
+        """
+        try:
+            histogram = CephService.send_command('osd', srv_spec=svc_id,
+                                                 prefix='perf histogram dump')
+        except SendCommandError as e:
+            if 'osd down' in e.message:
+                histogram = e.message
+            else:
+                raise
+
         return {
             'osd_map': self.get_osd_map()[svc_id],
             'osd_metadata': mgr.get_metadata('osd', svc_id),

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-details/osd-details.component.html

@@ -17,9 +17,10 @@
     </cd-table-performance-counter>
   </tab>
   <tab heading="Histogram">
-    <h3 *ngIf="osd.loaded && osd.histogram_failed">
-      Histogram not available -> <span class="text-warning">{{ osd.histogram_failed }}</span>
-    </h3>
+    <cd-warning-panel i18n *ngIf="osd.loaded && osd.histogram_failed">
+      Histogram not available: {{ osd.histogram_failed }}
+    </cd-warning-panel>
+
     <div class="row" *ngIf="osd.loaded && osd.details.histogram">
       <div class="col-md-6">
         <h4>Writes</h4>

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-details/osd-details.component.spec.ts

@@ -1,9 +1,13 @@
 import { HttpClientModule } from '@angular/common/http';
+import { DebugElement } from '@angular/core';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 
+import { of } from 'rxjs';
+
 import { TabsModule } from 'ngx-bootstrap';
 
 import { configureTestBed } from '../../../../../testing/unit-test-helper';
+import { OsdService } from '../../../../shared/api/osd.service';
 import { DataTableModule } from '../../../../shared/datatable/datatable.module';
 import { CdTableSelection } from '../../../../shared/models/cd-table-selection';
 import { SharedModule } from '../../../../shared/shared.module';
@@ -14,6 +18,9 @@ import { OsdDetailsComponent } from './osd-details.component';
 describe('OsdDetailsComponent', () => {
   let component: OsdDetailsComponent;
   let fixture: ComponentFixture<OsdDetailsComponent>;
+  let debugElement: DebugElement;
+  let osdService: OsdService;
+  let getDetailsSpy;
 
   configureTestBed({
     imports: [
@@ -23,7 +30,8 @@ describe('OsdDetailsComponent', () => {
       DataTableModule,
       SharedModule
     ],
-    declarations: [OsdDetailsComponent, OsdPerformanceHistogramComponent]
+    declarations: [OsdDetailsComponent, OsdPerformanceHistogramComponent],
+    providers: [OsdService]
   });
 
   beforeEach(() => {
@@ -31,6 +39,10 @@ describe('OsdDetailsComponent', () => {
     component = fixture.componentInstance;
 
     component.selection = new CdTableSelection();
+    debugElement = fixture.debugElement;
+    osdService = debugElement.injector.get(OsdService);
+
+    getDetailsSpy = spyOn(osdService, 'getDetails');
 
     fixture.detectChanges();
   });
@@ -38,4 +50,30 @@ describe('OsdDetailsComponent', () => {
   it('should create', () => {
     expect(component).toBeTruthy();
   });
+
+  it('should fail creating a histogram', () => {
+    const detailDataWithoutHistogram = {
+      osd_map: {},
+      osd_metadata: {},
+      histogram: 'osd down'
+    };
+    getDetailsSpy.and.returnValue(of(detailDataWithoutHistogram));
+    component.osd = { tree: { id: 0 } };
+    component.refresh();
+    expect(getDetailsSpy).toHaveBeenCalled();
+    expect(component.osd.histogram_failed).toBe('osd down');
+  });
+
+  it('should succeed creating a histogram', () => {
+    const detailDataWithHistogram = {
+      osd_map: {},
+      osd_metdata: {},
+      histogram: {}
+    };
+    getDetailsSpy.and.returnValue(of(detailDataWithHistogram));
+    component.osd = { tree: { id: 0 } };
+    component.refresh();
+    expect(getDetailsSpy).toHaveBeenCalled();
+    expect(component.osd.histogram_failed).toBe('');
+  });
 });

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-details/osd-details.component.ts

@@ -34,6 +34,7 @@ export class OsdDetailsComponent implements OnChanges {
   refresh() {
     this.osdService.getDetails(this.osd.tree.id).subscribe((data: any) => {
       this.osd.details = data;
+      this.osd.histogram_failed = '';
       if (!_.isObject(data.histogram)) {
         this.osd.histogram_failed = data.histogram;
         this.osd.details.histogram = undefined;

src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/osd/osd-list/osd-list.component.html

@@ -12,7 +12,7 @@
               type="button"
               class="btn btn-sm btn-primary dropdown-toggle tc_scrub_toggle"
               [ngClass]="{disabled: !tableComponent.selection.hasSelection}">
-        <ng-container i18n> Perform Task</ng-container>
+        <ng-container i18n>Perform Task </ng-container>
         <span class="caret"></span>
       </button>
       <ul *dropdownMenu
@@ -49,14 +49,11 @@
   </cd-osd-details>
 </cd-table>
 
-<ng-template #statusColor
-             let-value="value">
+<ng-template #statusColor let-value="value">
   <span *ngFor="let state of value; last as last">
-    <span [class.text-success]="'up' === state || 'in' === state"
-          [class.text-warning]="'down' === state || 'out' === state">
-      {{ state }}</span>
-    <span *ngIf="!last">, </span>
-    <!-- Has to be on the same line to prevent a space between state and comma. -->
+    <span class="label"
+           [ngClass]="{'label-success': ['in', 'up'].includes(state), 'label-danger': ['down', 'out'].includes(state)}">{{ state }}</span>
+    <span *ngIf="!last">&nbsp;</span>
   </span>
 </ng-template>
 

src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table/table.component.scss

@@ -185,6 +185,9 @@
       padding-bottom: 5px;
     }
     .datatable-body-row {
+      .label {
+        font-size: 0.9em;
+      }
       &.clickable:hover .datatable-row-group {
         background-color: $color-table-hover-row;
         transition-property: background;