| prometheanfire | gah, cloud-init is now dieing because it thinks another network device should use dhcp : | 00:16 |
|---|---|---|
| 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:39 |
| niluje | unittests are passing | 09:40 |
| === sambetts|afk is now known as sambetts | ||
| === shardy is now known as shardy_lunch | ||
| === shardy_lunch is now known as shardy | ||
| smoser | niluje, yes, merge proposal on launchpad would be good. | 13:05 |
| smoser | and that looks good. | 13:05 |
| niluje | okay | 13:08 |
| niluje | time lost because of launchpad: too much | 13:08 |
| niluje | :p | 13:08 |
| smoser | it shouldnt be that bad really. | 13:09 |
| niluje | it's better since you use git :) | 13:22 |
| 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:33 |
| 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:34 |
| smoser | you *do* have write access to ~jcastets/cloud-init | 14:35 |
| niluje | oh | 14:35 |
| smoser | a "fork" button would be nice | 14:36 |
| smoser | (on launchpad). | 14:36 |
| niluje | yeyyy | 14:36 |
| niluje | yes | 14:36 |
| 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:37 |
| 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:38 |
| 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:39 |
| 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:40 |
| === rangerpbzzzz is now known as rangerpb | ||
| niluje | what do I need to do to make a pull/merge request? | 14:41 |
| nacc | niluje: from the link you had -- propose for merging? | 14:42 |
| 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:44 |
| 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:45 |
| ubot5 | Ubuntu bug 1684869 in cloud-init "growing root partition does not always work with root=PARTUUID=" [Medium,Confirmed] | 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:46 |
| 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:47 |
| 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:49 |
| smoser | or at least have an option '--uefi' or something. | 14:59 |
| blackboxsw | +1 on that thought smoser maybe --uefi param | 15:02 |
| blackboxsw | we (Canonical) are the only consumers of those tools right? | 15:02 |
| smoser | blackboxsw, yeah. we "you and me" | 15:05 |
| smoser | niluje, i just put a comment on your mp. | 15:06 |
| smoser | what do you think ? | 15:06 |
| smoser | applied, that makes your change much shorter :) http://paste.ubuntu.com/24587355/ | 15:07 |
| 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:09 |
| 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:10 |
| 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:11 |
| 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:17 |
| 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:18 |
| niluje | ok | 15:20 |
| niluje | so let me push --force with your suggestion then | 15:20 |
| 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:36 |
| smoser | as i will squash when i merge (and use the commit message from the MP) | 15:37 |
| 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:41 |
| niluje | smoser: I was running the unittests, just pushed --force you can pull | 15:43 |
| smoser | niluje, htanks | 15:43 |
| rharper | smoser: ok | 15:43 |
| niluje | thanks for the merge and for your patience smoser | 15:52 |
| smoser | niluje, thanksk for contributing | 15:57 |
| niluje | I hope the scaleway datasource will be finished in the following days | 15:57 |
| 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:00 |
| smoser | niluje, yeah, i'm not sure what the best way to do this is. | 16:32 |
| 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:33 |
| blackboxsw | powersj: what is the intent behind -n versus --build-os for run_tree? | 16:54 |
| 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:56 |
| 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:57 |
| blackboxsw | so why would you build a deb in a different os than you would run tests on? | 16:59 |
| blackboxsw | (maybe in the case where our build environment is limited to xenial doesn't support new series X yet?) | 17:00 |
| 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:04 |
| powersj | for the case where someone has multiple -n arguments for example | 17:05 |
| 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:06 |
| blackboxsw | yes though we have documented it :) -- tree_run --verbose \ --os-name xenial --os-name stretch | 17:07 |
| blackboxsw | maybe we just drop the xenial example from the docs until that feature is supported | 17:08 |
| blackboxsw | ok I'm feeling better about it. had to re-read the docs. thanks. | 17:12 |
| powersj | :) | 17:20 |
| 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:32 |
| smoser | rharper, https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/324126 | 17:47 |
| blackboxsw | powersj: ahh right forgot about that | 17:55 |
| 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:56 |
| blackboxsw | hmm wait a sec. I don't see it ATM. lemme dig it up | 17:57 |
| blackboxsw | tools/make_tarball | 17:57 |
| 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 | 17:58 |
| 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:01 |
| dpb1_ | smoser: reading a couple things from rharper and I need some clarification -- what is "the event json object" | 18:02 |
| powersj | blackboxsw: and it looks like I need to fix all the doc strings... :\ | 18:04 |
| 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:06 |
| 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:07 |
| 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:10 |
| 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:11 |
| 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:12 | |
| rharper | smoser: will re-read | 18:33 |
| 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:35 |
| 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 | 18:36 |
| 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:20 |
| powersj | blackboxsw: haha thanks | 19:23 |
| 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:50 |
| blackboxsw | thanks powersj | 19:51 |
| smoser | https://code.launchpad.net/~powersj/cloud-init/+git/cloud-init/+merge/323351 | 20:13 |
| powersj | smoser: yes | 20:14 |
| smoser | hm.. | 20:14 |
| smoser | ah. | 20:14 |
| smoser | i see you saw it. | 20:14 |
| 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:15 |
| powersj | so I made a quick silly change to force it and reverted and the diff got generated | 20:16 |
| smoser | oh. funny. | 20:17 |
| smoser | blackboxsw, you think https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/323076 is ready, r ight? | 20:38 |
| 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:40 |
| 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:41 | |
| 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:42 |
| smoser | rharper, so what was the bug ? | 20:44 |
| rharper | #1685944 | 20:45 |
| rharper | lp:1685944 | 20:45 |
| rharper | bug 1685944 | 20:45 |
| ubot5 | 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 |
| rharper | there we go | 20:45 |
| 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:46 |
| 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:47 |
| 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:49 |
| smoser | in tools/net-convert.py | 20:50 |
| smoser | i' going to revert yours now. and then i have to run | 20:50 |
| rharper | ack | 20:51 |
| * 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 | 20:52 | |
| === rangerpb is now known as rangerpbzzzz | ||
| 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:51 |
| 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:52 |
| powersj | ok | 21:53 |
| 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:18 |
| blackboxsw | thx for the email ptr. | 22:21 |
| powersj | https://paste.ubuntu.com/24589541/ | 22:29 |
| powersj | ok added e2fsprogs to deps list for setting up container that seems to have fixed it :D | 22:36 |
| powersj | CentOS 6 is good; 7 not so much https://paste.ubuntu.com/24589593/ seems hard coded path for mkfs.ext4 | 22:39 |
| blackboxsw | wow powersj you redid all docstrings | 22:50 |
| blackboxsw | thanks man | 22:50 |
| blackboxsw | I love it | 22:50 |
| powersj | blackboxsw: np figured now is the time | 22:53 |
| powersj | I did use pydocstyle to test them too | 22:53 |
| powersj | few are incomplete in terms of args as I only took what was there and updated it. | 22:54 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
