Re: [Curtin-dev] [Merge] ~paride/curtin:pin-pyrsistent into curtin:master

2020-09-09 Thread Ryan Harper
Review: Approve Thanks for tracking this down. -- https://code.launchpad.net/~paride/curtin/+git/curtin/+merge/390446 Your team curtin developers is requested to review the proposed merge of ~paride/curtin:pin-pyrsistent into curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Po

Re: [Curtin-dev] [Merge] ~raharper/curtin:fix/uefi-remove-dups-v2 into curtin:master

2020-09-09 Thread Ryan Harper
@Dan I thought we were incorrectly detecting dups, but in-fact, they are duplicates. I asked for followup data from the submitter and confirmed there are duplicates. See comment 6: Boot* EFI Network 1 PcieRoot(0x0)/Pci(0x1,0x0)/Pci(0x0,0x0)/MAC(ecf4bbce7b8c,1)/IPv4(0.0.0.00.0.0.0,0,0) Boot

Re: [Curtin-dev] [Merge] ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master

2020-09-09 Thread Ryan Harper
I'm running a full vmtest over this branch; I'll report results soon. -- https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/390491 Your team curtin developers is requested to review the proposed merge of ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master. -- Mailing

[Curtin-dev] [Merge] ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master

2020-09-09 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master. Requested reviews: curtin developers (curtin-dev) Related bugs: Bug #1888726 in systemd (Ubuntu): "systemd-udevd regression: some renamed network interfaces stuck in "pending"

[Curtin-dev] [Merge] ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master

2020-09-09 Thread Ryan Harper
The proposal to merge ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master has been updated. Description changed to: vmtest: Fix multiple issues with vmtest on master - vmtest: nvme-bcache: do not mark multiple devices bootable that are not bootable, unskip Focal - vmtest: Fix Ps

Re: [Curtin-dev] [Merge] ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master

2020-09-09 Thread Ryan Harper
A couple more fixes needed for clean vmtest run: vmtest: Adjust regex to handling trailing slash in add-apt-repository URL generated Remove skipby for Centos70 UefiReuseEsp vmtest: disable lvm root xfs on /boot xfs test Re-running full vmtest. -- https://code.launchpad.net/~raharper/curtin/+git

Re: [Curtin-dev] [Merge] ~gyurco/curtin:imsm into curtin:master

2020-09-09 Thread Ryan Harper
Review: Needs Fixing Thanks for working on this. I've provided some feedback below in the code. In addition to what we have here, this will also need updates to curtin/block/schema.py as we're adding new fields to the raid structure. diff --git a/curtin/block/schemas.py b/curtin/block/schema

[Curtin-dev] [Bug 1895044] Re: update_nvram defaults to false, not true as per docs

2020-09-09 Thread Ryan Harper
mwhudson already saw this and fixed in master, though without filing a bug and no review AFAICT. =( https://git.launchpad.net/curtin/commit/?id=83944d61e95ea7eb17289625330f0863bec39488 So daily curtin groovy is fixed. This is released in 20.1 so we'll need to SRU to Xenial, Bionic, Focal. **

Re: [Curtin-dev] [Merge] ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master

2020-09-09 Thread Ryan Harper
-- Ran 5104 tests in 3943.630s FAILED (SKIP=829, errors=2) Wed, 09 Sep 2020 17:20:20 -0500: vmtest end [1] in 3945s Both are hung tasks (Guest-side kernel hang). I think this is ready for review, and discussion on how to land.

Re: [Curtin-dev] [Merge] ~gyurco/curtin:imsm into curtin:master

2020-09-10 Thread Ryan Harper
Thanks. Diff comments: > diff --git a/curtin/block/clear_holders.py b/curtin/block/clear_holders.py > index 116ee81..ce1399e 100644 > --- a/curtin/block/clear_holders.py > +++ b/curtin/block/clear_holders.py > @@ -165,11 +165,17 @@ def shutdown_mdadm(device): > """ > > blockdev = bl

Re: [Curtin-dev] [Merge] ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master

2020-09-10 Thread Ryan Harper
Paride, That sounds good. I'll pull those two commits out and rebase this one. -- https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/390491 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : curtin-dev

Re: [Curtin-dev] [Merge] ~sbykov/curtin:LP1895021 into curtin:master

2020-09-10 Thread Ryan Harper
Thanks for submitting this. Let's add one unittest for this: https://paste.ubuntu.com/p/vQfFfn57Xg/ diff --git a/tests/unittests/test_udev.py b/tests/unittests/test_udev.py index 919c7c0b..39c50e25 100644 --- a/tests/unittests/test_udev.py +++ b/tests/unittests/test_udev.py @@ -104,3 +104,15 @@

[Curtin-dev] [Merge] ~raharper/curtin:fix/add-apt-clean-update-vmtest-regex into curtin:master

2020-09-10 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:fix/add-apt-clean-update-vmtest-regex into curtin:master. Commit message: distro: run apt-get clean after dist-upgrade, install, upgrade operations Some vmtest scenarios install multiple large packages which consume storage temporarily and in

[Curtin-dev] [Merge] ~raharper/curtin:fix/refactor-uefi-duplicate-for-reuse into curtin:master

2020-09-10 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:fix/refactor-uefi-duplicate-for-reuse into curtin:master. Commit message: Refactor uefi_remove_duplicates into find/remove functions for reuse Splitting the function into find/remove functions allows reuse in the vmtest which exercises the code

[Curtin-dev] [Merge] ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master

2020-09-10 Thread Ryan Harper
The proposal to merge ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master has been updated. Description changed to: For more details, see: https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/390491 -- Your team curtin developers is subscribed to branch curtin:master.

[Curtin-dev] [Merge] ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master

2020-09-10 Thread Ryan Harper
The proposal to merge ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master has been updated. Commit message changed to: vmtest: Fix multiple issues with vmtest on master - Nvme-bcache: Do not mark multiple devices bootable that are not bootable, remove skip on Focal - Fix Psuedo

[Curtin-dev] [Merge] ~raharper/curtin:fix/add-apt-clean-update-vmtest-regex into curtin:master

2020-09-10 Thread Ryan Harper
The proposal to merge ~raharper/curtin:fix/add-apt-clean-update-vmtest-regex into curtin:master has been updated. Commit message changed to: distro: run apt-get clean after dist-upgrade, install, upgrade Some vmtest scenarios install multiple large packages which consume storage temporarily and

Re: [Curtin-dev] [Merge] ~raharper/curtin:fix/20200908-fix-vmtest-master into curtin:master

2020-09-10 Thread Ryan Harper
Split-outs: https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/390562 https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/390563 -- https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/390491 Your team curtin developers is subscribed to branch curtin:master. --

Re: [Curtin-dev] [Merge] ~gyurco/curtin:imsm into curtin:master

2020-09-14 Thread Ryan Harper
Review: Needs Fixing One suggestion in line. Diff comments: > diff --git a/curtin/block/mdadm.py b/curtin/block/mdadm.py > index 32b467c..9c1d54a 100644 > --- a/curtin/block/mdadm.py > +++ b/curtin/block/mdadm.py > @@ -837,4 +850,8 @@ def md_check(md_devname, raidlevel, devices=[], > spares=[])

Re: [Curtin-dev] [Merge] ~paride/curtin:20.1-29-g81144052-0ubuntu1 into curtin:ubuntu/devel

2020-09-14 Thread Ryan Harper
ager] +- vmtests: fix PreservePartitionWipeVg storage config +- Fix mdraid name creates broken configuration + [James Falcon] (LP: #1803933) +- vmtests: update skiptests +- vmtest: allow installed centos images to reboot (LP: #1881011) + + -- Ryan Harper Mon, 14 Sep 2020 10:5

Re: [Curtin-dev] [Merge] ~ltrager/curtin:lp1895067 into curtin:master

2020-09-15 Thread Ryan Harper
Review: Needs Fixing Diff comments: > diff --git a/curtin/commands/install_grub.py b/curtin/commands/install_grub.py > index 5f8311f..3da1713 100644 > --- a/curtin/commands/install_grub.py > +++ b/curtin/commands/install_grub.py > @@ -254,12 +254,21 @@ def gen_uefi_install_commands(grub_name, g

Re: [Curtin-dev] [Merge] ~paride/curtin:update-packaging into curtin:master

2020-09-16 Thread Ryan Harper
Review: Approve LGTM, thanks Paride. This is the same diff I generated, but cherry-picking the commits makes sense to me (sort of like if we committed them to master and to ubuntu/devel back when they were made). -- https://code.launchpad.net/~paride/curtin/+git/curtin/+merge/390827 Your team

[Curtin-dev] [Bug 1895922] Re: maas could not deploy a kvm-based vm via virsh

2020-09-17 Thread Ryan Harper
I believe this is a duplicate of: https://bugs.launchpad.net/curtin/+bug/1876258 This has already been fixed in curtin, you can test with curtin's daily PPA, or curtin in groovy. W.r.t uvtool; like MAAS, uvtool should generate serial disk attributes for all virtio disks by default. ** Chang

Re: [Curtin-dev] [Merge] ~paride/curtin:release/20.2-0ubuntu1 into curtin:ubuntu/devel

2020-09-28 Thread Ryan Harper
Review: Approve Thanks! -- https://code.launchpad.net/~paride/curtin/+git/curtin/+merge/391406 Your team curtin developers is requested to review the proposed merge of ~paride/curtin:release/20.2-0ubuntu1 into curtin:ubuntu/devel. -- Mailing list: https://launchpad.net/~curtin-dev Post to

[Curtin-dev] [Merge] ~raharper/curtin:fix/curtainer-minimal-ppa-keys into curtin:master

2020-10-05 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:fix/curtainer-minimal-ppa-keys into curtin:master. Commit message: tools/curtainer: dearmor gpg key and use apt-key add Older versions of apt do not support ASCII-armored gpg keys in the /etc/apt/trusted.gpg.d directory. Instead unwrap the gpg

Re: [Curtin-dev] [Merge] ~raharper/curtin:fix/curtainer-minimal-ppa-keys into curtin:master

2020-10-06 Thread Ryan Harper
@Paride Huh, I tried that but had issues; I must have made a mistake as I can confirm it;s working now: (sabro) ~ % lxc launch ubuntu-minimal:xenial x1min Creating x1min Starting x1min (sabro) ~ % lxc exec x1min bash root@x1min:~# echo deb http://ppa.launchpad.net/curtin-dev/daily/ubuntu xenial

[Curtin-dev] [Bug 1898760] Re: curthooks failing in vmtests.test_pollinate_useragent.BionicTestPollinateUserAgent (in curtin-vmtest-daily-f)

2020-10-07 Thread Ryan Harper
Agreed, looks like a transient network error to me as well. ** Changed in: curtin Status: New => Invalid -- You received this bug notification because you are a member of curtin developers, which is subscribed to curtin. https://bugs.launchpad.net/bugs/1898760 Title: curthooks failing

[Curtin-dev] [Merge] ~raharper/curtin:fix/multipath-lvm-dirty-setup into curtin:master

2020-10-07 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:fix/multipath-lvm-dirty-setup into curtin:master. Commit message: vmtests/multipath-lvm: dont assume device-mapper block names - Add comments in the early_command code to document what it is doing - Use the /device/mapper/mpatha symlink to

Re: [Curtin-dev] [Merge] ~paride/curtin:vmtest-fix-groovy-arm64-subarch into curtin:master

2020-10-07 Thread Ryan Harper
Review: Approve Nice catch! We missed that when landing the groovy branch. Which makes me think we don't have a good test to verify we're booting the kernel we expect to boot. -- https://code.launchpad.net/~paride/curtin/+git/curtin/+merge/391922 Your team curtin developers is requested to

Re: [Curtin-dev] [Merge] ~paride/curtin:fix-deb-clean into curtin:master

2020-10-16 Thread Ryan Harper
Review: Approve LGTM -- https://code.launchpad.net/~paride/curtin/+git/curtin/+merge/392367 Your team curtin developers is requested to review the proposed merge of ~paride/curtin:fix-deb-clean into curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : curtin-dev@list

[Curtin-dev] [Bug 1900712] Re: Unable to deploy Focal ARM64 "grub-install: error: failed to register the EFI boot entry: Operation not permitted."

2020-10-21 Thread Ryan Harper
I'm marking the curtin task invalid. The logs show commands that curtin invokes (grub-install/efibootmgr) returning errors. If further logs or new data show curtin isn't invoking these commands correctly, please reopen this task with more details. ** Changed in: curtin Status: Incomplete

[Curtin-dev] [Merge] ~raharper/curtin:fix/clear-holders-detect-mpath-partition into curtin:master

2020-10-21 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:fix/clear-holders-detect-mpath-partition into curtin:master. Commit message: clear-holders: fix identification of multipath partitions identify_partition calls multipath.is_mpath_partition using at sysfs path to the device but is_mpath_partition

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1893818 into curtin:master

2020-11-18 Thread Ryan Harper
> But in groovy+, the udev rule from multipath-tools that has > always attempted to remove the devices nodes for the partitions > of a disk that is a multipath member actually succeeds, and Won't this break curtin on older multipaths that don't have this rule? Or will this work with older multipa

Re: [Curtin-dev] [Merge] ~freddierice/curtin:mdadm-metadata into curtin:master

2020-11-22 Thread Ryan Harper
This looks fine to me. I think we have sufficient testing around raid; AFAIK, we don't do anything that requires 1.2. I don't think we need to add any additional testing unless we know of a scenario where a specific metadata level is known to not work. We don't have any tests on DDF or imsm

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1893818 into curtin:master

2020-11-22 Thread Ryan Harper
> > > But in groovy+, the udev rule from multipath-tools that has > > > always attempted to remove the devices nodes for the partitions > > > of a disk that is a multipath member actually succeeds, and > > > > Won't this break curtin on older multipaths that don't have this rule? > > No, if the dev

Re: [Curtin-dev] [Merge] ~nacc/curtin:nacc/lp1892494 into curtin:master

2020-11-22 Thread Ryan Harper
Diff comments: > diff --git a/curtin/commands/apt_config.py b/curtin/commands/apt_config.py > index e7d84c0..7cb1484 100644 > --- a/curtin/commands/apt_config.py > +++ b/curtin/commands/apt_config.py > @@ -343,20 +343,22 @@ def apply_preserve_sources_list(target): > raise > > > -de

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:pare-down-dasdview-parsing into curtin:master

2020-11-22 Thread Ryan Harper
Diff comments: > diff --git a/curtin/block/dasd.py b/curtin/block/dasd.py > index 7450300..8524d63 100644 > --- a/curtin/block/dasd.py > +++ b/curtin/block/dasd.py > @@ -461,42 +278,31 @@ class DasdDevice(CcwDevice): > """ Returns a boolean indicating if the specified device_id is not

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:only-eckd-dasds-extract_storage_config into curtin:master

2020-11-22 Thread Ryan Harper
Review: Needs Fixing Diff comments: > diff --git a/curtin/storage_config.py b/curtin/storage_config.py > index 494b142..84825f6 100644 > --- a/curtin/storage_config.py > +++ b/curtin/storage_config.py > @@ -981,6 +981,8 @@ class DasdParser(ProbertParser): > probe_data_key = 'dasd' > >

[Curtin-dev] [Merge] ~raharper/curtin:fix/dont-add-eoan-tests into curtin:master

2020-11-22 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:fix/dont-add-eoan-tests into curtin:master. Commit message: vmtests: Replace newly added Eoan test with Groovy Eoan is EOL, replace Eoan entry in newly added test with Groovy Requested reviews: Server Team CI bot (server-team-bot): continuous

Re: [Curtin-dev] [Merge] ~ltrager/curtin:lp1895067 into curtin:master

2020-12-01 Thread Ryan Harper
Review: Approve Thanks, LGTM -- https://code.launchpad.net/~ltrager/curtin/+git/curtin/+merge/390517 Your team curtin developers is requested to review the proposed merge of ~ltrager/curtin:lp1895067 into curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : curtin-de

[Curtin-dev] [Merge] ~raharper/curtin:fix/grub-boot-id-for-rhel-images into curtin:master

2020-12-02 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:fix/grub-boot-id-for-rhel-images into curtin:master. Commit message: install_grub: Fix bootloader-id for RHEL systems, must be redhat In 7310b4fe61465, the port from shell dropped a change where the boot-id value for Redhat family OSes is the

Re: [Curtin-dev] [Merge] ~raharper/curtin:fix/grub-boot-id-for-rhel-images into curtin:master

2020-12-02 Thread Ryan Harper
@ltrager if you've access to RHEL images (vs. Centos); it would be really good to have verification this resolves the install issue mention in the LP. -- https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/394766 Your team curtin developers is requested to review the proposed merge of

[Curtin-dev] [Merge] ~raharper/curtin:fix/verify-secondary-esp into curtin:master

2020-12-08 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:fix/verify-secondary-esp into curtin:master. Commit message: curthooks: verify secondary ESP partitions have fat32 format Ignore partitions with flag: boot which are not FAT32 formatted as they are not actual ESPs. LP: #1907280 Requested

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:only-eckd-dasds-extract_storage_config into curtin:master

2020-12-10 Thread Ryan Harper
Review: Approve Sorry for the delay, this looks good now. -- https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/394224 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : curtin-dev@lists.launchpad.net U

Re: [Curtin-dev] [Merge] ~oddbloke/curtin/+git/curtin:vmtests into curtin:master

2020-12-16 Thread Ryan Harper
Review: Approve \o/ -- https://code.launchpad.net/~oddbloke/curtin/+git/curtin/+merge/395436 Your team curtin developers is requested to review the proposed merge of ~oddbloke/curtin/+git/curtin:vmtests into curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : curtin

[Curtin-dev] [Merge] ~raharper/curtin:vmtest/enable-h-series into curtin:master

2021-01-13 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:vmtest/enable-h-series into curtin:master. Commit message: vmtest: add Hirsute release classes, tool to add vmtest class Add a tool to generate new release classes based on a previous release, for example: ./tools/vmtest-add-release -p tests

Re: [Curtin-dev] [Merge] ~raharper/curtin:vmtest/enable-h-series into curtin:master

2021-01-13 Thread Ryan Harper
make sync-images does not pull down the hirsute kernel variants for me; so maybe we also need to tackle that maas-daily vs maas-stable issues we see from time to time... -- https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/396278 Your team curtin developers is requested to review

[Curtin-dev] [Merge] ~raharper/curtin:fix/vmtest-image-sync-after-maas-url-rename into curtin:master

2021-01-14 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:fix/vmtest-image-sync-after-maas-url-rename into curtin:master. Commit message: vmtest: fix image-sync after maas URL stream rename Image syncing for vmtest has been failing since MAAS renamed their daily image stream to stable. In an effort to

Re: [Curtin-dev] [Merge] ~raharper/curtin:vmtest/enable-h-series into curtin:master

2021-01-14 Thread Ryan Harper
After sorting sync-images issues, local run of: ./tools/jenkins-runner -p 12 --filter=target_release=hirsute -- Ran 923 tests in 1957.220s OK (SKIP=139) Thu, 14 Jan 2021 14:42:59 -0600: vmtest end [0] in 1959s -- https://co

Re: [Curtin-dev] [Merge] ~raharper/curtin:vmtest/enable-h-series into curtin:master

2021-01-14 Thread Ryan Harper
I need to fix up tox; let me do that now. -- https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/396278 Your team curtin developers is requested to review the proposed merge of ~raharper/curtin:vmtest/enable-h-series into curtin:master. -- Mailing list: https://launchpad.net/~curtin-

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:fix-multipath-partition-verification into curtin:master

2021-01-14 Thread Ryan Harper
Added some inline comments/suggestions. It would be good to add a unittest that exercises the new path where it invokes udevadm_info() to check the DM_UUID and other changes this is introducing. Diff comments: > diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py > index 0cf0866..

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:fix-multipath-partition-verification-2 into curtin:master

2021-01-18 Thread Ryan Harper
Diff comments: > diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py > index 0cf0866..eca69d8 100644 > --- a/curtin/block/__init__.py > +++ b/curtin/block/__init__.py > @@ -158,9 +158,6 @@ def sys_block_path(devname, add=None, strict=True): > devname = os.path.normpath(devname

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:fix-multipath-partition-verification-2 into curtin:master

2021-01-19 Thread Ryan Harper
> Michael Hudson-Doyle (mwhudson) wrote 6 minutes ago: > I'm not sure what "really disks" means on a philosophical level but yes Maybe I'm missing the here, but just for the folks following along I'll be explicit. I'm referring to multipath partitions which a linear mapping to offsets on the mu

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:fix-multipath-partition-verification-2 into curtin:master

2021-01-19 Thread Ryan Harper
Diff comments: > diff --git a/curtin/block/multipath.py b/curtin/block/multipath.py > index 7ad1791..b1cf777 100644 > --- a/curtin/block/multipath.py > +++ b/curtin/block/multipath.py > @@ -88,11 +88,11 @@ def is_mpath_partition(devpath, info=None): > return result > > > -def mpath_par

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1893818 into curtin:master

2021-01-20 Thread Ryan Harper
Some more inline questions. I definitely want to discuss the devtype mpath-partition, mpath-disk values. We cannot write out a curtin storage config with type:mpath-disk type:mpath-partition values; they aren't in the block schema. Diff comments: > diff --git a/curtin/block/multipath.py b/c

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1878041 into curtin:master

2021-01-21 Thread Ryan Harper
I'm happy with this cleanup, thanks. It looks like there were two issues, first we'd timeout on enumerating the maps and the DM_NAME cleans that up nicely. The second was dealing with multipath disks (dm-X) with 4K sector sizes. Could you mention that in the commit message as well. Some inli

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:fix-multipath-partition-verification-2 into curtin:master

2021-01-21 Thread Ryan Harper
Review: Approve Looks good. -- https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/396462 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : curtin-dev@lists.launchpad.net Unsubscribe : https://launch

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1878041 into curtin:master

2021-01-21 Thread Ryan Harper
Review: Approve This is good. I suggest for commit message subject line: multipath: use udev DM_NAME for find_mpath_id, fix 4k sector calc -- https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/396538 Your team curtin developers is subscribed to branch curtin:master. -- Mailing

Re: [Curtin-dev] [Merge] ~xnox/curtin:lp1906379-2 into curtin:master

2021-01-25 Thread Ryan Harper
This looks good. Can we get a comment in the commit message describing the failure? In particular, I'm also looking to understand if this failure is reproducible in VMTest so we can add that scenario to VMTest in this commit as well. Diff comments: > diff --git a/curtin/commands/install_grub.p

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:revert-lp1892494 into curtin:master

2021-01-26 Thread Ryan Harper
Review: Approve I'm +1 on this. I suggest we follow up with a unittest/vmtest which does verify that we get keys in /etc/apt/trusted.gpg.d/ after apt-key add operations such that when a second try is pushed, it will be required to pass known working tests. -- https://code.launchpad.net/~mwhud

Re: [Curtin-dev] [Merge] ~nacc/curtin:nacc/lp1892494 into curtin:master

2021-01-26 Thread Ryan Harper
On Tue, Jan 26, 2021 at 7:59 PM Nish Aravamudan wrote: > Thanks for letting me know. I'll submit an updated branch with a testcase > from that bug. > Currently the vmtest doesn't verify the apt key that's written out as it uses add-apt-repository to add the curtin-dev repo in the apt_config_comm

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1893818 into curtin:master

2021-02-11 Thread Ryan Harper
Do we have a reuse existing mpath with partitions in vmtest? If not, that'd be a good thing to add. Diff comments: > diff --git a/curtin/storage_config.py b/curtin/storage_config.py > index e6c33cc..81b7385 100644 > --- a/curtin/storage_config.py > +++ b/curtin/storage_config.py > @@ -681,10 +6

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1893818 into curtin:master

2021-03-02 Thread Ryan Harper
That script is from tests/exampltes/centos_defaults.yaml It was inplace for dealing with an older grub in centos66... that said, I wonder what's changed in the Centos7X image such that the grub.cfg file isn't present any more... It master passed on this centos image: centos/centos70/amd64/2

[Curtin-dev] [Merge] ~raharper/curtin:fix/centos-grub into curtin:master

2021-03-02 Thread Ryan Harper
Ryan Harper has proposed merging ~raharper/curtin:fix/centos-grub into curtin:master. Commit message: vmtest/centos: handle different paths to grub config Centos images used to always have grub installed at /boot/grub/grub.conf. The most recent build of centos70, centos70/amd64/20210210_01/root

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1893818 into curtin:master

2021-03-02 Thread Ryan Harper
The new maas image moved grub config location. =( https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/399028 This fixed the multipath test case for sure. I also plan to see if I can only run that fix up on centos66 to confirm -- https://code.launchpad.net/~mwhudson/curtin/+git/cur

Re: [Curtin-dev] [Merge] ~dbungert/curtin:nonzero-fs_pass into curtin:master

2021-03-17 Thread Ryan Harper
Review: Needs Fixing Thanks for submitting the MR. Personally I don't think this is the right choice to make by default. I don't think we install anything but journaled filesystems in which fsck will merely replay the journal just like the kernel already does. Look at fsck.xfs(8) manpage f

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1868177 into curtin:master

2021-03-19 Thread Ryan Harper
o wipe all the things so we don't really care how it's done. Looking at git history, _wipe_superblock added strict=true when we added DASD support, and unfortunately I provided *no* justification for this change: commit f722ef6b6bffadbe0ac5c3e7b22ae77bf8ab297d Author: Ryan Harper Date:

Re: [Curtin-dev] [Merge] ~dbungert/curtin:nonzero-fs_pass into curtin:master

2021-03-19 Thread Ryan Harper
> We discussed this internally a bit and still think we should change the I do wish this sort of discussion would happen either on discourse or ubuntu-devel/ubuntu-server. > default. (I haven't heard back from the MAAS team yet). The kernel is not the > only thing that reads filesystems (UEFI

[Curtin-dev] [Merge] ~mwhudson/curtin:lp-1918990-dd-ext2 into curtin:master

2021-03-23 Thread Ryan Harper
The proposal to merge ~mwhudson/curtin:lp-1918990-dd-ext2 into curtin:master has been updated. Commit message changed to: swap: use dd to allocate swapfiles on ext2 and ext3 LP: #1918990 For more details, see: https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/400074 -- Your team

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1918990-dd-ext2 into curtin:master

2021-03-23 Thread Ryan Harper
Review: Approve LGTM, one nit if you're so inclined. Diff comments: > diff --git a/curtin/swap.py b/curtin/swap.py > index 11e95c4..bba263e 100644 > --- a/curtin/swap.py > +++ b/curtin/swap.py > @@ -135,9 +135,10 @@ def setup_swapfile(target, fstab=None, swapfile=None, > size=None, maxsize=None

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1868177 into curtin:master

2021-03-24 Thread Ryan Harper
I think this is fine. I _was_ thinking to only remove the strict=True default in _wipe_superblock in clear-holders. However no where else is code calling with strict=true; if *something* external were using wipe_superblock and strict=true, so this is the minimal change I think: (here's me wa

Re: [Curtin-dev] [Merge] ~dbungert/curtin:fs_pass_one into curtin:master

2021-03-24 Thread Ryan Harper
Back when I looked at changing this, I also included this in vmtest: ``` diff --git a/tests/vmtests/__init__.py b/tests/vmtests/__init__.py index 0b19d8f8..21a41aaa 100644 --- a/tests/vmtests/__init__.py +++ b/tests/vmtests/__init__.py @@ -556,6 +556,8 @@ DEFAULT_COLLECT_SCRIPTS = { [ ! -

Re: [Curtin-dev] [Merge] ~dbungert/curtin:fs_pass_one into curtin:master

2021-04-01 Thread Ryan Harper
I've been using this: https://paste.ubuntu.com/p/x2TQ7d2j9v/ which does a better job of collecting boot time (captures systemd-analyze at shutdown time instead of during boot). I run TestSimple which uses block-meta simple, it defaults to one root partition the size of the entire disk; this al

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:raid-min-levels into curtin:master

2021-04-12 Thread Ryan Harper
I'm kinda -1 on this. Is there a launchpad bug for this? >From mdadm manpage: -n, --raid-devices= Specify the number of active devices in the array. This, plus the number of spare devices (see below) must equal the number of component-devices (includ‐ ing "missing

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:raid-min-levels into curtin:master

2021-04-16 Thread Ryan Harper
Thanks for the link to the discussion. I think layouts are a requirement for relaxing the minimum drive check. If the config includes a layout specification, then we can assume the config is aware of the number of drives/data-copy requirements. -- https://code.launchpad.net/~mwhudson/curtin/

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:raid-min-levels into curtin:master

2021-04-21 Thread Ryan Harper
> Hi Ryan, > Hi Thimo, > I am the author of the PR > (https://github.com/canonical/subiquity/pull/929) as well as the > originating bug report > (https://bugs.launchpad.net/subiquity/+bug/1880023). Thanks for > considering the changes. A few comments from my side: > I think this change is firs

Re: [Curtin-dev] [Merge] ~ryan-p-norwood/curtin:1925399 into curtin:master

2021-04-22 Thread Ryan Harper
Review: Needs Fixing Thanks for submitting a patch to curtin! The updated regex looks fine. Would you add a unittest? tests/unittests/test_storage_config.py: You could copy and adapt test_disk_schema_accepts_nvme_eui or wwid into a test_disk_schema_accepts_nvme_uuid with the wwid:uuid. patte

[Curtin-dev] [Merge] ~ryan-p-norwood/curtin:1925399 into curtin:master

2021-04-22 Thread Ryan Harper
Ryan Harper has proposed merging ~ryan-p-norwood/curtin:1925399 into curtin:master. Commit message: Fix NVMe validation for namespaces with UUID NVMe namespaces are only verified if the wwn comes from either the NGUID or EUI64. With NVMe 1.3 a third ID, UUID is now[1] supported for a namespace

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:raid-min-levels into curtin:master

2021-04-22 Thread Ryan Harper
Thimo, Thanks for the thoughtful reply. I've replied more below to your questions. I would say here as a summary (tl;dr): I'm OK with curtin supporting two-disk RAID5 and RAID10. I'd like to see this in the form of adding raid_devices: N field to the type: raid structure. I don't have direct c

Re: [Curtin-dev] [Merge] ~ryan-p-norwood/curtin:1925399 into curtin:master

2021-04-23 Thread Ryan Harper
On Fri, Apr 23, 2021 at 8:19 AM Ryan wrote: > Hi, I've added the unit tests but there is a Jenkins build failure that I > cannot see. > Thanks. I cannot see it either; I'll ping someone who can. In the meantime if you run tox locally on the tree you should see the same thing jenkins shows. --

Re: [Curtin-dev] [Merge] ~ryan-p-norwood/curtin:1925399 into curtin:master

2021-04-26 Thread Ryan Harper
Thanks, I'll ping @paride on IRC On Mon, Apr 26, 2021 at 10:54 AM Ryan wrote: > I fixed the spacing errors in the unit test, but Jenkins is still failing. > Tox runs successfully in my environment. I'm not sure where to go from here. > -- > https://code.launchpad.net/~ryan-p-norwood/curtin/+git/

[Curtin-dev] [Merge] ~dbungert/curtin:lp-1925722-v2 into curtin:master

2021-04-26 Thread Ryan Harper
matching serial and grub_device true. (Thanks to Ryan Harper and Lee Trager for good discussions and fix proposals) LP: #1925722 For more details, see: https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/401825 -- Your team curtin developers is subscribed to branch curtin:master

Re: [Curtin-dev] [Merge] ~dbungert/curtin:lp-1925722-v2 into curtin:master

2021-04-26 Thread Ryan Harper
Review: Needs Fixing Thanks for the MP. I adjusted the commit message a bit. The field in the MP will be used in the git commit log so we have an expected format[1]. This looks good. One comment below about logging when we fail to get a match with the serial. 1. https://curtin.readthedocs.

Re: [Curtin-dev] [Merge] ~dbungert/curtin:lp-1925722-v2 into curtin:master

2021-04-26 Thread Ryan Harper
> 15:28:08 Please check for an update for distro-info-data. See > /usr/share/doc/distro-info-data/README.Debian for details. > > Do we need to 'apt update' the build machines? No... this is just the unfortunate timing post-release pre-distro-info-data package getting updated in the archive... @P

Re: [Curtin-dev] [Merge] ~ryan-p-norwood/curtin:1925399 into curtin:master

2021-04-26 Thread Ryan Harper
@Ryan I think the issue is temporary; in another recent MP, there's a package update that's pending (distro-info) which happens twice a year right after Ubuntu releases... In the mean time, have you had a chance to sign the CLA[1] 1. https://curtin.readthedocs.io/en/latest/topics/hacking.

Re: [Curtin-dev] [Merge] ~ryan-p-norwood/curtin:1925399 into curtin:master

2021-04-27 Thread Ryan Harper
> Thank you for the status update. I signed the CLA. Great! > Do you know the cadence for this change to make it into the downstream > Subiquity Yes. Subiquity tends to pull from curtin master since it's built as a snap. MAAS, the other consumer of subiquity, also pulls in curtin from master as

Re: [Curtin-dev] [Merge] ~ryan-p-norwood/curtin:1925399 into curtin:master

2021-04-27 Thread Ryan Harper
I'm +1 on this. Once launchpad updates with your CLA status this can be merged. Thanks for finding and fixing this issue! -- https://code.launchpad.net/~ryan-p-norwood/curtin/+git/curtin/+merge/401624 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://l

Re: [Curtin-dev] [Merge] ~ryan-p-norwood/curtin:1925399 into curtin:master

2021-04-29 Thread Ryan Harper
Review: Approve LP is all synced up with CLA status, marking Approved. -- https://code.launchpad.net/~ryan-p-norwood/curtin/+git/curtin/+merge/401624 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : curtin-dev@li

[Curtin-dev] [Merge] ~ryan-p-norwood/curtin:1925399 into curtin:master

2021-04-30 Thread Ryan Harper
The proposal to merge ~ryan-p-norwood/curtin:1925399 into curtin:master has been updated. Commit message changed to: Fix NVMe validation for namespaces with UUID NVMe namespaces are only verified if the wwn comes from either the NGUID or EUI64. With NVMe 1.3 a third ID, UUID is now[1] supported

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:mdadm-check-container into curtin:master

2021-05-07 Thread Ryan Harper
I like your initial commit message, can you update the commit box with that one as that's what the autolander will take when it squash merges. Once suggestion on the md_check refactor below. Diff comments: > diff --git a/curtin/block/schemas.py b/curtin/block/schemas.py > index 3923321..d846505

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:mdadm-check-container into curtin:master

2021-05-10 Thread Ryan Harper
Was there a LP bug number for this? If so please add the LP: # to the commit message. Looks good. -- https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/402384 Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:mdadm-check-container into curtin

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1927703 into curtin:master

2021-05-10 Thread Ryan Harper
Review: Approve LGTM -- https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/402473 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : curtin-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~c

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:convert-imsm into curtin:master

2021-06-11 Thread Ryan Harper
Diff comments: > diff --git a/curtin/storage_config.py b/curtin/storage_config.py > index 81b7385..ebe7bb9 100644 > --- a/curtin/storage_config.py > +++ b/curtin/storage_config.py > @@ -1047,16 +1046,25 @@ class RaidParser(ProbertParser): > # FIXME, need to handle rich md_name values,

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:convert-imsm into curtin:master

2021-06-24 Thread Ryan Harper
Review: Approve LGTM -- https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/404051 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : curtin-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~c

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:reformat-vs-preserve into curtin:master

2021-06-24 Thread Ryan Harper
Review: Needs Information Hrm, I didn't think this was true (that preserve means keeping partition table). looking at raid_handler, if you set wipe: superblock, it will wipe a verified. ``` create_raid = True if preserve: raid_verify(md_devname, raidlevel, device_paths, spare_device_paths)

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:reformat-vs-preserve into curtin:master

2021-06-30 Thread Ryan Harper
This looks great. Two comments: 1) let's update doc/topics/storage.rst with the comment you added in the disk handler around what preserve/wipe means for existing compound devices. 2) In the vmtest, create a partition table on top of the raid different than the final ptable in the storage conf

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:reformat-vs-preserve into curtin:master

2021-07-01 Thread Ryan Harper
Review: Approve Awesome! This looks great. -- https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/404623 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : curtin-dev@lists.launchpad.net Unsubscribe : h

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:lp-1928839 into curtin:master

2021-07-15 Thread Ryan Harper
Review: Approve This is quite an excellent solution; I had been thinking about using mount namespaces for this but private is much simpler. Diff comments: > diff --git a/curtin/util.py b/curtin/util.py > index be063d7..7be3033 100644 > --- a/curtin/util.py > +++ b/curtin/util.py > @@ -695,7 +69

Re: [Curtin-dev] [Merge] ~dbungert/curtin:20.1-3 into curtin:focal-maas-2021-10

2021-10-04 Thread Ryan Harper
Diff comments: > diff --git a/debian/rules b/debian/rules > index 933c8e4..aa17316 100755 > --- a/debian/rules > +++ b/debian/rules > @@ -18,7 +18,10 @@ override_dh_auto_install: > > override_dh_auto_test: > make unittest3 > +<<< debian/rules Some merge conflicts remain > > ov

Re: [Curtin-dev] [Merge] ~mwhudson/curtin:image-action into curtin:master

2021-10-06 Thread Ryan Harper
Diff comments: > diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py > index aae85c6..4b5a56b 100644 > --- a/curtin/commands/block_meta.py > +++ b/curtin/commands/block_meta.py > @@ -530,6 +539,23 @@ def get_path_to_storage_volume(volume, storage_config): > return vo

<    1   2   3   4   >