Merge pull request #30208 from ricardoasmarques/support-iscsi-controls-types

mgr/dashboard: Controls UI inputs based on "type"

Reviewed-by: Tiago Melo <tmelo@suse.com>
Reviewed-by: Volker Theile <vtheile@suse.com>

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

@@ -19,7 +19,7 @@ from ..services.iscsi_cli import IscsiGatewaysConfig
 from ..services.rbd import format_bitmask
 from ..services.tcmu_service import TcmuService
 from ..exceptions import DashboardException
-from ..tools import TaskManager
+from ..tools import str_to_bool, TaskManager
 
 
 @UiApiController('/iscsi', Scope.ISCSI)
@@ -75,7 +75,29 @@ class IscsiUi(BaseController):
     @Endpoint()
     @ReadPermission
     def settings(self):
-        return IscsiClient.instance().get_settings()
+        settings = IscsiClient.instance().get_settings()
+        if 'target_controls_limits' in settings:
+            target_default_controls = settings['target_default_controls']
+            for ctrl_k, ctrl_v in target_default_controls.items():
+                limits = settings['target_controls_limits'].get(ctrl_k, {})
+                if 'type' not in limits:
+                    # default
+                    limits['type'] = 'int'
+                    # backward compatibility
+                    if target_default_controls[ctrl_k] in ['Yes', 'No']:
+                        limits['type'] = 'bool'
+                        target_default_controls[ctrl_k] = str_to_bool(ctrl_v)
+                settings['target_controls_limits'][ctrl_k] = limits
+        if 'disk_controls_limits' in settings:
+            for backstore, disk_controls_limits in settings['disk_controls_limits'].items():
+                disk_default_controls = settings['disk_default_controls'][backstore]
+                for ctrl_k, ctrl_v in disk_default_controls.items():
+                    limits = disk_controls_limits.get(ctrl_k, {})
+                    if 'type' not in limits:
+                        # default
+                        limits['type'] = 'int'
+                    settings['disk_controls_limits'][backstore][ctrl_k] = limits
+        return settings
 
     @Endpoint()
     @ReadPermission
@@ -747,9 +769,6 @@ class IscsiTarget(RESTController):
             groups.append(group)
         groups = IscsiTarget._sorted_groups(groups)
         target_controls = target_config['controls']
-        for key, value in target_controls.items():
-            if isinstance(value, bool):
-                target_controls[key] = 'Yes' if value else 'No'
         acl_enabled = target_config['acl_enabled']
         target = {
             'target_iqn': target_iqn,

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/block.module.ts

@@ -15,6 +15,7 @@ import { TooltipModule } from 'ngx-bootstrap/tooltip';
 import { ActionLabels, URLVerbs } from '../../shared/constants/app.constants';
 import { FeatureTogglesGuardService } from '../../shared/services/feature-toggles-guard.service';
 import { SharedModule } from '../../shared/shared.module';
+import { IscsiSettingComponent } from './iscsi-setting/iscsi-setting.component';
 import { IscsiTabsComponent } from './iscsi-tabs/iscsi-tabs.component';
 import { IscsiTargetDetailsComponent } from './iscsi-target-details/iscsi-target-details.component';
 import { IscsiTargetDiscoveryModalComponent } from './iscsi-target-discovery-modal/iscsi-target-discovery-modal.component';
@@ -69,6 +70,7 @@ import { RbdTrashRestoreModalComponent } from './rbd-trash-restore-modal/rbd-tra
   declarations: [
     RbdListComponent,
     IscsiComponent,
+    IscsiSettingComponent,
     IscsiTabsComponent,
     IscsiTargetListComponent,
     RbdDetailsComponent,

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-setting/iscsi-setting.component.html

@@ -0,0 +1,57 @@
+<div class="form-group"
+     [formGroup]="settingsForm">
+  <label class="col-form-label"
+         for="{{ setting }}">{{ setting }}</label>
+  <select id="{{ setting }}"
+          name="{{ setting }}"
+          *ngIf="limits['type'] === 'enum'"
+          class="form-control custom-select"
+          [formControlName]="setting">
+    <option [ngValue]="null"></option>
+    <option *ngFor="let opt of limits['values']"
+            [ngValue]="opt">{{ opt }}</option>
+  </select>
+
+  <span *ngIf="limits['type'] !== 'enum'">
+    <input type="number"
+           *ngIf="limits['type'] === 'int'"
+           class="form-control"
+           [formControlName]="setting">
+
+    <input type="text"
+           *ngIf="limits['type'] === 'str'"
+           class="form-control"
+           [formControlName]="setting">
+
+    <ng-container *ngIf="limits['type'] === 'bool'">
+      <br>
+      <div class="custom-control custom-radio custom-control-inline">
+        <input type="radio"
+               [id]="setting + 'True'"
+               [value]="true"
+               [formControlName]="setting"
+               class="custom-control-input">
+        <label class="custom-control-label"
+               [for]="setting + 'True'">Yes</label>
+      </div>
+      <div class="custom-control custom-radio custom-control-inline">
+        <input type="radio"
+               [id]="setting + 'False'"
+               [value]="false"
+               class="custom-control-input"
+               [formControlName]="setting">
+        <label class="custom-control-label"
+               [for]="setting + 'False'">No</label>
+      </div>
+    </ng-container>
+  </span>
+
+  <span class="invalid-feedback"
+        *ngIf="settingsForm.showError(setting, formDir, 'min')">
+    <ng-container i18n>Must be greater than or equal to {{ limits['min'] }}.</ng-container>
+  </span>
+  <span class="invalid-feedback"
+        *ngIf="settingsForm.showError(setting, formDir, 'max')">
+    <ng-container i18n>Must be less than or equal to {{ limits['max'] }}.</ng-container>
+  </span>
+</div>

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-setting/iscsi-setting.component.spec.ts

@@ -0,0 +1,37 @@
+import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { FormControl, NgForm, ReactiveFormsModule } from '@angular/forms';
+
+import { configureTestBed } from '../../../../testing/unit-test-helper';
+import { CdFormGroup } from '../../../shared/forms/cd-form-group';
+import { SharedModule } from '../../../shared/shared.module';
+import { IscsiSettingComponent } from './iscsi-setting.component';
+
+describe('IscsiSettingComponent', () => {
+  let component: IscsiSettingComponent;
+  let fixture: ComponentFixture<IscsiSettingComponent>;
+
+  configureTestBed({
+    imports: [SharedModule, ReactiveFormsModule],
+    declarations: [IscsiSettingComponent]
+  });
+
+  beforeEach(() => {
+    fixture = TestBed.createComponent(IscsiSettingComponent);
+    component = fixture.componentInstance;
+    component.settingsForm = new CdFormGroup({
+      max_data_area_mb: new FormControl()
+    });
+    component.formDir = new NgForm([], []);
+    component.setting = 'max_data_area_mb';
+    component.limits = {
+      type: 'int',
+      min: 1,
+      max: 2048
+    };
+    fixture.detectChanges();
+  });
+
+  it('should create', () => {
+    expect(component).toBeTruthy();
+  });
+});

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-setting/iscsi-setting.component.ts

@@ -0,0 +1,31 @@
+import { Component, Input, OnInit } from '@angular/core';
+import { NgForm, Validators } from '@angular/forms';
+
+import { CdFormGroup } from '../../../shared/forms/cd-form-group';
+
+@Component({
+  selector: 'cd-iscsi-setting',
+  templateUrl: './iscsi-setting.component.html',
+  styleUrls: ['./iscsi-setting.component.scss']
+})
+export class IscsiSettingComponent implements OnInit {
+  @Input()
+  settingsForm: CdFormGroup;
+  @Input()
+  formDir: NgForm;
+  @Input()
+  setting: string;
+  @Input()
+  limits: object;
+
+  ngOnInit() {
+    const validators = [];
+    if ('min' in this.limits) {
+      validators.push(Validators.min(this.limits['min']));
+    }
+    if ('max' in this.limits) {
+      validators.push(Validators.max(this.limits['max']));
+    }
+    this.settingsForm.get(this.setting).setValidators(validators);
+  }
+}

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-details/iscsi-target-details.component.ts

@@ -8,6 +8,7 @@ import { TableComponent } from '../../../shared/datatable/table/table.component'
 import { Icons } from '../../../shared/enum/icons.enum';
 import { CdTableColumn } from '../../../shared/models/cd-table-column';
 import { CdTableSelection } from '../../../shared/models/cd-table-selection';
+import { BooleanTextPipe } from '../../../shared/pipes/boolean-text.pipe';
 import { IscsiBackstorePipe } from '../../../shared/pipes/iscsi-backstore.pipe';
 
 @Component({
@@ -42,7 +43,11 @@ export class IscsiTargetDetailsComponent implements OnChanges, OnInit {
   title: string;
   tree: TreeModel;
 
-  constructor(private i18n: I18n, private iscsiBackstorePipe: IscsiBackstorePipe) {}
+  constructor(
+    private i18n: I18n,
+    private iscsiBackstorePipe: IscsiBackstorePipe,
+    private booleanTextPipe: BooleanTextPipe
+  ) {}
 
   ngOnInit() {
     this.columns = [
@@ -249,6 +254,13 @@ export class IscsiTargetDetailsComponent implements OnChanges, OnInit {
     };
   }
 
+  private format(value) {
+    if (typeof value === 'boolean') {
+      return this.booleanTextPipe.transform(value);
+    }
+    return value;
+  }
+
   onNodeSelected(e: NodeEvent) {
     if (e.node.id) {
       this.title = e.node.value;
@@ -257,10 +269,11 @@ export class IscsiTargetDetailsComponent implements OnChanges, OnInit {
       if (e.node.id === 'root') {
         this.columns[2].isHidden = false;
         this.data = _.map(this.settings.target_default_controls, (value, key) => {
+          value = this.format(value);
           return {
             displayName: key,
             default: value,
-            current: tempData[key] || value
+            current: !_.isUndefined(tempData[key]) ? this.format(tempData[key]) : value
           };
         });
         // Target level authentication was introduced in ceph-iscsi config v11
@@ -276,10 +289,13 @@ export class IscsiTargetDetailsComponent implements OnChanges, OnInit {
       } else if (e.node.id.toString().startsWith('disk_')) {
         this.columns[2].isHidden = false;
         this.data = _.map(this.settings.disk_default_controls[tempData.backstore], (value, key) => {
+          value = this.format(value);
           return {
             displayName: key,
             default: value,
-            current: !_.isUndefined(tempData.controls[key]) ? tempData.controls[key] : value
+            current: !_.isUndefined(tempData.controls[key])
+              ? this.format(tempData.controls[key])
+              : value
           };
         });
         this.data.push({
@@ -293,7 +309,7 @@ export class IscsiTargetDetailsComponent implements OnChanges, OnInit {
           return {
             displayName: key,
             default: undefined,
-            current: value
+            current: this.format(value)
           };
         });
       }

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-form/iscsi-target-form.component.spec.ts

@@ -31,7 +31,7 @@ describe('IscsiTargetFormComponent', () => {
     target_default_controls: {
       cmdsn_depth: 128,
       dataout_timeout: 20,
-      immediate_data: 'Yes'
+      immediate_data: true
     },
     required_rbd_features: {
       'backstore:1': 0,

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.html

@@ -35,20 +35,10 @@
             <div class="form-group row"
                  *ngFor="let setting of disk_default_controls[bs] | keyvalue">
               <div class="col-sm-12">
-                <label class="col-form-label"
-                       for="{{ setting.key }}">{{ setting.key }}</label>
-                <input type="number"
-                       class="form-control"
-                       [formControlName]="setting.key">
-                <span class="invalid-feedback"
-                      *ngIf="settingsForm.showError(setting.key, formDir, 'min')">
-                  <ng-container i18n>Must be greater than or equal to {{ disk_controls_limits[bs][setting.key]['min'] }}.</ng-container>
-                </span>
-                <span class="invalid-feedback"
-                      *ngIf="settingsForm.showError(setting.key, formDir, 'max')">
-                  <ng-container i18n>Must be less than or equal to {{ disk_controls_limits[bs][setting.key]['max'] }}.</ng-container>
-                </span>
-                <span class="form-text text-muted">{{ helpText[setting.key]?.help }}</span>
+                <cd-iscsi-setting [settingsForm]="settingsForm"
+                                  [formDir]="formDir"
+                                  [setting]="setting.key"
+                                  [limits]="getDiskControlLimits(bs, setting.key)"></cd-iscsi-setting>
               </div>
             </div>
           </ng-container>

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.spec.ts

@@ -7,6 +7,7 @@ import { BsModalRef } from 'ngx-bootstrap/modal';
 
 import { configureTestBed, i18nProviders } from '../../../../testing/unit-test-helper';
 import { SharedModule } from '../../../shared/shared.module';
+import { IscsiSettingComponent } from '../iscsi-setting/iscsi-setting.component';
 import { IscsiTargetImageSettingsModalComponent } from './iscsi-target-image-settings-modal.component';
 
 describe('IscsiTargetImageSettingsModalComponent', () => {
@@ -14,7 +15,7 @@ describe('IscsiTargetImageSettingsModalComponent', () => {
   let fixture: ComponentFixture<IscsiTargetImageSettingsModalComponent>;
 
   configureTestBed({
-    declarations: [IscsiTargetImageSettingsModalComponent],
+    declarations: [IscsiTargetImageSettingsModalComponent, IscsiSettingComponent],
     imports: [SharedModule, ReactiveFormsModule, HttpClientTestingModule, RouterTestingModule],
     providers: [BsModalRef, i18nProviders]
   });
@@ -34,6 +35,27 @@ describe('IscsiTargetImageSettingsModalComponent', () => {
         baz: 3
       }
     };
+    component.disk_controls_limits = {
+      'backstore:1': {
+        foo: {
+          min: 1,
+          max: 512,
+          type: 'int'
+        },
+        bar: {
+          min: 1,
+          max: 512,
+          type: 'int'
+        }
+      },
+      'backstore:2': {
+        baz: {
+          min: 1,
+          max: 512,
+          type: 'int'
+        }
+      }
+    };
     component.backstores = ['backstore:1', 'backstore:2'];
 
     component.ngOnInit();

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.ts

@@ -1,5 +1,5 @@
 import { Component, OnInit } from '@angular/core';
-import { FormControl, Validators } from '@angular/forms';
+import { FormControl } from '@angular/forms';
 
 import * as _ from 'lodash';
 import { BsModalRef } from 'ngx-bootstrap/modal';
@@ -20,37 +20,27 @@ export class IscsiTargetImageSettingsModalComponent implements OnInit {
   backstores: any;
 
   settingsForm: CdFormGroup;
-  helpText: any;
 
   constructor(public modalRef: BsModalRef, public iscsiService: IscsiService) {}
 
   ngOnInit() {
-    this.helpText = this.iscsiService.imageAdvancedSettings;
-
     const fg = {
       backstore: new FormControl(this.imagesSettings[this.image]['backstore'])
     };
     _.forEach(this.backstores, (backstore) => {
       const model = this.imagesSettings[this.image][backstore] || {};
       _.forIn(this.disk_default_controls[backstore], (_value, key) => {
-        const validators = [];
-        if (this.disk_controls_limits && key in this.disk_controls_limits[backstore]) {
-          if ('min' in this.disk_controls_limits[backstore][key]) {
-            validators.push(Validators.min(this.disk_controls_limits[backstore][key]['min']));
-          }
-          if ('max' in this.disk_controls_limits[backstore][key]) {
-            validators.push(Validators.max(this.disk_controls_limits[backstore][key]['max']));
-          }
-        }
-        fg[key] = new FormControl(model[key], {
-          validators: validators
-        });
+        fg[key] = new FormControl(model[key]);
       });
     });
 
     this.settingsForm = new CdFormGroup(fg);
   }
 
+  getDiskControlLimits(backstore, setting) {
+    return this.disk_controls_limits[backstore][setting];
+  }
+
   save() {
     const backstore = this.settingsForm.controls['backstore'].value;
     const settings = {};

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-iqn-settings-modal/iscsi-target-iqn-settings-modal.component.html

@@ -14,44 +14,10 @@
         <div class="form-group row"
              *ngFor="let setting of settingsForm.controls | keyvalue">
           <div class="col-sm-12">
-            <label class="col-form-label"
-                   for="{{ setting.key }}">{{ setting.key }}</label>
-            <input class="form-control"
-                   *ngIf="!isRadio(setting.key)"
-                   type="number"
-                   [formControlName]="setting.key">
-            <span class="invalid-feedback"
-                  *ngIf="settingsForm.showError(setting.key, formDir, 'min')">
-              <ng-container i18n>Must be greater than or equal to {{ target_controls_limits[setting.key]['min'] }}.</ng-container>
-            </span>
-            <span class="invalid-feedback"
-                  *ngIf="settingsForm.showError(setting.key, formDir, 'max')">
-              <ng-container i18n>Must be less than or equal to {{ target_controls_limits[setting.key]['max'] }}.</ng-container>
-            </span>
-
-            <ng-container *ngIf="isRadio(setting.key)">
-              <br>
-              <div class="custom-control custom-radio custom-control-inline">
-                <input type="radio"
-                       [id]="setting.key + 'Yes'"
-                       value="Yes"
-                       [formControlName]="setting.key"
-                       class="custom-control-input">
-                <label class="custom-control-label"
-                       [for]="setting.key + 'Yes'">Yes</label>
-              </div>
-              <div class="custom-control custom-radio custom-control-inline">
-                <input type="radio"
-                       [id]="setting.key + 'No'"
-                       value="No"
-                       class="custom-control-input"
-                       [formControlName]="setting.key">
-                <label class="custom-control-label"
-                       [for]="setting.key + 'No'">No</label>
-              </div>
-            </ng-container>
-
-            <span class="form-text text-muted">{{ helpText[setting.key]?.help }}</span>
+            <cd-iscsi-setting [settingsForm]="settingsForm"
+                              [formDir]="formDir"
+                              [setting]="setting.key"
+                              [limits]="getTargetControlLimits(setting.key)"></cd-iscsi-setting>
           </div>
         </div>
       </div>

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-iqn-settings-modal/iscsi-target-iqn-settings-modal.component.spec.ts

@@ -7,6 +7,7 @@ import { BsModalRef } from 'ngx-bootstrap/modal';
 
 import { configureTestBed, i18nProviders } from '../../../../testing/unit-test-helper';
 import { SharedModule } from '../../../shared/shared.module';
+import { IscsiSettingComponent } from '../iscsi-setting/iscsi-setting.component';
 import { IscsiTargetIqnSettingsModalComponent } from './iscsi-target-iqn-settings-modal.component';
 
 describe('IscsiTargetIqnSettingsModalComponent', () => {
@@ -14,7 +15,7 @@ describe('IscsiTargetIqnSettingsModalComponent', () => {
   let fixture: ComponentFixture<IscsiTargetIqnSettingsModalComponent>;
 
   configureTestBed({
-    declarations: [IscsiTargetIqnSettingsModalComponent],
+    declarations: [IscsiTargetIqnSettingsModalComponent, IscsiSettingComponent],
     imports: [SharedModule, ReactiveFormsModule, HttpClientTestingModule, RouterTestingModule],
     providers: [BsModalRef, i18nProviders]
   });
@@ -26,7 +27,24 @@ describe('IscsiTargetIqnSettingsModalComponent', () => {
     component.target_default_controls = {
       cmdsn_depth: 1,
       dataout_timeout: 2,
-      first_burst_length: 'Yes'
+      first_burst_length: true
+    };
+    component.target_controls_limits = {
+      cmdsn_depth: {
+        min: 1,
+        max: 512,
+        type: 'int'
+      },
+      dataout_timeout: {
+        min: 2,
+        max: 60,
+        type: 'int'
+      },
+      first_burst_length: {
+        max: 16777215,
+        min: 512,
+        type: 'int'
+      }
     };
     component.ngOnInit();
     fixture.detectChanges();

src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-target-iqn-settings-modal/iscsi-target-iqn-settings-modal.component.ts

@@ -1,5 +1,5 @@
 import { Component, OnInit } from '@angular/core';
-import { FormControl, Validators } from '@angular/forms';
+import { FormControl } from '@angular/forms';
 
 import * as _ from 'lodash';
 import { BsModalRef } from 'ngx-bootstrap/modal';
@@ -18,25 +18,13 @@ export class IscsiTargetIqnSettingsModalComponent implements OnInit {
   target_controls_limits: any;
 
   settingsForm: CdFormGroup;
-  helpText: any;
 
   constructor(public modalRef: BsModalRef, public iscsiService: IscsiService) {}
 
   ngOnInit() {
     const fg = {};
-    this.helpText = this.iscsiService.targetAdvancedSettings;
-
     _.forIn(this.target_default_controls, (_value, key) => {
-      const validators = [];
-      if (this.target_controls_limits && key in this.target_controls_limits) {
-        if ('min' in this.target_controls_limits[key]) {
-          validators.push(Validators.min(this.target_controls_limits[key]['min']));
-        }
-        if ('max' in this.target_controls_limits[key]) {
-          validators.push(Validators.max(this.target_controls_limits[key]['max']));
-        }
-      }
-      fg[key] = new FormControl(this.target_controls.value[key], { validators: validators });
+      fg[key] = new FormControl(this.target_controls.value[key]);
     });
 
     this.settingsForm = new CdFormGroup(fg);
@@ -54,7 +42,7 @@ export class IscsiTargetIqnSettingsModalComponent implements OnInit {
     this.modalRef.hide();
   }
 
-  isRadio(control) {
-    return ['Yes', 'No'].indexOf(this.target_default_controls[control]) !== -1;
+  getTargetControlLimits(setting) {
+    return this.target_controls_limits[setting];
   }
 }

src/pybind/mgr/dashboard/frontend/src/app/shared/api/iscsi.service.ts

@@ -11,57 +11,6 @@ import { ApiModule } from './api.module';
 export class IscsiService {
   constructor(private http: HttpClient) {}
 
-  targetAdvancedSettings = {
-    cmdsn_depth: {
-      help: ''
-    },
-    dataout_timeout: {
-      help: ''
-    },
-    first_burst_length: {
-      help: ''
-    },
-    immediate_data: {
-      help: ''
-    },
-    initial_r2t: {
-      help: ''
-    },
-    max_burst_length: {
-      help: ''
-    },
-    max_outstanding_r2t: {
-      help: ''
-    },
-    max_recv_data_segment_length: {
-      help: ''
-    },
-    max_xmit_data_segment_length: {
-      help: ''
-    },
-    nopin_response_timeout: {
-      help: ''
-    },
-    nopin_timeout: {
-      help: ''
-    }
-  };
-
-  imageAdvancedSettings = {
-    hw_max_sectors: {
-      help: ''
-    },
-    max_data_area_mb: {
-      help: ''
-    },
-    osd_op_timeout: {
-      help: ''
-    },
-    qfull_timeout: {
-      help: ''
-    }
-  };
-
   listTargets() {
     return this.http.get(`api/iscsi/target`);
   }