[00:16] <prometheanfire> gah, cloud-init is now dieing because it thinks another network device should use dhcp :
[09:39] <niluje> smoser: https://github.com/brmzkw/cloud-init-scaleway/commit/881495e7930dacf5a7195e560ec66589e44b55df
[09:39] <niluje> want me to make a pull request with that on launchpad?
[09:39] <niluje> sorry, it took me a long time to make the patch :<
[09:40] <niluje> unittests are passing
[13:05] <smoser> niluje, yes, merge proposal on launchpad would be good.
[13:05] <smoser> and that looks good.
[13:08] <niluje> okay
[13:08] <niluje> time lost because of launchpad: too much
[13:08] <niluje> :p
[13:09] <smoser> it shouldnt be that bad really.
[13:22] <niluje> it's better since you use git :)
[14:33] <smoser> niluje, did you make one ?
[14:33] <smoser> what is your launchpad user name ?
[14:33] <niluje> smoser: jcastets, I'm trying to figure out how to push on launchpad
[14:33] <niluje> # git push lp fix-net-cfg -v
[14:33] <niluje> Pushing to git+ssh://jcastets@git.launchpad.net/cloud-init
[14:33] <niluje> fatal: remote error: Permission denied.
[14:34] <smoser>     git remote add LP_USER ssh://LP_USER@git.launchpad.net/~LP_USER/cloud-init
[14:34] <smoser>     git push LP_USER master
[14:34] <smoser> (from HACKING.rst)
[14:34] <smoser> the key thing being you don't have write access to /cloud-init
[14:35] <smoser> you *do* have write access to ~jcastets/cloud-init
[14:35] <niluje> oh
[14:36] <smoser> a "fork" button would be nice
[14:36] <smoser> (on launchpad).
[14:36] <niluje> yeyyy
[14:36] <niluje> yes
[14:37] <niluje> except that lp is not github, I don't see other advantages of it
[14:37] <niluje> it's a pain to use, the interface is un-understandable
[14:38] <niluje> like I don't see my git repository here: https://code.launchpad.net/~jcastets
[14:38] <niluje> but I see it here: https://code.launchpad.net/~jcastets/cloud-init
[14:38] <nacc> niluje: https://code.launchpad.net/~jcastets/+git
[14:39] <nacc> niluje: note bzr is the default still
[14:39] <niluje> okay
[14:39] <nacc> niluje: that will change at some point
[14:39] <nacc> (sooner rather than later hopefully)
[14:39] <niluje> I guess it's a mess because I had a copy of cloud-init with bzr
[14:39] <niluje> anyway here's my branch https://code.launchpad.net/~jcastets/cloud-init/+git/cloud-init/+ref/fix-net-cfg
[14:40] <smoser> niluje, there is also https://git.launchpad.net/~jcastets/cloud-init
[14:40] <smoser> but that doesnt show review
[14:40] <smoser> s
[14:41] <niluje> what do I need to do to make a pull/merge request?
[14:42] <nacc> niluje: from the link you had -- propose for merging?
[14:44] <niluje> correct, thanks
[14:44] <niluje> https://code.launchpad.net/~jcastets/cloud-init/+git/cloud-init/+merge/324114
[14:44] <niluje> here it is
[14:44] <smoser> niluje, http://cloudinit.readthedocs.io/en/latest/topics/hacking.html *should* cover this all :)
[14:44] <smoser> if you read it and it didn't make sense, help me improve it
[14:44] <niluje> ok
[14:45] <niluje> You can see a web view of your repository and navigate to the branch at:
[14:45] <niluje> https://code.launchpad.net/~LP_USER/cloud-init/
[14:45] <niluje> +git is missing then, don't you think?
[14:45] <blackboxsw> 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] <smoser> https://code.launchpad.net/~jcastets/cloud-init/
[14:46] <smoser> that shows me git
[14:46] <smoser> niluje, it doesnt for you?
[14:46] <niluje> smoser: git *and* lp
[14:46] <niluje> http://blackhole.brmzkw.info/2017-05-16/launchpad.png
[14:47] <smoser> hm.. i dont know why it shows the "other repositories' there.
[14:47] <smoser> but you click on the branc 'fix-net-cfg' there, and then click 'Propose for merging'
[14:47] <niluje> yeah anyway the "other repositories" is shown even if +git is in the url
[14:47] <niluje> I just did smoser : https://code.launchpad.net/~jcastets/cloud-init/+git/cloud-init/+merge/324114
[14:49] <smoser> 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] <smoser> or at least have an option '--uefi' or something.
[15:02] <blackboxsw> +1 on that thought smoser maybe --uefi param
[15:02] <blackboxsw> we (Canonical) are the only consumers of those tools right?
[15:05] <smoser> blackboxsw, yeah. we "you and me"
[15:06] <smoser> niluje, i just put a comment on your mp.
[15:06] <smoser> what do you think ?
[15:07] <smoser> applied, that makes your change much shorter :) http://paste.ubuntu.com/24587355/
[15:09] <niluje> 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] <niluje> in the unittests, the file in DHCP_CONTENT_1 contains PROTO='dhcp' ; IPV4ADDR='192.168....'
[15:10] <niluje> I guess it doesn't really make sense do have both and a valid /run/net-<iface>.cfg file won't ever contain such config
[15:10] <niluje> but if it does, you probably want to have the addr in the cloud-init config object
[15:11] <niluje> 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] <smoser> niluje, well, the files are what klibc's ipconfig generates
[15:17] <smoser> ie, they are real world example
[15:17] <smoser> so i'd like to keep them as they are, but we just ignore the setting of 'address' on dhcp types.
[15:18] <smoser> 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] <niluje> ok
[15:20] <niluje> so let me push --force with your suggestion then
[15:36] <smoser> niluje, you didn't do that , id dyou ?
[15:36] <smoser> please do and then i'll pull.
[15:36] <smoser> and, you dont need to --force if you do not want
[15:37] <smoser> as i will squash when i merge (and use the commit message from the MP)
[15:41] <smoser> rharper, i'd appreciate your quick-ish review of https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/323420
[15:41] <smoser> i *think* your comments are all addressed.
[15:43] <niluje> smoser: I was running the unittests, just pushed --force you can pull
[15:43] <smoser> niluje, htanks
[15:43] <rharper> smoser: ok
[15:52] <niluje> thanks for the merge and for your patience smoser
[15:57] <smoser> niluje, thanksk for contributing
[15:57] <niluje> I hope the scaleway datasource will be finished in the following days
[16:00] <niluje> 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-<iface>.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-<iface>.cfg file from the initrd.
[16:32] <smoser> niluje, yeah, i'm not sure what the best way to do this is.
[16:33] <smoser> as it is right now, kernel command line overrides the datasource configuration.
[16:33] <smoser> so your datasource would be "trumped" by the cmdline
[16:54] <blackboxsw> powersj: what is the intent behind -n versus --build-os for run_tree?
[16:56] <powersj> -n, aka --os-name, is the OS you wish to run tests on
[16:56] <powersj> --build-os is what OS you want to build the deb on
[16:57] <blackboxsw> 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] <blackboxsw> as defined in tests/cloud_tests/args.py
[16:59] <blackboxsw> so why would you build a deb in a different os than you would run tests on?
[17:00] <blackboxsw> (maybe in the case where our build environment is limited to xenial doesn't support new series X yet?)
[17:04] <powersj> exactly
[17:04] <powersj> We don't exactly have the logic in there either to build for multiple OSes in a single run either
[17:05] <powersj> for the case where someone has multiple -n arguments for example
[17:06] <blackboxsw> 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] <blackboxsw> yes though we have documented it :)     -- tree_run --verbose \ --os-name xenial --os-name stretch
[17:08] <blackboxsw> maybe we just drop the xenial example from the docs until that feature is supported
[17:12] <blackboxsw> ok I'm feeling better about it. had to re-read the docs. thanks.
[17:20] <powersj> :)
[17:32] <powersj> 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] <smoser> rharper, https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/324126
[17:55] <blackboxsw> powersj: ahh right forgot about that
[17:56] <powersj> blackboxsw: you mentioned "SKIP_UNCOMMITTED_CHANGES_CHECK", where is that?
[17:56] <blackboxsw> most of the comments are cosmetic at the moment. trying to wrap up the logic/code review
[17:56] <blackboxsw> powersj: completely undocumented :/
[17:57] <blackboxsw> hmm wait a sec. I don't see it ATM. lemme dig it up
[17:57] <blackboxsw> tools/make_tarball
[17:58] <blackboxsw> tools/make-tarball.
[17:58] <blackboxsw> which I think is what barfs when you have uncommited changes local in the tree while running make deb
[17:58] <blackboxsw> or bddeb as it were
[18:01] <powersj> right, so we tar up the working tree, push to builder, extract, commit everything, and run bddeb
[18:01] <powersj> I don't see in that process where we would want to use that check?
[18:02] <dpb1_> smoser: reading a couple things from rharper and I need some clarification -- what is "the event json object"
[18:04] <powersj> blackboxsw: and it looks like I need to fix all the doc strings... :\
[18:06] <blackboxsw> 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] <smoser> dpb1_, probably a json formatrted description of the event (like a hotplug attach of a disk)
[18:07] <blackboxsw> 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] <powersj> we should add a pep257 test then too
[18:07] <blackboxsw> not a bad check for new changes
[18:10] <smoser> blackboxsw, i think you're suggesting that all unit tests should have docstrings that adhear to that pep.
[18:10] <smoser> i'm not opposed to that.
[18:10] <powersj> 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] <blackboxsw> 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] <dpb1_> 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] <rharper> smoser: will re-read
[18:35] <rharper> 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] <rharper> dpb1_: cloudinit/reporting/handlers:WebHookHandler 's publish_event takes the event object, and json.dumps(object.as_dict())
[18:36] <dpb1_> rharper: I'm making changes to the doc, fyi
[18:36] <dpb1_> in flight
[18:36] <rharper> ack
[19:20] <blackboxsw> 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] <blackboxsw> man that hurt my brain.
[19:23] <powersj> blackboxsw: haha thanks
[19:50] <powersj> 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] <blackboxsw> thanks powersj
[20:13] <smoser> https://code.launchpad.net/~powersj/cloud-init/+git/cloud-init/+merge/323351
[20:14] <powersj> smoser: yes
[20:14] <smoser> hm..
[20:14] <smoser> ah.
[20:14] <smoser> i see you saw it.
[20:15] <smoser>  initialisation
[20:15] <smoser> you had a commit that changed that to a z and then back to an s
[20:15] <smoser> i didn't realize there was a revert
[20:15] <powersj> yeah launchpad was being silly and not generating a diff
[20:16] <powersj> so I made a quick silly change to force it and reverted and the diff got generated
[20:17] <smoser> oh. funny.
[20:38] <smoser> blackboxsw, you think https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/323076 is ready, r ight?
[20:40] <rharper> 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] <smoser> ugh.
[20:41] <rharper> 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] <smoser> bah
[20:42] <blackboxsw> 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] <smoser> k
[20:44] <smoser> rharper, so what was the bug ?
[20:45] <rharper> #1685944
[20:45] <rharper> lp:1685944
[20:45] <rharper> bug 1685944
[20:45] <rharper> there we go
[20:46] <smoser> rharper, yeah, but what *was* wrong
[20:46] <rharper> the whole patch
[20:46] <smoser> oh. ther netplan renderer. huck
[20:46] <rharper> netplan.py should call with the right order
[20:46] <rharper> smoser: xnox has a branch to fix
[20:46] <rharper> lemme find
[20:47] <rharper> https://code.launchpad.net/~xnox/cloud-init/+git/cloud-init/+merge/324012
[20:47] <rharper> 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] <rharper> 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] <smoser> so eseentially we should grab xnox
[20:49] <smoser> and revert yours
[20:49] <smoser> right?
[20:49] <smoser> xnox's branch
[20:49] <smoser> plus
[20:49] <smoser> -    r.render_network_state(args.directory, ns)
[20:49] <smoser> +    r.render_network_state(ns, target=args.directory)
[20:50] <smoser> in tools/net-convert.py
[20:50] <smoser> i' going to revert yours now. and then i have to run
[20:51] <rharper> ack
[20:52]  * rharper steps out for the vet
[20:52] <smoser> reverting && toxing && git pushing
[20:52] <smoser> then tomnorrow get the others
[20:52]  * smoser runs
[21:51] <powersj> blackboxsw: https://code.launchpad.net/~powersj/cloud-init/+git/cloud-init/+merge/324136
[21:51] <powersj> new merge request with fresh diff and updates to doc strings
[21:51] <powersj> updated util functions as well and got rid of silly functions that you called out.
[21:51] <powersj> I need to run through the tests again and then I'll mark as "needs review"
[21:52] <blackboxsw> 6732 lines (+2526/-1200) :)
[21:52] <blackboxsw> heh
[21:52] <blackboxsw> wow
[21:52] <blackboxsw> I'll browse through it now powersj to see if my other comments were handled thanks !
[21:53] <powersj> ok
[22:18] <blackboxsw> 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] <powersj> blackboxsw: see my email on snappy failures
[22:18] <powersj> but the centos 6 tests just failed because of new tests... sigh
[22:21] <blackboxsw> thx for the email ptr.
[22:29] <powersj> https://paste.ubuntu.com/24589541/
[22:36] <powersj> ok added e2fsprogs to deps list for setting up container that seems to have fixed it :D
[22:39] <powersj> CentOS 6 is good; 7 not so much https://paste.ubuntu.com/24589593/ seems hard coded path for mkfs.ext4
[22:50] <blackboxsw> wow powersj you redid all docstrings
[22:50] <blackboxsw> thanks man
[22:50] <blackboxsw> I love it
[22:53] <powersj> blackboxsw: np figured now is the time
[22:53] <powersj> I did use pydocstyle to test them too
[22:54] <powersj> few are incomplete in terms of args as I only took what was there and updated it.