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

Revert "Add support to resize rootfs if using LVM (#721)" #887

Merged
merged 1 commit into from
May 3, 2021

Conversation

OddBloke
Copy link
Collaborator

Proposed Commit Message

Revert "Add support to resize rootfs if using LVM (#721)"

This reverts commit 74fa008bfcd3263eb691cc0b3f7a055b17569f8b.

During pre-release testing, we discovered two issues with this commit.
Firstly, there's a typo in the udevadm command that causes a TypeError
for _all_ growpart executions.  Secondly, the LVM resizing does not
appear to successfully resize everything up to the LV, though some
things do get resized.

We certainly want this change, so we'll be happy to review and land it
alongside an integration test which confirms that it is working as
expected.

LP: #1922742

Additional Context

I'm working on an integration test to cover this LVM case, as it's a fairly complex use of our test framework. A rough draft can be seen in https://paste.ubuntu.com/p/y6zG3fdTZv/

Test Steps

LXD containers do not perform disk setup (as they don't have any disks), so CI does not exercise this. LXD VM testing does trip over this issue, so I'm currently performing a full VM test run locally.

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

This reverts commit 74fa008.

During pre-release testing, we discovered two issues with this commit.
Firstly, there's a typo in the udevadm command that causes a TypeError
for _all_ growpart executions.  Secondly, the LVM resizing does not
appear to successfully resize everything up to the LV, though some
things do get resized.

We certainly want this change, so we'll be happy to review and land it
alongside an integration test which confirms that it is working as
expected.
@OddBloke
Copy link
Collaborator Author

(Please leave this for me to land: I won't look at the results of my test run until the morning, and don't want this to land without that testing completing.)

@OddBloke
Copy link
Collaborator Author

OddBloke commented May 3, 2021

My testing has completed successfully.

@OddBloke OddBloke merged commit 5f5fa5e into canonical:master May 3, 2021
@OddBloke OddBloke deleted the revert branch May 3, 2021 14:56
@OddBloke
Copy link
Collaborator Author

Here's the fleshed-out test:

"""Integration tests for cc_growpart.

TODO:
* Write tests for all of cc_growpart's functionality.
import pytest
"""

import pytest


class TestGrowPartLVM:
    """
    Test that LVM partitions are correctly resized.

    This test uses a bootcmd to create a loop device and partition it using
    LVM.  The underlying partition does not used the whole loop "disk", nor
    does the Logical Volume we create use the whole of the Physical Volume.

    This test checks that, after boot, the Logical Volume has been resized to
    use the whole loop device (with some allowance for partitioning/LVM
    overhead).
    """

    # These steps pulled from https://ops.tips/blog/lvm-on-loopback-devices/
    LVM_USER_DATA = """\
    #cloud-config
    bootcmd:
        # Create our LVM "disk"
        - dd if=/dev/zero of=/lvm0.img bs=50 count=1M
        # Use loop7 because snaps take up the early numbers
        - losetup /dev/loop7 /lvm0.img
        # Create an LVM partition on the first half of the disk
        - echo "start=1,size=25M,type=8e" | sfdisk /dev/loop7
        # Update the kernel's partition table
        - partx --update /dev/loop7
        # Create our LVM PV and VG
        - pvcreate /dev/loop7p1
        - vgcreate myvg /dev/loop7p1
        # Create our LV with a smaller size than the whole PV
        - lvcreate --size 10M --name lv1 myvg
        # Create a filesystem to resize
        - mkfs.ext4 /dev/mapper/myvg-lv1
    growpart:
        devices: ["/dev/mapper/myvg-lv1"]
    """

    # Our disk is 50M; with 4MB extents, this will mean 48M for the Logical
    # Volume, so use a slightly lower threshold than that
    LVM_LOWER_THRESHOLD = 47 * 1024 * 1024

    @pytest.mark.user_data(LVM_USER_DATA)
    def test_resize_successful(self, client):
        def _get_size_of(device):
            ret = client.execute(
                ["lsblk", "-b", "--output", "SIZE", "-n", "-d", device]
            )
            if ret.ok:
                return int(ret.stdout.strip())
            pytest.fail("Failed to get size of {}: {}".format(device, ret))

        assert _get_size_of("/dev/loop7p1") > self.LVM_LOWER_THRESHOLD
        assert _get_size_of("/dev/mapper/myvg-lv1") > self.LVM_LOWER_THRESHOLD

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