| === Wulf4 is now known as Wulf | ||
| niluje | thanks smoser :) let's review your review | 08:22 |
|---|---|---|
| * Putti got added to the contributor agreement group! :) | 10:34 | |
| niluje | smoser: I still don't understand anything to the launchpad interface, but all your comments have been addressed :) | 10:37 |
| niluje | I cant rebase my commits into one if you want me to | 10:38 |
| Putti | oh, the tox tests take like 5-10 min to run :/ | 11:12 |
| Putti | they are contacting the outside world? | 11:12 |
| === shardy is now known as shardy_lunch | ||
| Putti | whee, first merge request done. Was kinda tedious – I have to look up if I can integrate it somehow with my command-line flow. | 11:39 |
| niluje | Putti: if you are doing some hacking/debugging | 11:51 |
| niluje | run tox -e <env> | 11:51 |
| niluje | like tox -e py27 | 11:51 |
| niluje | or to run a single test tox -e py27 -- cloudinit/unittest/xxx.py | 11:51 |
| niluje | or tox -e py27 -- cloudinit/unittest/xxx.py:Classname or Classname:test_name | 11:52 |
| Putti | ok | 11:52 |
| niluje | or call nose directly: .tox/py27/bin/python -m nose --tests cloudinit/unittest/xxx/.py -x -s | 11:52 |
| niluje | tox has always been super slow for me for everything | 11:53 |
| Putti | niluje, but maybe huge part of the slowness is just that it contacts all the web servers? | 11:54 |
| Putti | for me the test tries to contact google but fails | 11:56 |
| Putti | maybe the slowness is because of this: "FAIL: test_get_data_returns_false_if_not_on_gce (tests.unittests.test_datasource.test_gce.TestDataSourceGCE)" | 11:57 |
| Putti | I will link the full log, a minute... | 11:58 |
| Putti | http://paste.debian.net/plain/976118 | 11:58 |
| Putti | The ISP I am currently connected redirects to their own web page if the resource is not found, maybe the unit test gets confused by this? | 12:03 |
| niluje | lol | 12:08 |
| niluje | anyway IMO a unittest should never hit a network resource | 12:08 |
| * Putti agrees with that | 12:09 | |
| niluje | but I'm pretty sure they don't | 12:09 |
| Putti | I guess a temporary fix is for me to use different dns. | 12:09 |
| niluje | which env is it? py27? | 12:09 |
| Putti | correct | 12:09 |
| Putti | py27 | 12:09 |
| niluje | sec | 12:10 |
| === shardy_lunch is now known as shardy | ||
| niluje | yeah nevermind | 12:11 |
| niluje | but I guess if you find the place where a network resource is hit, cloud init maintainers will be interested in a fix | 12:13 |
| Putti | not too hard to find: cloud-init/tests/unittests/test_datasource/test_gce.py :) | 12:15 |
| Putti | I might look into how to fix it | 12:15 |
| === mpontillo_ is now known as mpontillo | ||
| powersj | Putti: if you find a test is escaping out to the network please file a bug. However, I am fairly certain nothing attempts to get out. | 14:51 |
| powersj | Those calls should be mocked, as those metadata services should not available outside the cloud providers network | 14:51 |
| Putti | it seems like the GCE test mocks the request but somehow it lets the dns request pass through. | 14:58 |
| blackboxsw | Putti: I can peek at that today too if you don't end up tackling it | 15:01 |
| Putti | blackboxsw, thanks, I'm pretty new to cloud-init and also I have very minimal experience with unit tests in python so if you just have time it would be nice. Though, I'm so curious about this problem, so we will see if I figure out the fix (in case one is needed) for this before you. | 15:04 |
| blackboxsw | Putti: +1 and welcome to the club, I'm relatively new to cloud-init too. Putti appreciate your efforts fire off any questions you might have and we can field them. to speed unit test debug cycles you can put a pdb.set_strace() over in the unit test you care about and tox -e py3 -- --tests tests.unittests.test_datasource.test_gce.TestOpenStackDataSource -s | 15:24 |
| blackboxsw | Putti: +1 and welcome to the club, I'm relatively new to cloud-init too. Putti appreciate your efforts fire off any questions you might have and we can field them. to speed unit test debug cycles you can put a pdb.set_strace() over in the unit test you care about and tox -e py3 -- --tests tests.unittests.test_datasource.test_gce.TestDataSourceGCE -s | 15:24 |
| blackboxsw | put you probably already know that | 15:25 |
| Putti | I actually didn't know that, thanks :) | 15:25 |
| blackboxsw | *but... I need to go find my typo-free keyboad | 15:25 |
| blackboxsw | :) | 15:25 |
| Putti | blackboxsw, I have some ideas for a possible fix. 1. We could mock cloudinit/url_helper.py's readurl() in the test_gce.py so that "allow_redirects=True" will be changed to False. Or the metadata.google.internal domain could be changed to 169.254.169.254 (this seems to be some sort of special ip) like some other datasource tests already do. | 15:32 |
| blackboxsw | Putti: checking that unit test now to see exactly what you mean. | 15:37 |
| Putti | Here is some information about the magic ip address: https://askleo.com/why_cant_i_connect_with_a_169254xx_ip_address/ | 15:40 |
| blackboxsw | right that address, 169.254.169.254, is link local, which means those packets (if they hit your network) shouldn't be forwarded beyond your router. so expectation is that link-local ips will timeout on your local network. But in the interest of expedience in unit tests, instead of long timeouts we should cut those tests off to avoid lengthy timeout waits. (or we should ensure that our timeout length is as | 15:46 |
| blackboxsw | small as possible. | 15:46 |
| blackboxsw | s/cut those tests off/mock those external queries to 169.254.*.* so that the expected response is immediate/ | 15:47 |
| Putti | the question is: how do we do that? | 15:48 |
| Putti | ooh, I see, mocking them | 15:48 |
| Putti | mhm, but aren't they already mocked? It is just dns that is slowing down, and now there is no dns | 15:48 |
| Putti | I mean if we use ip address | 15:49 |
| Putti | Hmm, I wonder if this is something that should be fixed in one of those http request mocking python modules we use. But as a temporary fix for us would be removing host names and use ip addresses instead. | 15:54 |
| blackboxsw | Putti yeah strange about your pastebin. Since that unit test is mocking platform_reports_gce() to return False, I'm not certain why your env is attempting to access metadata.google.internal hostname | 15:56 |
| blackboxsw | hmm Putti that mock doesn't really work it seems. If I put a pdb.set_trace() in cloudinit/sources/DataSourceGCE.py get_data(): | 16:02 |
| Putti | yes it doesn't ... I got a new idea why, a minute! | 16:02 |
| blackboxsw | https://www.irccloud.com/pastebin/1xAbn5DV/ | 16:02 |
| blackboxsw | we shouldn't get there if we .tox/py3/bin/python3 -m nose --tests tests/unittests/test_datasource/test_gce.py:TestDataSourceGCE.test_get_data_returns_false_if_not_on_gce -x -s | 16:02 |
| blackboxsw | but we do | 16:02 |
| blackboxsw | yeah that mock setup in that test is broken | 16:03 |
| Putti | I'm trying now adding "_set_mock_metadata" to the test | 16:04 |
| Putti | erm "_set_mock_metadata()" | 16:04 |
| Putti | mhm, I guess there is no point in adding that... I guess I need to focus on reading the code for a minute :P | 16:06 |
| blackboxsw | Putti: can you test this patch? http://paste.ubuntu.com/25076047/ | 16:09 |
| Putti | yes, a moment | 16:09 |
| blackboxsw | yeah I don't like mocks, they don't always behave as you'd expect them. In the case of this unit test the setUp() declared the mocked return_value for cloudinit.sources.DataSourceGCE.platform_reports_gce as True. Then the subsequent unit test tried to override that return value incorrectly to False. | 16:12 |
| blackboxsw | using the patch decorator on the specific unit test we can ensure that the unit test has properly overridden that return_value to False instead of setUp's "True". | 16:12 |
| blackboxsw | the problem with our tests was that our broken mock as actually still returning True for platform_reports_gce() so get_data() was attempting to perform all those queries against metadata.google.internal which it never should have gotten to | 16:14 |
| Putti | paste.ubuntu.com requires you to login (which doesn't work without javascript) if you want to see the plaintext without javascript. Another moment. | 16:15 |
| blackboxsw | oops | 16:16 |
| Putti | git doesn't seem to like the patch | 16:18 |
| * Putti does the changes manually | 16:18 | |
| Putti | blackboxsw, the patch works, though no idea what it does :D | 16:22 |
| Putti | aha, ok, I read your explanation now, and it makes sense | 16:23 |
| Putti | thanks! | 16:23 |
| blackboxsw | Putti: awesome. So that @mock.patch is me telling the specific unit test to mock the DataSourceGCE.platform_reports_gce within our unit test only. Then I set return_value = False there to the mocked method. This ensures the unit tests overrides the mock which was established in setUp() | 16:24 |
| blackboxsw | yeah no prob | 16:24 |
| blackboxsw | thanks for finding the bug and working through it w/ me | 16:24 |
| Putti | :) | 16:24 |
| blackboxsw | you can file a merge proposal if you want with that patch and I'll approve and land it if you'd like to go through the process. Or I can, either way | 16:24 |
| Putti | blackboxsw, you should do it, you're the author | 16:25 |
| blackboxsw | Putti could you please file a bug against and link your initial problem paste then :) gotta get you some karma for finding this issue and bringing it to us. (and we need bugs associated w/ any cloud-init fix) https://login.launchpad.net/08ubw7bY4m0DWmsi/+decide | 16:26 |
| Putti | sure, bug report coming soon | 16:27 |
| blackboxsw | https://bugs.launchpad.net/cloud-init/+filebug rather | 16:27 |
| blackboxsw | thanks again | 16:27 |
| powersj | blackboxsw: can you send me a pastebin of the mount image callback stuff | 16:30 |
| Putti | blackboxsw, https://bugs.launchpad.net/cloud-init/+bug/1703935 | 16:30 |
| ubot5 | Ubuntu bug 1703935 in cloud-init "GCE unit test tries to connect to the network" [Undecided,New] | 16:30 |
| blackboxsw | awesome thank you again | 16:31 |
| Putti | not very nice view: https://launchpadlibrarian.net/328486103/error .. Maybe I should have added .txt extension. | 16:32 |
| nacc | Putti: is that for the bug attachment? | 16:33 |
| nacc | Putti: edit it and mark it as text/plain instead? | 16:33 |
| Putti | nacc, yes | 16:33 |
| Putti | doign that :) | 16:33 |
| nacc | Putti: it's currently text/html which is probably wrong :) | 16:33 |
| * nacc has an open self-bug for lp-attach to fix that in that particular tool :) | 16:33 | |
| Putti | do you know if I can file somewhere a bug against paste.ubuntu.com ? | 16:51 |
| Putti | nvm, https://launchpad.net/canonical-pastebin ... but it seems that it is not under a free license :( | 16:54 |
| blackboxsw | smoser: still working on ipv6 support. scaly implementation looks easy on an already configured interface with a valid broadcast over UDP was easy with scaly http://paste.ubuntu.com/25064349/. | 17:20 |
| smoser | blackboxsw, i think that the dhclient paht is probably sane as a first pass. | 17:20 |
| smoser | freebsd is affected too though and need to think about that. | 17:20 |
| blackboxsw | but, this requires that the interface is already up and configured. using dhclient and cleaning up after seems the lowest hanging fruit as unicast packet sending on a raw socket requires a bit more reading for me (couple days probably() | 17:21 |
| blackboxsw | smoser: +1 and centos dhclient supports a slightly different set of cmdline options that we'd have to account for (minimally "-I" is interpreted differently ) | 17:22 |
| dpb1 | blackboxsw, smoser: first pass making dhclient-capable distros work seems sane. This isn't working anywhere that I know of? | 17:24 |
| dpb1 | (on aws) | 17:24 |
| smoser | blackboxsw, powersj i guess we could/should run c-i 'tox' at least with networking taken down. | 18:25 |
| Putti | +1 | 18:28 |
| jhodapp | I'm curious if there's any special way of loading certain kernel modules via cloud-init or if I might simply use modprobe to do so in the free form command run section of the yaml config file? | 20:30 |
| larsks | jhodapp: that should work (using runcmds, or just submitting a /bin/sh script in place of or in addition to the YAML file) | 20:31 |
| jhodapp | larsks, ok, so there's really not any special provision for doing so | 20:32 |
| larsks | Right. | 20:32 |
| jhodapp | ok | 20:32 |
| smoser | blackboxsw, https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/327325 you can push that if you want. | 22:15 |
| blackboxsw | thx smoser, I have a working dhclient client run using your WIP branch on aws + a new dhcp_request_clean() function in cloudinit/net/_init_.py. | 22:16 |
| smoser | nice | 22:17 |
| blackboxsw | just polishing off a simple dhclient-script that could-init would deliver to ensure no other side-effects come from run-hookos | 22:17 |
| blackboxsw | just polishing off a simple dhclient-script that could-init would deliver to ensure no other side-effects come from run-hooks | 22:17 |
| blackboxsw | s/polish/pollish? | 22:17 |
| smoser | blackboxsw, do you have something pushe d? | 22:32 |
| blackboxsw | smoser will pull it down from my aws instance and push in a min | 22:32 |
| smoser | yeah | 22:33 |
| blackboxsw | smoser: a junker WIP for the moment https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+ref/aws-dhclient | 22:37 |
| blackboxsw | I hacked up the assign_ipv4_link_local context manager to call the dhcp_request_clean() | 22:38 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!