Merge pull request #199 from dwlehman/trim-volume-size

Trim volume size as needed to fit in pool free space
linux-system-roles · Mar 12, 2021 · 9581348 · 9581348
2 parents 566bfa1 + 50a3801
commit 9581348
Show file tree

Hide file tree

Showing 7 changed files with 172 additions and 61 deletions.
diff --git a/README.md b/README.md
@@ -95,6 +95,10 @@ must contain only a single item.
 The `size` specifies the size of the file system. The format for this is intended to
 be human-readable, e.g.: "10g", "50 GiB".
 
+__NOTE__: The requested volume size may be reduced as necessary so the volume can
+          fit in the available pool space, but only if the required reduction is
+          not more than 2% of the requested volume size.
+
 ##### `fs_type`
 This indicates the desired file system type to use, e.g.: "xfs", "ext4", "swap".
 The default is determined according to the OS and release

diff --git a/library/blivet.py b/library/blivet.py
@@ -130,6 +130,9 @@
     set_up_logging()
     log = logging.getLogger(BLIVET_PACKAGE + ".ansible")
 
+
+MAX_TRIM_PERCENT = 2
+
 use_partitions = None  # create partitions on pool backing device disks?
 disklabel_type = None  # user-specified disklabel type
 safe_mode = None       # do not remove any existing devices or formatting
@@ -445,8 +448,16 @@ def _resize(self):
             if not self._device.resizable:
                 return
 
-            if self._device.format.resizable:
-                self._device.format.update_size_info()
+            trim_percent = (1.0 - float(self._device.max_size / size))*100
+            log.debug("resize: size=%s->%s ; trim=%s", self._device.size, size, trim_percent)
+            if size > self._device.max_size and trim_percent <= MAX_TRIM_PERCENT:
+                log.info("adjusting %s resize target from %s to %s to fit in free space",
+                         self._volume['name'],
+                         size,
+                         self._device.max_size)
+                size = self._device.max_size
+                if size == self._device.size:
+                    return
 
             if not self._device.min_size <= size <= self._device.max_size:
                 raise BlivetAnsibleError("volume '%s' cannot be resized to '%s'" % (self._volume['name'], size))
@@ -610,10 +621,18 @@ def _create(self):
             raise BlivetAnsibleError("invalid size '%s' specified for volume '%s'" % (self._volume['size'], self._volume['name']))
 
         fmt = self._get_format()
+        trim_percent = (1.0 - float(parent.free_space / size))*100
+        log.debug("size: %s ; %s", size, trim_percent)
         if size > parent.free_space:
-            raise BlivetAnsibleError("specified size for volume '%s' exceeds available space in pool '%s' (%s)" % (size,
-                                                                                                                   parent.name,
-                                                                                                                   parent.free_space))
+            if trim_percent > MAX_TRIM_PERCENT:
+                raise BlivetAnsibleError("specified size for volume '%s' exceeds available space in pool '%s' (%s)"
+                                         % (size, parent.name, parent.free_space))
+            else:
+                log.info("adjusting %s size from %s to %s to fit in %s free space", self._volume['name'],
+                                                                                    size,
+                                                                                    parent.free_space,
+                                                                                    parent.name)
+                size = parent.free_space
 
         try:
             device = self._blivet.new_lv(name=self._volume['name'],

diff --git a/tests/tests_create_lv_size_equal_to_vg.yml b/tests/tests_create_lv_size_equal_to_vg.yml
@@ -0,0 +1,48 @@
+---
+- hosts: all
+  become: true
+  vars:
+    storage_safe_mode: false
+    mount_location: '/opt/test1'
+    volume_group_size: '10g'
+    lv_size: '10g'
+    unused_disk_subfact: '{{ ansible_devices[unused_disks[0]] }}'
+    disk_size: '{{ unused_disk_subfact.sectors|int *
+                   unused_disk_subfact.sectorsize|int }}'
+
+  tasks:
+    - include_role:
+        name: linux-system-roles.storage
+
+    - include_tasks: get_unused_disk.yml
+      vars:
+        min_size: "{{ volume_group_size }}"
+        max_return: 1
+
+    - name: Create one lv which size is equal to vg size
+      include_role:
+        name: linux-system-roles.storage
+      vars:
+          storage_pools:
+            - name: foo
+              disks: "{{ unused_disks }}"
+              volumes:
+                - name: test1
+                  size: "{{ lv_size }}"
+                  mount_point: "{{ mount_location }}"
+
+    - include_tasks: verify-role-results.yml
+
+    - name: Clean up
+      include_role:
+        name: linux-system-roles.storage
+      vars:
+          storage_pools:
+            - name: foo
+              disks: "{{ unused_disks }}"
+              state: "absent"
+              volumes:
+                - name: test1
+                  mount_point: "{{ mount_location }}"
+
+    - include_tasks: verify-role-results.yml
diff --git a/tests/tests_lvm_auto_size_cap.yml b/tests/tests_lvm_auto_size_cap.yml
@@ -0,0 +1,91 @@
+---
+- hosts: all
+  become: true
+
+  tasks:
+    - include_role:
+        name: linux-system-roles.storage
+
+    - include_tasks: get_unused_disk.yml
+      vars:
+        min_size: 10g
+        max_return: 1
+
+    - command: lsblk -b -l --noheadings -o NAME,SIZE
+      register: storage_test_lsblk
+      changed_when: false
+
+    - set_fact:
+        test_disk_size: "{{ storage_test_lsblk.stdout_lines|map('regex_search', '^' + unused_disks[0] + '\\s+\\d+$')|select('string')|first|regex_replace('^\\w+\\s+', '') }}"
+
+    - package:
+        name: bc
+        state: installed
+
+    - command:
+        cmd: bc
+        stdin: "{{ test_disk_size }} *2"
+      register: doubled_size
+      changed_when: false
+
+    - name: Test handling of too-large LVM volume size
+      block:
+        - name: Try to create a pool containing one volume twice the size of the backing disk
+          include_role:
+            name: linux-system-roles.storage
+          vars:
+            storage_pools:
+                - name: foo
+                  type: lvm
+                  disks: "{{ unused_disks }}"
+                  volumes:
+                    - name: test1
+                      size: "{{ doubled_size.stdout|trim }}"
+        - name: unreachable task
+          fail:
+            msg: UNREACH
+      rescue:
+        - name: Check that we failed in the role
+          assert:
+            that:
+              - ansible_failed_result.msg != 'UNREACH'
+              - blivet_output.failed and
+                blivet_output.msg|regex_search('specified size for volume.+exceeds available')
+            msg: "Role has not failed when it should have"
+
+    - name: Create a pool containing one volume the same size as the backing disk
+      include_role:
+        name: linux-system-roles.storage
+      vars:
+        storage_pools:
+            - name: foo
+              disks: "{{ unused_disks }}"
+              volumes:
+                - name: test1
+                  size: "{{ test_disk_size }}"
+
+    - include_tasks: verify-role-results.yml
+
+    - name: Repeat the previous invocation to verify idempotence
+      include_role:
+        name: linux-system-roles.storage
+      vars:
+        storage_pools:
+            - name: foo
+              type: lvm
+              disks: "{{ unused_disks }}"
+              volumes:
+                - name: test1
+                  size: "{{ test_disk_size }}"
+
+    - include_tasks: verify-role-results.yml
+
+    - name: Clean up
+      include_role:
+        name: linux-system-roles.storage
+      vars:
+        storage_pools:
+            - name: foo
+              disks: "{{ unused_disks }}"
+              state: absent
+              volumes: []
diff --git a/tests/tests_lvm_errors.yml b/tests/tests_lvm_errors.yml
@@ -11,8 +11,6 @@
       - '/non/existent/disk'
     invalid_size: 'xyz GiB'
     unused_disk_subfact: '{{ ansible_devices[unused_disks[0]] }}'
-    too_large_size: '{{ (unused_disk_subfact.sectors|int + 1) *
-                        unused_disk_subfact.sectorsize|int }}'
 
   tasks:
     - include_role:
@@ -86,39 +84,6 @@
               - ansible_failed_result.msg != 'UNREACH'
             msg: "Role has not failed when it should have"
 
-    # the following does not work properly
-    # - name: Verify the output
-    #   assert:
-    #     that: "{{ blivet_output.failed and
-    #               blivet_output.msg|regex_search('invalid size.+for volume') and
-    #               not blivet_output.changed }}"
-    #     msg: "Unexpected behavior w/ invalid volume size"
-
-    - name: Test for correct handling of too-large volume size.
-      block:
-        - name: Try to create LVM with a too-large volume size.
-          include_role:
-            name: linux-system-roles.storage
-          vars:
-            storage_pools:
-              - name: foo
-                disks: "{{ unused_disks }}"
-                volumes:
-                  - name: test1
-                    size: "{{ too_large_size }}"
-                    mount_point: "{{ mount_location1 }}"
-
-        - name: unreachable task
-          fail:
-            msg: UNREACH
-
-      rescue:
-        - name: Check that we failed in the role
-          assert:
-            that:
-              - ansible_failed_result.msg != 'UNREACH'
-            msg: "Role has not failed when it should have"
-
     # the following does not work properly
     # - name: Verify the output
     #   assert:
@@ -138,7 +103,7 @@
                 disks: "{{ unused_disks[0] }}"
                 volumes:
                   - name: test1
-                    size: "{{ too_large_size }}"
+                    size: "{{ volume_size }}"
                     mount_point: "{{ mount_location1 }}"
 
         - name: unreachable task
@@ -171,7 +136,7 @@
                 disks: []
                 volumes:
                   - name: test1
-                    size: "{{ too_large_size }}"
+                    size: "{{ volume1_size }}"
                     mount_point: "{{ mount_location1 }}"
 
         - name: unreachable task

diff --git a/tests/tests_misc.yml b/tests/tests_misc.yml
@@ -7,7 +7,7 @@
     volume_group_size: '5g'
     volume1_size: '4g'
     unused_disk_subfact: '{{ ansible_devices[unused_disks[0]] }}'
-    too_large_size: '{{ (unused_disk_subfact.sectors|int + 1) *
+    too_large_size: '{{ (unused_disk_subfact.sectors|int * 1.2) *
                         unused_disk_subfact.sectorsize|int }}'
 
   tasks:

diff --git a/tests/tests_resize.yml b/tests/tests_resize.yml
@@ -9,7 +9,7 @@
     invalid_size1: 'xyz GiB'
     invalid_size2: 'none'
     unused_disk_subfact: '{{ ansible_devices[unused_disks[0]] }}'
-    too_large_size: '{{ (unused_disk_subfact.sectors|int + 1) *
+    too_large_size: '{{ unused_disk_subfact.sectors|int * 1.2 *
                         unused_disk_subfact.sectorsize|int }}'
     disk_size: '{{ unused_disk_subfact.sectors|int *
                    unused_disk_subfact.sectorsize|int }}'
@@ -122,23 +122,7 @@
                     size: "{{ disk_size }}"
                     mount_point: "{{ mount_location }}"
 
-        - name: Unreachable task
-          fail:
-            msg: UNREACH
-
-      rescue:
-        - name: Check that we failed in the role
-          assert:
-            that:
-              - ansible_failed_result.msg != 'UNREACH'
-            msg: "Role has not failed when it should have"
-
-        - name: Verify the output
-          assert:
-            that: "blivet_output.failed and
-                   blivet_output.msg|regex_search('volume.+cannot be resized to.+') and
-                   not blivet_output.changed"
-            msg: "Unexpected behavior w/ invalid volume size"
+    - include_tasks: verify-role-results.yml
 
     - name: Test for correct handling of invalid size specification
       block: