[17:39] <ahosmanMSFT> Hey rharper and blackboxsw I just saw your message. Was at a team event all day yesterday.
[17:39] <rharper> np
[17:39] <ahosmanMSFT> @rharper: I see you got a PR up, looking at it now
[17:39] <rharper> great
[17:40] <ahosmanMSFT> How did testing go?
[17:40] <rharper> all seems well on the branch I proposed
[17:40] <rharper> i can run things the way you do, as well as using a --deb, etc
[17:41] <ahosmanMSFT> oh so images launch?
[17:41] <rharper> I think the two core issues were the missing start() in the image.py and using vm_name vs. image_id
[17:41] <rharper> yes
[17:42] <ahosmanMSFT> Nice, two birds with one stone. I suppose that permanently fixes the ssh issue?
[17:42] <rharper> I believe so
[17:50] <ahosmanMSFT> I was also looking into preserve instance, have you looked into that. Since I delete the resource group, the instance gets deleted when reserve instance gets invoked. I have a fix in mind to not delete the resource group, but the user would be in charge of deleting it. What do you think @rharper and @blackboxsw?
[17:51] <rharper> ahosmanMSFT: w.r.t preserve-instance,  I do believe that's expected; it may be useful to emit the resource group name in the logging , something like "No removing resourse group %s due to --preserve-instance setting; requires manual removal"
[17:55] <ahosmanMSFT> Great, that's what I was thinking. Is preserve instance passed as a boolean?
[17:57] <ahosmanMSFT> Also @rharper, approved the PR is another reviewer needed
[17:58] <rharper> ahosmanMSFT: yes, boolean, and yes blackboxsw or Odd_Bloke will need to review as well
[18:03] <ahosmanMSFT> Ok @rharper looking into that now
[18:07] <rharper> nice
[18:11] <ahosmanMSFT> @rharper In stages.py preserve_instance is initialized and destroys VM if False, this doesn't account for the resource group.
[18:11] <ahosmanMSFT> How would you like this done
[18:11] <ahosmanMSFT> I can omit the destruction of the resource group but I would have to add to stages
[18:12] <ahosmanMSFT> which should be generic by nature
[18:57] <minfrin> Hey all, always being dragged off somewhere after answering the question.
[18:57] <minfrin> I am trying to find out how cloud-init can be told to mkswap and swapon a swap disk - most specifically a swap disk, not a swap file.
[18:58] <minfrin> This doesn't appear to be enough:
[18:58] <minfrin> fs_setup:  - label: vidi    device: /dev/xvde    filesystem: ext4  - label: swap    device: /dev/xvdg    filesystem: swapmounts:- [ /dev/xvde, /var/lib/vidispine, ext4, defaults, 0, 0 ]- [ /dev/xvdg, none, swap, sw, 0, 0 ]
[18:58] <blackboxsw> minfrin: can you paste somewhere with fomatting? can't quite grok that.
[18:59] <blackboxsw> https://paste.ubuntu.com would work
[18:59] <blackboxsw> I'm assuming that's part of #cloud-config
[19:00] <blackboxsw> or paste.openstack.org maybe
[19:01] <minfrin> Yep, formatting a bit stuffed. Here is a paste: https://paste.ubuntu.com/p/TpjfznyJZJ/
[19:10] <blackboxsw> thx, ok valid yaml.
[19:12] <blackboxsw> minfrin: missing  something like a top-level swap: key maybe?
[19:13] <blackboxsw> minfrin: like this maybe https://paste.ubuntu.com/p/sg7Wky7tZ9/
[19:16] <blackboxsw> I'm not totally sure, just looking through the code in cloudinit/config/cc_mounts.py https://github.com/canonical/cloud-init/blob/master/cloudinit/config/cc_mounts.py#L458-L467
[19:16] <blackboxsw> it looks like needsswap is set True if the overall cloud-config contains a top-level "swap" key with definitions
[19:16] <blackboxsw> and then swapon gets called https://github.com/canonical/cloud-init/blob/master/cloudinit/config/cc_mounts.py#L510L511
[19:19] <rharper> ahosmanMSFT: I'll take a look
[19:19] <ahosmanMSFT> @rharper: I actually have a fix
[19:20] <ahosmanMSFT> https://paste.ubuntu.com/p/4KzMXmjCKS/
[19:20] <ahosmanMSFT> update azurecloudtest/platform.destroy
[19:21] <ahosmanMSFT> can you add that with your fix, so we can get it merged faster
[19:21] <rharper> ahosmanMSFT: I suspect we want some sort of args passed down into the Instance or Image class from stages
[19:21] <rharper> ahosmanMSFT: no, we'll do this in a separate PR
[19:21] <ahosmanMSFT> ok
[19:22] <ahosmanMSFT> rharper: Did that with the paste, what do you think? I can push the PR
[19:23] <minfrin> @blackboxsw: If I'm reading this correctly from https://github.com/canonical/cloud-init/blob/06e324ff8edb3126e5a8060757a48ceab2b1a121/cloudinit/config/cc_mounts.py#L481 - if line[2] is swap we get swapon.
[19:25] <minfrin> blackboxsw: In theory this line should be enough to trigger a swapon?
[19:25] <minfrin> - [ /dev/xvdg, none, swap, sw, 0, 0 ]
[19:30]  * minfrin tries https://paste.ubuntu.com/p/sg7Wky7tZ9/
[19:36] <ahosmanMSFT> @rharper: https://github.com/canonical/cloud-init/compare/master...AOhassan:azurecitestsupdate?expand=1
[19:55] <minfrin> blackboxsw: Configured as per the paste, the mkswap fails as per the paste: https://paste.ubuntu.com/p/6pBDvzsST8/
[19:56] <minfrin> It looks like despite no maxsize being passed, cloud-init passes the empty string:
[19:56] <minfrin> Command: ['/sbin/mkswap', '/dev/xvdg', '-L', 'swap', '']
[19:56] <minfrin> That then fails, as expected:
[19:56] <rharper> ahosmanMSFT: I don't think that's goign to work, as we need the PlatformComponent instance from collect_test_data() in collect;  I suspect we may want to pass the args.preserve_instance  setting into the partial so the platform instance retains this setting
[19:56] <minfrin> Stderr: mkswap: invalid block count argument: ''
[20:30] <minfrin> blaackboxsw: Looks like with maxsize passed, we're still passed the empty string.
[20:34] <blackboxsw> interesting minfrin, could be a bug there. trying to see if we had an integration tests covering this
[20:34] <blackboxsw> hrm nope
[20:35] <blackboxsw> minfrin: have an updated #cloud-config snippet you could paste
[20:35] <blackboxsw> ?
[20:38] <rharper> blackboxsw: afaict, there's not way with swap: to create this against a block device, which is what it sounds like minfrin wants;  I would suggest to use bootcmd: ['mkswap', '/dev/xxxx']; and then a mount:  entry
[20:39] <rharper> in general the swapfile is more portable which is why cloud-init prefers that;  if you capture an image from an instance, the configured swap goes with it; vs having to ensure you have a secondary disk always present at a specific location
[20:41] <blackboxsw> rharper: minfrin while I keep making wild guesses at this :) I also noticed Odd_Bloke's cloud-config test for swap SRU test was referencing just the device name, not the full device path https://github.com/cloud-init/ubuntu-sru/blob/master/bugs/b59870ca.txt#L16-L28
[20:41] <minfrin> I've raised this as a bug here: https://bugs.launchpad.net/cloud-init/+bug/1862417
[20:41] <ubot5> Ubuntu bug 1862417 in cloud-init "cloud-init: Attempt to mkswap on a partition fails: invalid block count argument: ''" [Undecided,New]
[20:41] <blackboxsw> thanks for the bug minfrin, we can respond there.
[20:42] <minfrin> Passing the empty string as the "size" parameter definitely looks odd.
[20:43] <rharper> blackboxsw: well, the swap cloud-config specifies filename, but we don't specifically say it cannot be an existing block device; but the code as written isn't going to correctly mkswap on a disk or partition;  and I don't think it was intended to;
[20:43] <rharper> as I mentioned, it's non-portable with image capture;  that doesn't mean cloudinit can't support mkswap on devices;
[20:47] <minfrin> What confused me was examples that showed swap disks being created, but not examples where mkswap was included.
[20:49] <rharper> minfrin: which example?
[20:50] <blackboxsw> minfrin: also in your bug, I see you specified  mount_default_fields: [ None, None, "auto", "defaults", "0", "2" ]
[20:50] <blackboxsw> I think proper yaml should be none instead of None. otherwise it is treated as the string "None"
[20:51] <blackboxsw> instead of python's None
[20:51] <blackboxsw> not sure if that affects things or not
[20:53] <blackboxsw> sorry I mean null in yaml
[20:53] <blackboxsw> example of what I mean https://paste.ubuntu.com/p/4M7nFWWBnQ/
[20:56] <blackboxsw> though I realize that cloud-config is documented in cloud-init's rtd page. we must have a mapping that sets that appropriately
[21:00] <minfrin> rharper: this is what I worked from: https://cloudinit.readthedocs.io/en/latest/topics/examples.html#adjust-mount-points-mounted?
[21:01] <blackboxsw> funny also that those docs there say: # complete.  This must be an array, and must have 7 fields.
[21:01] <blackboxsw> yet the docs provide a list of 6 items
[21:01] <rharper> the comments field
[21:01] <rharper> minfrin: thanks
[21:02] <blackboxsw> even in code     defvals = [None, None, "auto", def_mnt_opts, "0", "2"]   6 items. I'll propose a tiny doc fix for that aspect now
[21:06] <rharper> hrm, somethings strange, as the fs_setup config for swap shouldn't have the trailing '' ;  I  see it
[21:07] <rharper> it's a disk, so cc_disk_setup uses lookup_force_flag and passes in the filesystem type, and swap is not one of the types, so it returns ''
[21:07] <rharper> which is appended to the cmd and fails
[21:09] <blackboxsw> rharper: but it checks             if force_flag: before trying to append ''
[21:09] <rharper> right
[21:09] <blackboxsw> so I think it rejects that
[21:09] <rharper> just saw that
[21:09] <rharper> so... how did it get the extra '' ?
[21:09]  * blackboxsw was wondering about fs_opts being non-zero
[21:09] <rharper> I have a unittest that does not fail
[21:09] <rharper> fs_cfg.get('extra_opts', [])
[21:09] <blackboxsw> but I can't see where extra_opts comes from
[21:09] <blackboxsw> yeah
[21:09] <rharper> yeah, this is very strange
[21:10] <blackboxsw> minfrin: do you see a log in /var/log/cloud-init.log like "Using cmd: " just after a log "Creating file system %s on %s"
[21:11] <blackboxsw> minfrin: actually if possible. please attach the tarfile of 'cloud-init collect-logs' to your bug https://bugs.launchpad.net/cloud-init/+bug/1862417
[21:11] <ubot5> Ubuntu bug 1862417 in cloud-init "cloud-init: Attempt to mkswap on a partition fails: invalid block count argument: ''" [Undecided,New]
[21:11] <blackboxsw> then we can check it out
[21:13] <blackboxsw> ohh well I guess that's reported anyway in Command: ['/sbin/mkswap', '/dev/xvdg', '-L', 'swap', '']
[21:13] <rharper> blackboxsw: well, I'd like to confirm the while log and the user-data that was provided ...
[21:13] <rharper> I'm just not seeing how we can get an extra element in the code path
[21:13] <blackboxsw> yeah in either case the logs would help
[21:13] <blackboxsw> yeah same
[21:22] <minfrin> I looked in the source, and the only reference to mkswap is here: https://github.com/canonical/cloud-init/blob/06e324ff8edb3126e5a8060757a48ceab2b1a121/cloudinit/config/cc_mounts.py#L264
[21:23] <minfrin> This is the version we are using:
[21:23] <minfrin> ii  cloud-init                        19.4-33-gbb4131a2-0ubuntu1~18.04.1  all          Init scripts for cloud instances
[21:23] <blackboxsw> minfrin: and here: https://github.com/canonical/cloud-init/blob/master/cloudinit/config/cc_disk_setup.py#L971
[21:24] <blackboxsw> that gets constructed becauase mkfs.swap utiltiy doesn't exist
[21:24] <rharper> im testing the supplied config in openstack instanc enow
[21:24] <blackboxsw> this is the module where  the problem lies
[21:24] <blackboxsw> cc_disk_setup.py somehow
[21:24] <rharper> cc_disk_setup
[21:25] <blackboxsw> yeah somewhere in https://github.com/canonical/cloud-init/blob/master/cloudinit/config/cc_disk_setup.py#L984-L996
[21:25] <blackboxsw> where we are getting another '' injected
[21:25] <rharper> 2020-02-07 21:25:08,540 - cc_disk_setup.py[WARNING]: Force flag for swap is unknown.
[21:25] <rharper> 2020-02-07 21:25:08,541 - cc_disk_setup.py[DEBUG]: Creating file system swap on /dev/vdc
[21:25] <rharper> 2020-02-07 21:25:08,541 - cc_disk_setup.py[DEBUG]:      Using cmd: ['/sbin/mkswap', '/dev/vdc', '-L', 'swap']
[21:25] <rharper> 2020-02-07 21:25:08,541 - util.py[DEBUG]: Running command ['/sbin/mkswap', '/dev/vdc', '-L', 'swap'] with allowed return codes [0] (shell=False, capture=True)
[21:25] <rharper> 2020-02-07 21:25:08,627 - util.py[DEBUG]: Creating fs for /dev/vdc took 0.103 seconds
[21:27] <minfrin> I updated https://bugs.launchpad.net/cloud-init/+bug/1862417 with the full log.
[21:27] <ubot5> Ubuntu bug 1862417 in cloud-init "cloud-init: Attempt to mkswap on a partition fails: invalid block count argument: ''" [Medium,Incomplete]
[21:27] <blackboxsw> thanks minfrin
[21:27] <blackboxsw> and success on rharper's side.
[21:27] <blackboxsw> hrm
[21:28] <rharper> minfrin: at a min, the swap: config needs to go
[21:28] <rharper> it writes a file on top of the block device; which messes things up
[21:31] <minfrin> Removing the swap section still shows the mkswap with the extra empty string at the end.
[21:31] <rharper> yeah, that's the part I don't get
[21:31] <rharper> so somethings disconnected between what you're running/user-data and cloud-init code
[21:34] <minfrin> Github's search is not that great, I stumbled on this PR: https://github.com/canonical/cloud-init/pull/143
[21:34] <minfrin> Looks like a fix about a month ago.
[21:35] <rharper> but you show 19.4-33
[21:36] <rharper> which has the fix
[21:36] <rharper> but maybe, you don't have that version ?
[21:36] <rharper> which would explain the failure
[21:37] <minfrin> The machine I'm working on uses cloud-init to update itself, it might only have the fix after the updates.
[21:37] <rharper> ah, interesting
[21:37] <rharper> cat /etc/cloud/build.info
[21:37] <rharper> that'll give us a point in time for which version you have
[21:38] <rharper> and I suspect you're right, the top of your cloud-init.log will the original version
[21:38] <minfrin> build_name: serverserial: 20190514
[21:38] <minfrin> Definitely way older than the PR.
[21:38] <rharper> yep
[21:39] <minfrin> I suspect the fix in our case is to use the latest image of Ubuntu from end Jan 2020.
[21:39] <rharper> yep
[21:39] <rharper> and drop the swap: section;
[21:39] <rharper> otherwise, things look good on my recent images with similar cloud-config
[21:43] <rharper> https://paste.ubuntu.com/p/y8JBCrfK9K/
[22:00] <Odd_Bloke> rharper: Is this the force flag issue that was fixed recently?
[22:00] <Odd_Bloke> Oh, I was scrolled up.
[22:00] <rharper> Odd_Bloke: indeed it was
[22:00] <Odd_Bloke> < rharper> Yes.
[22:01] <Odd_Bloke> :)
[22:01] <rharper> minfrin: found the PR and we confirmed the image in use was out of date
[22:01] <rharper> and we can have a PR to update the lookup_flags to add the -f
[22:01] <rharper> for fstype swap
[22:06] <minfrin> Thanks for the help getting to the bottom of this, I appreciate it.
[22:16] <rharper> minfrin: yw
[22:29] <minfrin> Managed to confirm - very latest Ubuntu Bionix image is fixed:
[22:29] <minfrin> 2020-02-07 22:21:09,266 - cc_disk_setup.py[WARNING]: Force flag for swap is unknown.2020-02-07 22:21:09,266 - cc_disk_setup.py[DEBUG]: Creating file system swap on /dev/xvdg2020-02-07 22:21:09,266 - cc_disk_setup.py[DEBUG]:      Using cmd: ['/sbin/mkswap', '/dev/xvdg', '-L', 'swap']2020-02-07 22:21:09,266 - util.py[DEBUG]: Running command
[22:29] <minfrin> ['/sbin/mkswap', '/dev/xvdg', '-L', 'swap'] with allowed return codes [0] (shell=False, capture=True)2020-02-07 22:21:09,290 - util.py[DEBUG]: Creating fs for /dev/xvdg took 0.058 seconds
[22:31] <rharper> minfrin: nice!
[22:32] <rharper> https://github.com/canonical/cloud-init/pull/207
[22:32] <rharper> I put that up for the warning