blackboxsw | aciba: I think I got your comments on PR 1572 | 16:22 |
---|---|---|
aciba | blackboxsw: approved, thanks! | 16:36 |
blackboxsw | https://github.com/canonical/cloud-init/pull/1574 merged. holmanb is https://github.com/canonical/cloud-init/pull/1585 ready for review? | 17:56 |
ubottu | Pull 1574 in canonical/cloud-init "sources/azure: refactor chassis asset tag handling" [Merged] | 17:56 |
ubottu | Pull 1585 in canonical/cloud-init "Add btrfs and lvm support to lxd storage options (SC-1026)" [Open] | 17:56 |
holmanb | @blackboxsw: yes I believe so. Unit test coverage now includes package install, command used, and additionally the get_package function coverage per requests. Integration tests planned to be a separate PR. | 18:00 |
blackboxsw | thanks holmanb: on it and that works for me. integration tests there will be significantly "bigger". Also, will there by time to review https://github.com/canonical/cloud-init/pull/1581 today? If we have closure on that from cloud-init it'll unblock the subiquity-related PR that'll stuff a run-parts script in that directory. | 18:01 |
ubottu | Pull 1581 in canonical/cloud-init "UC015.U1.1 clean: allow third party cleanup scripts in /etc/cloud/clean.d" [Open] | 18:01 |
falcojr | blackboxsw holmanb : I can review that one too | 18:01 |
blackboxsw | what's holding up the subiquity-part is just confirmation about whether we think /etc/cloud/clean.d is the directory we want to use for this. (or somewhere else) | 18:01 |
falcojr | blackboxsw: I just commented on your other subiquity PR to | 18:02 |
blackboxsw | falcojr: thanks on 1592 as well. looks like we'll wrap that up next week | 18:02 |
holmanb | @blackboxsw: yep, I'll prioritize that | 18:03 |
holmanb | to be clear: prioritize #1581 | 18:03 |
blackboxsw | holmanb: great. looks like Dan is good from subiquity side for the related PR to 1581 https://github.com/canonical/subiquity/pull/1347#issuecomment-1185757100 | 18:04 |
ubottu | Pull 1347 in canonical/subiquity "cloud_init_files: render a clean script to /etc/cloud/clean.d" [Open] | 18:04 |
blackboxsw | I'll manually drive integration test of both (painful.... takes about 45 mins for me) | 18:05 |
blackboxsw | falcojr: which PR? | 18:06 |
falcojr | blackboxsw: autoinstall, 1572 | 18:06 |
* blackboxsw checks github emails as the PRs themselves aren't showing me comments in the UI | 18:06 | |
blackboxsw | ahh cloud-init repo not subiquity repo PRs | 18:07 |
falcojr | ahh, sorry | 18:07 |
blackboxsw | @falcojr think it's worth augmenting an integration test for passing opaque autoinstall and ensuring the snap is installed? | 18:11 |
blackboxsw | just so the live logic of the cc_ubuntu_autoinstall module is exercised (even though it basically noops beyond schema validation) | 18:11 |
falcojr | blackboxsw: sure, couldn't hurt | 18:12 |
blackboxsw | a bit of extra time. maybe we'd hit snap install timeout issues... but looks like we aready snap install hello-world in test_combined. I'll just add subiquity/autoinstall checks there too | 18:16 |
falcojr | sounds good | 18:16 |
blackboxsw | falcojr: integration test up. I can't add it to test_combined.py because that is a test_common tests are broad than ubuntu.... so I don't want to dump ubuntu_autoinstall tests into that too. I'm waiting on integration test results in azure now | 18:48 |
blackboxsw | to make sure I don't have to rework the test assertions | 18:49 |
blackboxsw | test needs a snap install subiquity --classic | 18:54 |
falcojr | sounds good, I'll take a look | 19:09 |
falcojr | blackboxsw: test looks good. Did you see my question about frequency though? | 19:18 |
blackboxsw | falcojr: twas missed, thx fixed/pushed. | 19:27 |
blackboxsw | falcojr: going through https://github.com/canonical/cloud-init/pull/1590/files now nice use of features. that'll make things simpler for downstream pkging | 19:36 |
ubottu | Pull 1590 in canonical/cloud-init "cc_set_passwords fixes (SC-1183)" [Open] | 19:36 |
blackboxsw | holmanb: +1 on https://github.com/canonical/cloud-init/pull/1585 for you with or without that nit integration test suggestion. I tested both lvm and btrfs storage_backends and both don't fail. But since you are working a specific integration test separately, maybe no need to touch 1585. You're call to merge or update/merge | 19:42 |
ubottu | Pull 1585 in canonical/cloud-init "Add btrfs and lvm support to lxd storage options (SC-1026)" [Open] | 19:42 |
blackboxsw | falcojr: does #1592 really have to land before 1590? | 20:08 |
blackboxsw | they don't seem strictly dependent | 20:09 |
falcojr | blackboxsw: no, I suppose not...but I thought I could use 1590 to fix whatever else might need to be fixed if 1592 doesn't work | 20:33 |
blackboxsw | falcojr: makes sense, content/behavior looks good to me on 1590. I was wondering if it's worth moving forward on 1590 and sorting any missing issues in 1592 or whatever needs to follow if the hashed_passwd approach doesn't resolve things. | 20:35 |
falcojr | blackboxsw: sure, I think that's fine | 20:49 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!