From ce341cfba931468fb64ee069817832c5dee5eee3 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Stephan=20M=C3=BCller?= <smueller@suse.com>
Date: Fri, 21 Sep 2018 10:08:19 +0200
Subject: [PATCH] mgr/dashboard: Unset any compression setting for pools in UI
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

The dashboard API now can delete all compression options if the
compression mode is set to 'unset' during edit. Other dismissed options
will be overwritten with the reset value on submit, which will cause
the deletion of the previously set settings.

Fixes: https://tracker.ceph.com/issues/36355
Signed-off-by: Stephan Müller <smueller@suse.com>
---
 .../app/ceph/pool/pool-form/pool-form-info.ts |  1 +
 .../pool/pool-form/pool-form.component.html   | 10 ---
 .../pool-form/pool-form.component.spec.ts     | 84 +++++++++++++++----
 .../pool/pool-form/pool-form.component.ts     | 57 ++++++++-----
 4 files changed, 104 insertions(+), 48 deletions(-)

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-info.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-info.ts
index 931083b0f6d..e1e56eadfce 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-info.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-info.ts
@@ -4,6 +4,7 @@ export class PoolFormInfo {
   pool_names: string[];
   osd_count: number;
   is_all_bluestore: boolean;
+  bluestore_compression_algorithm: string;
   compression_algorithms: string[];
   compression_modes: string[];
   crush_rules_replicated: CrushRule[];
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html
index 7299a5c5676..f67bbdbba72 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html
@@ -317,10 +317,6 @@
                         id="mode"
                         name="mode"
                         formControlName="mode">
-                  <option i18n
-                          ngValue="">
-                    -- Select a compression mode --
-                  </option>
                   <option *ngFor="let mode of info.compression_modes"
                           [value]="mode">
                     {{ mode }}
@@ -352,12 +348,6 @@
                             ngValue="">
                       -- No erasure compression algorithm available --
                     </option>
-                    <option *ngIf="info.compression_algorithms &&
-                                   info.compression_algorithms.length > 0"
-                            i18n
-                            ngValue="">
-                      -- Select a compression algorithm --
-                    </option>
                     <option *ngFor="let algorithm of info.compression_algorithms"
                             [value]="algorithm">
                       {{ algorithm }}
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts
index fcb601cfac2..dd550fd6af6 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts
@@ -114,8 +114,9 @@ describe('PoolFormComponent', () => {
       pool_names: [],
       osd_count: OSDS,
       is_all_bluestore: true,
-      compression_algorithms: [],
-      compression_modes: [],
+      bluestore_compression_algorithm: 'snappy',
+      compression_algorithms: ['snappy'],
+      compression_modes: ['none', 'passive'],
       crush_rules_replicated: [],
       crush_rules_erasure: []
     };
@@ -351,6 +352,17 @@ describe('PoolFormComponent', () => {
         expect(control.value).toBe('2 KiB');
       });
 
+      it('validates that odd size validator works as expected', () => {
+        const odd = (min, max) => component['oddBlobSize'](min, max);
+        expect(odd('10', '8')).toBe(true);
+        expect(odd('8', '-')).toBe(false);
+        expect(odd('8', '10')).toBe(false);
+        expect(odd(null, '8')).toBe(false);
+        expect(odd('10', '')).toBe(false);
+        expect(odd('10', null)).toBe(false);
+        expect(odd(null, null)).toBe(false);
+      });
+
       it('validates ratio to be only valid between 0 and 1', () => {
         isValid(form.get('ratio'));
         hasError(setValue('ratio', -0.1), 'min');
@@ -924,22 +936,58 @@ describe('PoolFormComponent', () => {
         hasError(setPgNum(4), 'noDecrease');
       });
 
-      it(`always provides the application metadata array with submit even if it's empty`, () => {
-        component.data.applications.selected = [];
-        testSubmit(
-          {
-            application_metadata: [],
-            compression_algorithm: 'lz4',
-            compression_max_blob_size: 1048576,
-            compression_min_blob_size: 524288,
-            compression_mode: 'passive',
-            compression_required_ratio: 0.8,
-            pg_num: 32,
-            pool: 'somePoolName'
-          },
-          'pool/edit',
-          'update'
-        );
+      describe('submit', () => {
+        const markControlAsPreviouslySet = (controlName) => form.get(controlName).markAsPristine();
+
+        beforeEach(() => {
+          ['algorithm', 'maxBlobSize', 'minBlobSize', 'mode', 'pgNum', 'ratio', 'name'].forEach(
+            (name) => markControlAsPreviouslySet(name)
+          );
+          fixture.detectChanges();
+        });
+
+        it(`always provides the application metadata array with submit even if it's empty`, () => {
+          expect(form.get('mode').dirty).toBe(false);
+          component.data.applications.selected = [];
+          testSubmit(
+            {
+              application_metadata: [],
+              pool: 'somePoolName'
+            },
+            'pool/edit',
+            'update'
+          );
+        });
+
+        it(`will always provide reset value for compression options`, () => {
+          setValue('minBlobSize', '').markAsDirty();
+          setValue('maxBlobSize', '').markAsDirty();
+          setValue('ratio', '').markAsDirty();
+          testSubmit(
+            {
+              application_metadata: ['rbd', 'rgw'],
+              compression_max_blob_size: 0,
+              compression_min_blob_size: 0,
+              compression_required_ratio: 0,
+              pool: 'somePoolName'
+            },
+            'pool/edit',
+            'update'
+          );
+        });
+
+        it(`will unset mode not used anymore`, () => {
+          setValue('mode', 'none').markAsDirty();
+          testSubmit(
+            {
+              application_metadata: ['rbd', 'rgw'],
+              compression_mode: 'unset',
+              pool: 'somePoolName'
+            },
+            'pool/edit',
+            'update'
+          );
+        });
       });
     });
   });
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts
index fc8002c04e5..1eb9c6ab496 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts
@@ -132,7 +132,8 @@ export class PoolFormComponent implements OnInit {
   }
 
   private initInfo(info: PoolFormInfo) {
-    info.compression_algorithms = info.compression_algorithms.filter((m) => m.length > 0);
+    this.form.silentSet('algorithm', info.bluestore_compression_algorithm);
+    info.compression_modes.push('unset');
     this.info = info;
   }
 
@@ -227,9 +228,9 @@ export class PoolFormComponent implements OnInit {
       this.pgCalc();
     });
     this.form.get('mode').valueChanges.subscribe(() => {
-      ['minBlobSize', 'maxBlobSize', 'ratio'].forEach((name) =>
-        this.form.get(name).updateValueAndValidity()
-      );
+      ['minBlobSize', 'maxBlobSize', 'ratio'].forEach((name) => {
+        this.form.get(name).updateValueAndValidity({ emitEvent: false });
+      });
     });
     this.form.get('minBlobSize').valueChanges.subscribe(() => {
       this.form.get('maxBlobSize').updateValueAndValidity({ emitEvent: false });
@@ -382,13 +383,13 @@ export class PoolFormComponent implements OnInit {
     CdValidators.validateIf(this.form.get('minBlobSize'), () => this.activatedCompression(), [
       Validators.min(0),
       CdValidators.custom('maximum', (size) =>
-        this.compareBlobSize(size, this.form.getValue('maxBlobSize'))
+        this.oddBlobSize(size, this.form.getValue('maxBlobSize'))
       )
     ]);
     CdValidators.validateIf(this.form.get('maxBlobSize'), () => this.activatedCompression(), [
       Validators.min(0),
       CdValidators.custom('minimum', (size) =>
-        this.compareBlobSize(this.form.getValue('minBlobSize'), size)
+        this.oddBlobSize(this.form.getValue('minBlobSize'), size)
       )
     ]);
     CdValidators.validateIf(this.form.get('ratio'), () => this.activatedCompression(), [
@@ -397,10 +398,10 @@ export class PoolFormComponent implements OnInit {
     ]);
   }
 
-  private compareBlobSize(minimum, maximum) {
-    return Boolean(
-      minimum && maximum && this.formatter.toBytes(minimum) >= this.formatter.toBytes(maximum)
-    );
+  private oddBlobSize(minimum, maximum) {
+    minimum = this.formatter.toBytes(minimum);
+    maximum = this.formatter.toBytes(maximum);
+    return Boolean(minimum && maximum && minimum >= maximum);
   }
 
   activatedCompression() {
@@ -429,9 +430,8 @@ export class PoolFormComponent implements OnInit {
       this.form.setErrors({ cdSubmitButton: true });
       return;
     }
-    const pool = {};
+    const pool = { pool: this.form.getValue('name') };
     this.extendByItemsForSubmit(pool, [
-      { api: 'pool', name: 'name', edit: true },
       { api: 'pool_type', name: 'poolType' },
       { api: 'pg_num', name: 'pgNum', edit: true },
       this.form.getValue('poolType') === 'replicated'
@@ -445,7 +445,7 @@ export class PoolFormComponent implements OnInit {
         name: 'ecOverwrites',
         fn: () => ['ec_overwrites']
       });
-      if (this.form.getValue('mode')) {
+      if (this.form.getValue('mode') !== 'none') {
         this.extendByItemsForSubmit(pool, [
           {
             api: 'compression_mode',
@@ -458,15 +458,26 @@ export class PoolFormComponent implements OnInit {
             api: 'compression_min_blob_size',
             name: 'minBlobSize',
             fn: this.formatter.toBytes,
-            edit: true
+            edit: true,
+            resetValue: 0
           },
           {
             api: 'compression_max_blob_size',
             name: 'maxBlobSize',
             fn: this.formatter.toBytes,
-            edit: true
+            edit: true,
+            resetValue: 0
           },
-          { api: 'compression_required_ratio', name: 'ratio', edit: true }
+          { api: 'compression_required_ratio', name: 'ratio', edit: true, resetValue: 0 }
+        ]);
+      } else if (this.editing) {
+        this.extendByItemsForSubmit(pool, [
+          {
+            api: 'compression_mode',
+            name: 'mode',
+            edit: true,
+            fn: () => 'unset'
+          }
         ]);
       }
     }
@@ -488,22 +499,28 @@ export class PoolFormComponent implements OnInit {
       name,
       attr,
       fn,
-      edit
+      edit,
+      resetValue
     }: {
       api: string;
       name: string;
       attr?: string;
       fn?: Function;
       edit?: boolean;
+      resetValue?: any;
     }
   ) {
-    if (this.editing && !edit) {
+    if (this.editing && (!edit || this.form.get(name).pristine)) {
       return;
     }
     const value = this.form.getValue(name);
-    const apiValue = fn ? fn(value) : attr ? _.get(value, attr) : value;
+    let apiValue = fn ? fn(value) : attr ? _.get(value, attr) : value;
     if (!value || !apiValue) {
-      return;
+      if (edit && !_.isUndefined(resetValue)) {
+        apiValue = resetValue;
+      } else {
+        return;
+      }
     }
     pool[api] = apiValue;
   }
-- 
2.39.5