Skip to content
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

Create snapshots with normalized sizes #11

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

ygalblum
Copy link
Collaborator

@ygalblum ygalblum commented Aug 9, 2023

check.py - print new volumes array with normalized values when successful
role - use the normalized array instead of the one passed by the user
Test Infra - Allow creating multiple volumes and snapshots
Tests: Add test to verify that FREE based snapshots with the same percentage get the same size

Copy link
Member

@swapdisk swapdisk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The create action is not working for me. For example, I'm running with this...

  lvm_snapshots_volumes:
    - vg: rhel_rhel7
      lv: root
      size: 1g
    - vg: rhel_rhel7
      lv: data
      size: 1g

My VG has 10g free, so plenty of free space for this. I'm seeing the same issue using percent of or absolute sizes.

When I run the create action, I see the space check passes...

TASK [lvm_snapshots : Verify that there is enough storage space] ***************
task path: /home/bmader/swapdisk/ygalblum/ipu-lvm-snapshot/roles/lvm_snapshots/tasks/check.yml:9
ok: [default] => changed=false 
  failed_when_result: false
  rc: 0
  stderr: |-
    Shared connection to 192.168.122.63 closed.
  stderr_lines: <omitted>
  stdout: |-
    [{"lv": "root", "size": 1073741824.0, "vg": "rhel_rhel7"}, {"lv": "data", "size": 1073741824.0, "vg": "rhel_rhel7"}]
  stdout_lines: <omitted>
Thursday 10 August 2023  13:31:51 -0500 (0:00:00.203)       0:00:02.663 ******* 

TASK [lvm_snapshots : Store check return in case of failure] *******************
task path: /home/bmader/swapdisk/ygalblum/ipu-lvm-snapshot/roles/lvm_snapshots/tasks/check.yml:21
skipping: [default] => changed=false 
  skip_reason: Conditional result was False
Thursday 10 August 2023  13:31:51 -0500 (0:00:00.020)       0:00:02.709 ******* 

TASK [lvm_snapshots : Assert results] ******************************************
task path: /home/bmader/swapdisk/ygalblum/ipu-lvm-snapshot/roles/lvm_snapshots/tasks/check.yml:26
ok: [default] => changed=false 
  msg: The Volume Groups have enough space to create the requested snapshots
Thursday 10 August 2023  13:31:51 -0500 (0:00:00.024)       0:00:02.734 ******* 

But when it tries to create the snapshots, the task fails with insufficient space...

TASK [lvm_snapshots : Create snapshots] ****************************************
task path: /home/bmader/swapdisk/ygalblum/ipu-lvm-snapshot/roles/lvm_snapshots/tasks/create.yml:30
failed: [default] (item={'lv': 'root', 'size': 1073741824.0, 'vg': 'rhel_rhel7'}) => changed=false 
  ansible_loop_var: item
  err: |2-
      Volume group "rhel_rhel7" has insufficient free space (2559 extents): 32124 required.
  invocation:
    module_args:
      active: true
      force: false
      lv: root
      opts: null
      pvs: null
      resizefs: false
      shrink: true
      size: '1073741824.0'
      snapshot: root_ripu
      state: present
      thinpool: null
      vg: rhel_rhel7
  item:
    lv: root
    size: 1073741824.0
    vg: rhel_rhel7
  msg: Creating logical volume 'root' failed
  rc: 5

Looks like maybe we are passing bytes to lvol, but it's defaulting to megabytes?

check.py - align snapshot sizes to extent size
check.py - print new volumes array with normalized values when successful
role- use the normalized array instead of the one passed by the user
Test Infra: Allow creating multiple volumes and snapshots
Tests: Add test to verify that FREE based snapshots with the same percentage get the same size

Signed-off-by: Ygal Blum <ygal.blum@gmail.com>
@ygalblum
Copy link
Collaborator Author

First, you are right, the issue was that the default unit is megabytes. The returned JSON should include the unit type 'B'.

But, I needed to understand why my tests did not fail. The reason it failed was that the test was requesting the snapshot size smaller than the size of the original volume. As a result, lvcreate limited the request to the size of the volume. Since there are two volumes each of 2G and therefore two snapshots (again each of 2G) totaling the request to 8G on a 9G volume, the test passed. Once I've changed the volumes' sizes to 3G, the test started failing as expected.

However, even with the fix, using 50%FREE still passed the check and failed the creation So, I investigated further and found that the size of a logical volume must align to the size of the extent of the volume group and the lvcreate command rounds up the request automatically. As a result, a request for 2 snapshots each of 50%FREE passed the check, but failed the creation because the first request took more than 50% of the free due to alignment.

The fix to the second issue should have probably gone to a separate PR. But, at the current stage of the repo, I guess we can live with adding it as part of this one.

@swapdisk
Copy link
Member

No problem. I'll play with this in my lab environment today.

@swapdisk
Copy link
Member

Everything works in my RHEL7 lab. LGTM.

@swapdisk swapdisk merged commit 33a4850 into redhat-cop:main Aug 14, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants