[14:59] <meena> holman: have you got any machines with ZFS where you're testing this growpart stuff on?
[15:00] <meena> I'm currently baffled by my log output, and might need to add some more logs, because, well, it's just baffling nothingness
[15:10] <meena> so, right now, for ZFS, this code is failing here: https://github.com/canonical/cloud-init/blob/main/cloudinit/config/cc_growpart.py#L436-L458 (i marked both conditionals, because if it were to continue, it would fail the next condition, too)
[15:13] <meena> i love that we call         result = util.get_mount_info(devent) and then discard the filesystem info
[15:20] <minimal> meena: yeah the growpart code in general is "funky", when I've worked on it in the past I ended up putting extra debug statements throughout the code to see what was happening
[15:20] <minimal> what exactly is the failure? that the ZFS device is not a block device?
[15:24] <meena> minimal: exactly
[15:24] <meena> it's just some weird string that means nothing to the os, so os.stat will do nothing
[15:24] <meena> we handle it correctly in get_size() but not that function
[15:30] <minimal> meena: I'm not familiar with ZFS but in general for Unix/Linux/etc I thought a storage device (containing partitions) was always a block device...
[15:56] <meena> minimal: not with ZFS;
[15:57] <minimal> I'm thinking of "classic" Unix...
[15:57] <meena> ZFS does the LVM layer and the FS layer, in Linux, or, AIX terms… and as such it's a bit opaque
[15:59] <holman> uh-oh - hmm
[15:59] <meena> so, i have an incredibly ugly patch that I'm not happy with
[16:00] <holman> I thought we had integration test coverage
[16:00] <meena> I'd like to fix get_size() to work with ZFS
[16:00] <minimal> I assume ZFS on Linux behaves in the same fashion
[16:00] <meena> i don't want to know how ZFS on Linux behaves.
[16:00] <holman> iirc there's a compat layer to make zfs work on linux
[16:00] <meena> I would like to remain ignorant of ZFS on Linux for at least another week or five
[16:01] <minimal> I mean whether on Linux a ZFS device is a block device or not
[16:01] <holman> oh, no I think it's the same on linux in that way
[16:01] <holman> you get a pool of disk and add the disk to the pool
[16:02] <meena> this is the make it work patch: https://gist.github.com/igalic/b39206b322336f4de4365c342e1f6bc7 I'll work on make it pretty soon
[16:02] <meena> what if you have more than one disk in a pool?
[16:03] <holman> that works
[16:04] <minimal> I'm "happily oblivious" to ZFS in general :-)
[16:05] <holman> I've "happily not touched" ZFS in about 3 years
[16:06] <holman> and the last time I looked at it its performance on nvme left much to be desired
[16:06] <meena> the thing i like about ZFS is that I haven't lost any data to it, in about… ever.
[16:07] <holman> there are definitely pros to ZFS :-)
[16:07] <minimal> that reminds me, I need to find time to fight with cc_growpart testcases for some changes I've made locally :-( I find making code changes are easy, adapting existing testcases or adding new testcase is the real struggle
[16:08] <holman> the growpart code is monstrous for unit tests
[16:09] <holman> it should be broken into more composable units and the tests adapted to test those units
[16:12] <meena> I'm could add a new fs paramter to resize() and then pass that to get_size() so that i can treat ZFS specially, or just assume if get_size() gets something that doesn't start with /dev that it might be ZFS?
[16:16] <minimal> holman: it's more due to my limited python skills rather than being specific to the growpart testcases
[16:16] <holman> minimal: nah, the growpart unittests are just bad ;-)
[16:18] <minimal> I have changes to support LUKSv1 resizing without needing key but as the changes introduce some new subp's I first need to fix the existing tests (for LUKSv2) accordingly before I can then add similar tests for LUKSv1
[16:18] <holman> meena: sounds reasonable?
[16:18] <minimal> anyway I'll have another crack at it later today
[16:19] <holman> meena: I don't know of any cloud images that are using zfs on root, and I believe the ubuntu installer stuff has its own disk/filesystem code, so you might be the first to exercise this codepath? I don't know the history of this code
[16:20] <holman> meena: we have one meager integration test that tests growpart (added a couple of years ago) https://github.com/canonical/cloud-init/blob/main/tests/integration_tests/modules/test_growpart.py
[16:20] <holman> meena: that could probably be extended for zfs pretty easily
[16:21] <meena> I'm not on Linux with ZFS on root, just on regular old FreeBSD with ZFS on Root, of which there are plenty of cloud images
[16:21] <holman> meena: ohhhh, right
[16:22]  * holman facepalm
[16:22] <meena> I'm the FreeBSD dev, remember?
[16:23] <meena> Either way, I'm just confused how this ever worked before…
[16:23] <holman> hehe
[16:23] <holman> oh yeah me too
[16:24] <holman> that get_size() stuff might have been a late addition for the logs which just unintentionally broke zfs, I'd have to dig into the history
[16:25]  * holman hopes git blame doesn't point back at himself
[16:25] <meena> either way, i have to figure out a cool way to find zfs size now
[16:25] <holman> meena: that lxd integration test^ creates a local sparse file, adds it to the vm as a disk, and then it uses bootcmd to create a tiny partition prior to letting growpart increase the size
[16:26] <meena> wow, that… sounds … horrific.
[16:31] <holman> meena: writing it certainly was :)
[16:31] <meena> zpool get -H -o value size zroot                                                              16:31
[16:31] <meena> 17.5G
[16:31] <meena> so… that's not the value we want
[16:32] <meena> we want, whatever the heck lseek() retruns
[16:32] <meena> or?
[16:33] <holman> but that integration test allows us to exercise the code in CI 
[16:34] <holman> meena: question about growpart on zfs
[16:35] <holman> I've only ever created zfs filesystems on full disks
[16:35] <holman> never a partition
[16:36] <holman> can you actually do that with zfs?
[16:38] <meena> we have mkfs.zfs in FreeBSD which you can use to commit horrendous crimes 
[16:38] <meena> mostly used in image building, i think
[16:39] <meena> speaking of crimes: https://gist.github.com/igalic/8dec389052e4113db039036e160a65d0
[16:48] <meena> I think the problem is None == None ;)
[16:49] <meena> Optional[int] could return NaN :P
[19:13] <meena> zpool get -p -H -o value size zroot
[19:13] <meena> gets an integer
[19:18] <minimal> holman: you must be able to use ZFS on a partition, otherwise how else would you use ZFS on a bootable UEFI system (where it needs to have a ESP partition/FAT filesystem)?
[19:44] <holman> minimal: ESP doesn't _have_ to be FAT, grub supports ZFS ;)
[19:45] <holman> it's just safer to use FAT because it's no longer changing
[19:45] <minimal> holman: the UEFI spec only defines FAT for ESP though...
[19:45] <holman> also that, yes
[19:46] <minimal> actually how would Grub be loaded from ESP by UEFI if the ESP filesystem isn't FAT? ;-)
[19:47] <holman> minimal: grub is written to the disk (before the filesystem starts), grub's config is written to ESP, no?
[19:47] <holman> been a while since I've read up on it
[19:48] <minimal> sounds like you're thinking of Grub in the boot sector, that is for BIOS, not UEFI booting
[19:49] <holman> ahhh, yes probably
[19:50] <minimal> for UEFI the "firmware" either uses a UEFI boot variable (stored on machine's NVRAM) which points to a file in an ESP partition (which has to be FAT fs) or else it fallback to looking for "BOOT/BOORX64.EFI" in the ESP filesystem(s)
[19:50] <minimal> Grub is stored as a file in one of more of the directories on the ESP partition/FAT filesystem
[19:51] <holman> I guess I need to update my priors :-)
[19:52] <minimal> I'm very familiar with various booting scenarios ;-)
[19:53] <minimal> so for a single disk Linux machine to boot UEFI with a ZFS rootfs you'd need to GPT partition the disk with a small ESP (FAT) partition and then another partition using the rest of the disk for ZFS
[19:54] <minimal> regardless of whether you're using Grub, or direct Linux kernel UEFI booting, or Limine, or systemd-boot, or some other UEFI-based bootloader
[19:56] <minimal> meena: several days ago you asked about "make cloud-init uninstall itself", I dug up the method I've used in the past (for NoCloud on bare metal), I add this init.d script to the OS disk image and enable it: https://gist.github.com/dermotbradley/0db72a3d3bde41c60c5f3a9930dda6a1
[19:57] <meena> minimal: wanna see mine?
[19:57] <minimal> sure
[19:58] <holman> that's related to what I was get at - if you hand zfs a partition, shouldn't you be able to resize that partition and than increase its size in the zfs pool?
[19:58] <meena> minimal: https://codeberg.org/meena/test-cloud-init/src/branch/tofu/aws/ami-user-data.yml 
[19:59] <meena> I'm using the FreeBSD official cloud-init images (why isn't that default??! hopefull it will be, but right now they're buggy … or, inconsistent) to install my own cloud-init
[19:59] <meena> that was a long parenthetical.
[20:00] <holman> which makes me wonder if the value we're getting from devent2dev is incorrect
[20:01] <holman> meena: have you ran freebsd on azure before?
[20:01] <meena> holman: only briefly; It'll be my next test target
[20:02] <meena> Also because of hvsock
[20:07] <minimal> holman: sorry, I thought you'd earlier said something about ZFS only working on a "whole disk", i.e. unpartitioned, and that's why I mentioned needing to partition in order to create ESP for UEFI. I can't seem to find what you wrote earlier that gave me that impression lol
[20:08] <holman> minimal: all good, I'm just trying to undstand the semantics of zfs and partition resize within the context of cloud-init's growpart
[20:09] <holman> and wasn't very clear with what I asked
[20:13] <minimal> seems like increase the actual partition, the "zpool online -e", then export pool, then import pool - though not sure if that works with a mounted rootfs
[20:15] <minimal> that's just a (possibly wrong) guess from a brief online search
[20:23] <meena> https://github.com/freebsd/freebsd-src/pull/1163
[20:23] -ubottu:#cloud-init- Pull 1163 in freebsd/freebsd-src "ec2: homedir bug fixed in cloud-init 24.1.4" [Open]
[23:24] <meena> okay,slightly less unhappy with the state of this (now untested code)