[00:16] gah, cloud-init is now dieing because it thinks another network device should use dhcp : [09:39] smoser: https://github.com/brmzkw/cloud-init-scaleway/commit/881495e7930dacf5a7195e560ec66589e44b55df [09:39] want me to make a pull request with that on launchpad? [09:39] sorry, it took me a long time to make the patch :< [09:40] unittests are passing === sambetts|afk is now known as sambetts === shardy is now known as shardy_lunch === shardy_lunch is now known as shardy [13:05] niluje, yes, merge proposal on launchpad would be good. [13:05] and that looks good. [13:08] okay [13:08] time lost because of launchpad: too much [13:08] :p [13:09] it shouldnt be that bad really. [13:22] it's better since you use git :) [14:33] niluje, did you make one ? [14:33] what is your launchpad user name ? [14:33] smoser: jcastets, I'm trying to figure out how to push on launchpad [14:33] # git push lp fix-net-cfg -v [14:33] Pushing to git+ssh://jcastets@git.launchpad.net/cloud-init [14:33] fatal: remote error: Permission denied. [14:34] git remote add LP_USER ssh://LP_USER@git.launchpad.net/~LP_USER/cloud-init [14:34] git push LP_USER master [14:34] (from HACKING.rst) [14:34] the key thing being you don't have write access to /cloud-init [14:35] you *do* have write access to ~jcastets/cloud-init [14:35] oh [14:36] a "fork" button would be nice [14:36] (on launchpad). [14:36] yeyyy [14:36] yes [14:37] except that lp is not github, I don't see other advantages of it [14:37] it's a pain to use, the interface is un-understandable [14:38] like I don't see my git repository here: https://code.launchpad.net/~jcastets [14:38] but I see it here: https://code.launchpad.net/~jcastets/cloud-init [14:38] niluje: https://code.launchpad.net/~jcastets/+git [14:39] niluje: note bzr is the default still [14:39] okay [14:39] niluje: that will change at some point [14:39] (sooner rather than later hopefully) [14:39] I guess it's a mess because I had a copy of cloud-init with bzr [14:39] anyway here's my branch https://code.launchpad.net/~jcastets/cloud-init/+git/cloud-init/+ref/fix-net-cfg [14:40] niluje, there is also https://git.launchpad.net/~jcastets/cloud-init [14:40] but that doesnt show review [14:40] s === rangerpbzzzz is now known as rangerpb [14:41] what do I need to do to make a pull/merge request? [14:42] niluje: from the link you had -- propose for merging? [14:44] correct, thanks [14:44] https://code.launchpad.net/~jcastets/cloud-init/+git/cloud-init/+merge/324114 [14:44] here it is [14:44] niluje, http://cloudinit.readthedocs.io/en/latest/topics/hacking.html *should* cover this all :) [14:44] if you read it and it didn't make sense, help me improve it [14:44] ok [14:45] You can see a web view of your repository and navigate to the branch at: [14:45] https://code.launchpad.net/~LP_USER/cloud-init/ [14:45] +git is missing then, don't you think? [14:45] thanks smoser for the xenial SRU closeout on https://bugs.launchpad.net/cloud-init/+bug/1684869 I'm reading through your changes now [14:46] Ubuntu bug 1684869 in cloud-init "growing root partition does not always work with root=PARTUUID=" [Medium,Confirmed] [14:46] https://code.launchpad.net/~jcastets/cloud-init/ [14:46] that shows me git [14:46] niluje, it doesnt for you? [14:46] smoser: git *and* lp [14:46] http://blackhole.brmzkw.info/2017-05-16/launchpad.png [14:47] hm.. i dont know why it shows the "other repositories' there. [14:47] but you click on the branc 'fix-net-cfg' there, and then click 'Propose for merging' [14:47] yeah anyway the "other repositories" is shown even if +git is in the url [14:47] I just did smoser : https://code.launchpad.net/~jcastets/cloud-init/+git/cloud-init/+merge/324114 [14:49] blackboxsw, it might make sense to have get-proposed-cloudimg get the -uefi for xenial. since it actually is the same as the yakkety+ ones are for 'disk.img' [14:59] or at least have an option '--uefi' or something. [15:02] +1 on that thought smoser maybe --uefi param [15:02] we (Canonical) are the only consumers of those tools right? [15:05] blackboxsw, yeah. we "you and me" [15:06] niluje, i just put a comment on your mp. [15:06] what do you think ? [15:07] applied, that makes your change much shorter :) http://paste.ubuntu.com/24587355/ [15:09] smoser: you're much more qualified than me to know what's the best thing to do here, but IMO we could keep my patch but maybe change the unittests [15:10] in the unittests, the file in DHCP_CONTENT_1 contains PROTO='dhcp' ; IPV4ADDR='192.168....' [15:10] I guess it doesn't really make sense do have both and a valid /run/net-.cfg file won't ever contain such config [15:10] but if it does, you probably want to have the addr in the cloud-init config object [15:11] so my suggestion is instead of your patch remove the IPV{4,6}ADDR from the example files when PROTO='dhcp' in the unittests [15:17] niluje, well, the files are what klibc's ipconfig generates [15:17] ie, they are real world example [15:17] so i'd like to keep them as they are, but we just ignore the setting of 'address' on dhcp types. [15:18] as other "normal" ways of saying "please use dhcp" do not provide you with an address, so this way other parts of the code wouldn't unintentionally expect/rely on it. [15:20] ok [15:20] so let me push --force with your suggestion then [15:36] niluje, you didn't do that , id dyou ? [15:36] please do and then i'll pull. [15:36] and, you dont need to --force if you do not want [15:37] as i will squash when i merge (and use the commit message from the MP) [15:41] rharper, i'd appreciate your quick-ish review of https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/323420 [15:41] i *think* your comments are all addressed. [15:43] smoser: I was running the unittests, just pushed --force you can pull [15:43] niluje, htanks [15:43] smoser: ok [15:52] thanks for the merge and for your patience smoser [15:57] niluje, thanksk for contributing [15:57] I hope the scaleway datasource will be finished in the following days [16:00] We set ip=:::::eth0: in the cmdline because otherwise our servers with two network interfaces don't boot (because the default gw needs to be eth0 and not eth1), but unlike regular initrds we don't generate /run/net-.cfg. I don't know if I should spend some time to see how to get rid of ip= from the cmdline or how to generate a valid /run/net-.cfg file from the initrd. [16:32] niluje, yeah, i'm not sure what the best way to do this is. [16:33] as it is right now, kernel command line overrides the datasource configuration. [16:33] so your datasource would be "trumped" by the cmdline [16:54] powersj: what is the intent behind -n versus --build-os for run_tree? [16:56] -n, aka --os-name, is the OS you wish to run tests on [16:56] --build-os is what OS you want to build the deb on [16:57] It seems like the consolidated args we are providing don't quite make sense for all actions I'm not quite sure why BDDEB arg sets are provided as subsets for tree_collect and tree_run [16:57] as defined in tests/cloud_tests/args.py [16:59] so why would you build a deb in a different os than you would run tests on? [17:00] (maybe in the case where our build environment is limited to xenial doesn't support new series X yet?) [17:04] exactly [17:04] We don't exactly have the logic in there either to build for multiple OSes in a single run either [17:05] for the case where someone has multiple -n arguments for example [17:06] ahh I see now you've also commented on the mp. thanks. was just trying to understand why we might want to have --build-os artful -os-name xenial in our tree_runs. [17:07] yes though we have documented it :) -- tree_run --verbose \ --os-name xenial --os-name stretch [17:08] maybe we just drop the xenial example from the docs until that feature is supported [17:12] ok I'm feeling better about it. had to re-read the docs. thanks. [17:20] :) [17:32] blackboxsw: FYI the launchpad diff is old. I keep getting "oops: cannot generate diff" so a few of your changes are not needed [17:47] rharper, https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/324126 [17:55] powersj: ahh right forgot about that [17:56] blackboxsw: you mentioned "SKIP_UNCOMMITTED_CHANGES_CHECK", where is that? [17:56] most of the comments are cosmetic at the moment. trying to wrap up the logic/code review [17:56] powersj: completely undocumented :/ [17:57] hmm wait a sec. I don't see it ATM. lemme dig it up [17:57] tools/make_tarball [17:58] tools/make-tarball. [17:58] which I think is what barfs when you have uncommited changes local in the tree while running make deb [17:58] or bddeb as it were [18:01] right, so we tar up the working tree, push to builder, extract, commit everything, and run bddeb [18:01] I don't see in that process where we would want to use that check? [18:02] smoser: reading a couple things from rharper and I need some clarification -- what is "the event json object" [18:04] blackboxsw: and it looks like I need to fix all the doc strings... :\ [18:06] powersj: smoser if you guys agree w/ docstrings adhering to https://www.python.org/dev/peps/pep-0257/ I don't mind if we leave that out of powersj consolidated branch and I can followup and clean up all the integration tests by turning a crank and fixing docstrings (since I'm the one complaining) [18:07] dpb1_, probably a json formatrted description of the event (like a hotplug attach of a disk) [18:07] it's busy work that needn't block your branch in my mind powersj. It just makes the code more legible/maintainable long term [18:07] we should add a pep257 test then too [18:07] not a bad check for new changes [18:10] blackboxsw, i think you're suggesting that all unit tests should have docstrings that adhear to that pep. [18:10] i'm not opposed to that. [18:10] blackboxsw: "Launchpad encountered an internal error during the following operation: generating the diff for a merge proposal. It was logged with id OOPS-e452bec09711d44ff1a102538807d0e2. Sorry for the inconvenience." [18:11] in my comments on powersj branch, it was just that all methods added ahdere to pep257 single line docstring, blank line, additional details if needed (param descriptions etc). [18:12] smoser: oh, maybe the JSON that is already reported in the fire-and-forget reporting that cloud-init already does [18:12] * dpb1_ is starting to put those pieces together [18:33] smoser: will re-read [18:35] dpb1_: right, so cloudinit/reporting/events.py has the Object; there is a as_dict() method, which is then used in the WebReporter to post to a remote URL handler [18:36] dpb1_: cloudinit/reporting/handlers:WebHookHandler 's publish_event takes the event object, and json.dumps(object.as_dict()) [18:36] rharper: I'm making changes to the doc, fyi [18:36] in flight [18:36] ack [19:20] powersj: ok I've finally finished reviewing your branch. had a suggestion for a couple of the tests/cloud_tests/utils that might take a little time, but we can iterate. Thanks again. I'll peek back at the branch when you say it's ready [19:20] man that hurt my brain. [19:23] blackboxsw: haha thanks [19:50] blackboxsw: the util change was easy. Many calls were already using cloud-init's util functions versus the ones that pass to it. So I removed the test util calls in a few remaining places. [19:51] thanks powersj [20:13] https://code.launchpad.net/~powersj/cloud-init/+git/cloud-init/+merge/323351 [20:14] smoser: yes [20:14] hm.. [20:14] ah. [20:14] i see you saw it. [20:15] initialisation [20:15] you had a commit that changed that to a z and then back to an s [20:15] i didn't realize there was a revert [20:15] yeah launchpad was being silly and not generating a diff [20:16] so I made a quick silly change to force it and reverted and the diff got generated [20:17] oh. funny. [20:38] blackboxsw, you think https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/323076 is ready, r ight? [20:40] smoser: on that net-convert; my MP was backwards; it's the netplan renderer which flipped the args (render_network_state should always take (network_state, target=target) [20:41] ugh. [20:41] my fault, I didn't notice that until i was also net-converting with sysconfig as well [20:41] * rharper puts on the cone of shame [20:42] bah [20:42] smoser: +1 on the make deb target. it's only step one, there will be a followup small branch to extract dependencies from the built package (rpm|deb) files [20:42] k [20:44] rharper, so what was the bug ? [20:45] #1685944 [20:45] lp:1685944 [20:45] bug 1685944 [20:45] bug 1685944 in cloud-init "tools/net-convert: fix argument order for render_network_state" [Low,Fix committed] https://launchpad.net/bugs/1685944 [20:45] there we go [20:46] rharper, yeah, but what *was* wrong [20:46] the whole patch [20:46] oh. ther netplan renderer. huck [20:46] netplan.py should call with the right order [20:46] smoser: xnox has a branch to fix [20:46] lemme find [20:47] https://code.launchpad.net/~xnox/cloud-init/+git/cloud-init/+merge/324012 [20:47] I had suggested to also fix up the unittest callers to pass target=value instead of just state, dir. but that's not strictly required [20:47] I have both of that in a branch for fixing up some sysconfig stuff, but I've not had time to do a PR yet [20:49] so eseentially we should grab xnox [20:49] and revert yours [20:49] right? [20:49] xnox's branch [20:49] plus [20:49] - r.render_network_state(args.directory, ns) [20:49] + r.render_network_state(ns, target=args.directory) [20:50] in tools/net-convert.py [20:50] i' going to revert yours now. and then i have to run [20:51] ack [20:52] * rharper steps out for the vet [20:52] reverting && toxing && git pushing [20:52] then tomnorrow get the others [20:52] * smoser runs === rangerpb is now known as rangerpbzzzz [21:51] blackboxsw: https://code.launchpad.net/~powersj/cloud-init/+git/cloud-init/+merge/324136 [21:51] new merge request with fresh diff and updates to doc strings [21:51] updated util functions as well and got rid of silly functions that you called out. [21:51] I need to run through the tests again and then I'll mark as "needs review" [21:52] 6732 lines (+2526/-1200) :) [21:52] heh [21:52] wow [21:52] I'll browse through it now powersj to see if my other comments were handled thanks ! [21:53] ok [22:18] hrm did integration tests just fail due to the snappy changes smoser landed today? https://jenkins.ubuntu.com/server/job/cloud-init-integration-proposed-z/6/console [22:18] blackboxsw: see my email on snappy failures [22:18] but the centos 6 tests just failed because of new tests... sigh [22:21] thx for the email ptr. [22:29] https://paste.ubuntu.com/24589541/ [22:36] ok added e2fsprogs to deps list for setting up container that seems to have fixed it :D [22:39] CentOS 6 is good; 7 not so much https://paste.ubuntu.com/24589593/ seems hard coded path for mkfs.ext4 [22:50] wow powersj you redid all docstrings [22:50] thanks man [22:50] I love it [22:53] blackboxsw: np figured now is the time [22:53] I did use pydocstyle to test them too [22:54] few are incomplete in terms of args as I only took what was there and updated it.