| === shardy is now known as shardy_lunch | ||
| === shardy_lunch is now known as shardy | ||
| jgrimm | smoser, i'm gonna work on https://bugs.launchpad.net/cloud-init/+bug/1583841 if you don't mind | 13:24 |
|---|---|---|
| smoser | that is odd that we explicitly install ruby1.8 | 13:25 |
| smoser | oh... because we just dont know i guess? as the gems installer is not from the archive/ | 13:25 |
| smoser | maybe. but thanks! | 13:25 |
| jgrimm | i'm assuming at one point there were multiple rubys and it needed specific one | 13:25 |
| jgrimm | also noticed that our chef example config is broken, i have that working locally.. i'll submit bug & MR | 13:26 |
| jgrimm | the upstream apt repo changed | 13:26 |
| smoser | jgrimm, fyi, cloud-init got let in this morning to zesty. | 13:29 |
| jgrimm | i saw! | 13:29 |
| jgrimm | \o/ | 13:30 |
| jgrimm | powersj, if i want to run the tox citest against code changes i've made in my local cloud-init repo. i'm guessing i need to create a dep and supply it, otherwise it will test against archive cloud-init? | 14:26 |
| jgrimm | s/dep/deb | 14:26 |
| rharper | jgrimm: yes; | 14:29 |
| rharper | I think we should add that workflow example to the tests.rst doc | 14:29 |
| jgrimm | thanks, right.. i found it in practice, but the docs could be clearer | 14:30 |
| rharper | I'm pretty sure you can: ./package/bddeb ; and then feed the resulting deb to the test ... now; magicalChicken did work on adding a 'build deb' stage | 14:30 |
| jgrimm | sweet | 14:30 |
| rharper | as not everyone has deps installed for binary build | 14:30 |
| jgrimm | right | 14:30 |
| rharper | I've just not tested it, so don't know the flags/steps etc | 14:30 |
| rharper | if you figure them out, a MR for doc updates would be great | 14:30 |
| jgrimm | :) i was mostly looking for confirmation of what i was seeing | 14:31 |
| jgrimm | and yes, on docs updates | 14:31 |
| rharper | =) | 14:31 |
| powersj | or we could just accept wes' merge that does it for you :) | 14:39 |
| jgrimm | heh, indeed i want that to happen soon | 14:39 |
| === nacc_ is now known as nacc | ||
| smoser | rharper, https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/321578 | 15:06 |
| rharper | looking | 15:07 |
| rharper | neat | 15:08 |
| rharper | reviewed, somewhat concerned by always filtering 'stolen' mac entries; I'd like to scope the filtering to just the rename case where we know that we don't want those interface mappings | 15:13 |
| smoser | rharper, read my comment there please. and see what you think | 16:04 |
| smoser | bah. adding the RuntimeError makes unit tests fail on my system | 16:11 |
| smoser | RuntimeError: duplicate mac found! both 'virbr0-nic' and 'virbr0' have mac '52:54:00:4c:a1:4a' | 16:12 |
| rharper | smoser: looking | 16:14 |
| rharper | smoser: if all callers are expecting 'physical' nics then maybe the method ought it be called get_physical_interfaces_by_mac(); it certainly appears that all users of the method were interested in 'physical' only; | 16:16 |
| rharper | smoser: shouldn't we mocking the response there? | 16:16 |
| rharper | w.r.t your RunTimeError | 16:16 |
| rharper | that looks like host data | 16:16 |
| smoser | well, yeah, it *is* host data. | 16:18 |
| smoser | which yeah should be mocked, but is a good find. thats a bridge, which i think most of the time will have the address of a nic also. | 16:19 |
| rharper | y | 16:19 |
| rharper | hehe | 16:19 |
| smoser | i dont like the word 'physical' | 16:19 |
| smoser | because explicitly im' including veth | 16:19 |
| smoser | "base nics" ? i dont know. | 16:19 |
| rharper | I don't know either | 16:20 |
| rharper | in the docs I wrote | 16:20 |
| smoser | but yeah. all callers were expecting "ethernet interfaces" | 16:20 |
| rharper | I mentioned this was a mix of criteria | 16:20 |
| rharper | s/ethernet/network interfaces; and I suspect 'present without configuration/composition' | 16:21 |
| smoser | :) | 16:21 |
| smoser | get_interfaces_present_without_configuration_by_mac() | 16:21 |
| rharper | heh | 16:21 |
| smoser | rolls right off the tounge | 16:21 |
| rharper | clearly | 16:21 |
| rharper | get_renameable_interfaces | 16:21 |
| smoser | so i think i'm going to just exclude bridges for now | 16:22 |
| rharper | I think the property is certainly key | 16:22 |
| rharper | we were before as well (excluding bridges) and (name=veth*) | 16:22 |
| smoser | well, except i'm explicitly including in that list stuff that has a '0' in addr_assign_type | 16:22 |
| smoser | well the get_interfaces_by_mac() didn't previously exlude bridges | 16:23 |
| rharper | which is fine, no? those devices were *created* in the kernel vs. added/renamed/modified during boot | 16:23 |
| smoser | i dont follow. | 16:27 |
| smoser | so i just changed get_interfaces_by_mac() to exclude bridges | 16:27 |
| smoser | which i think is right. because bridges are not "present_without_configuration" | 16:28 |
| rharper | I don't have a bridge around , what addr type was it? | 16:30 |
| rharper | there's one; 1 for me | 16:30 |
| rharper | so would have been allowed,; yeah I'm in agreement; I was confused; in the fallback networing code, there is some filtering done there when selecting nics; in that case bridges, lo, and nics with 'veth*' in the name were dropped | 16:31 |
| rharper | it might be worthwhile to consolidate this 'get_interfaces_by_mac_without_config_excluding_lo' into something common | 16:32 |
| nacc | is 'lo' the literal value used? maybe get_interfaces_by_mac(with_config: bool, excludes:list) ? | 16:33 |
| smoser | nacc, yes. | 16:35 |
| smoser | invalid_interfaces = set(['lo']) | 16:35 |
| nacc | makes for at least a shorter function name :) | 16:35 |
| nacc | i would possibly make flags: enum or something, if there are multiple types of withouts | 16:35 |
| rharper | nacc: yeah, so ls -1 /sys/class/net/* are all of the 'net' devices; some of those don't support renaming, and also aren't useful for a default "first nic which we can likely expect DHCP to work on" | 16:36 |
| nacc | rharper: yep, makes sense | 16:36 |
| rharper | smoser: fyi, just tested the zesty cloud-init in a uc16 image; it cleans up the default netplan config perfectly and rendered a new one as expected; nice! | 16:41 |
| smoser | \o/ | 16:43 |
| rharper | testing in OpenStack next, and then I'll upload an image for plars to check out on maas | 16:43 |
| smoser | rharper, nothing is ever easy | 17:06 |
| rharper | =) | 17:06 |
| jgrimm | powersj, i haven't looked into it, but it would be nice to be able to override 'enabled: False' without editing a testcase.yaml.. | 17:10 |
| smoser | rharper, and for the resolvconf/ifupdown i need to file a debian bug | 17:11 |
| powersj | jgrimm: like when you try running it by hand with -t <testcase> and have it still run? | 17:11 |
| jgrimm | yep! | 17:11 |
| powersj | ah! ok :) | 17:11 |
| powersj | I'll jot that one down | 17:11 |
| jgrimm | powersj, as there are testcases disabed due to 'taking too long' for part of normal ci | 17:11 |
| jgrimm | but i want to easily run it | 17:12 |
| powersj | jgrimm: none should be disabled for taking too long, only if 1) we know they don't work 2) they are not done 3) it is covered by another test case | 17:12 |
| jgrimm | powersj, see tests/cloud_tests/configs/example/TODO.md | 17:14 |
| jgrimm | powersj, but indeed, i did not find the chef one to actually take longer than anything else really to run | 17:14 |
| nacc | jgrimm: ok, just verified the fix for the cloud-images does work | 17:15 |
| nacc | Odd_Bloke: --^ fyi | 17:15 |
| jgrimm | cool | 17:15 |
| jgrimm | nacc, i'm assuming iscsi. :) | 17:15 |
| powersj | jgrimm: ah yeah that list is things that have not even been written yet | 17:16 |
| nacc | jgrimm: ack | 17:17 |
| jgrimm | powersj, heh.. well someone wrote ' (takes > 60 seconds to run)' so I assumed they'd actually run it | 17:17 |
| jgrimm | powersj, indeed comments can lie. :) | 17:18 |
| powersj | ah ok :) | 17:21 |
| jgrimm | powersj, i'm using that install_run_chef_recipes.yaml to test https://bugs.launchpad.net/cloud-init/+bug/1678145 ... I'm ambivalent whether I enable it for citest? thoughts? | 17:22 |
| powersj | If you get it running, go for it | 17:23 |
| jgrimm | right now i've only cobbled in a single test just to see that chef installed.. as good enough, with FIXME to add more comprehensively | 17:23 |
| powersj | if you don't let me know and I can finish it up. I'd prefer the test while you are working on it :) | 17:23 |
| jgrimm | my hestitations were 1) that someone explicitly said it should be disabled (per comment I showed you) 2) it depends on an upstream repo .. I'm not thrilled with external network dependencies in CI 3) only LTSes supported by external apt repo | 17:24 |
| jgrimm | so #3 probably needs to wait for wes's featureflag to do skips?? | 17:25 |
| magicalChicken | jgrimm: that would be pretty easy to do using feature flags, just add a 'ubuntu_lts' flag and require it there | 17:27 |
| jgrimm | yep! indeed quite handy | 17:27 |
| powersj | hmm I agree that the 3rd party repo is undesirable :\ | 17:28 |
| jgrimm | heh, but that's what the feature is there for.. easy of installing the upstream | 17:29 |
| jgrimm | but shouldn't a hard error if the resource is unavailable would be desired behavior | 17:30 |
| powersj | I know having unreliable and false negatives is annoying, so do we have any idea how available it is? If it is failing for you even during development then I think that is an easy no. | 17:32 |
| jgrimm | powersj, i have no idea at all whether its _really_ unreliable.. just my overall concern with external resources | 17:32 |
| jgrimm | heh, except that its a hard failure today because they changed the repo | 17:33 |
| jgrimm | and we'd not noticed as we don't have any test. :) | 17:33 |
| powersj | right so a test would have caught this :) | 17:33 |
| powersj | haha | 17:33 |
| jgrimm | exactly! | 17:33 |
| magicalChicken | powersj, jgrimm: i could add in support for overriding base feature flags from cmdline. that way the feature could be unavailable everywhere by default, and we could just enable it when we know the resource is available | 17:33 |
| * powersj wants to just add it and if smoser sees too many failures we can pull it | 17:34 | |
| jgrimm | i think that may be handy generally | 17:34 |
| jgrimm | powersj, yep, i'm fine with enabling it. skip again if its not worth it | 17:34 |
| jgrimm | but.. i'm going to wait for feature flags | 17:34 |
| smoser | which is this ? | 17:35 |
| smoser | talking about a test for chef ? | 17:35 |
| jgrimm | yep, specifically install_run_chef_recipes.yaml | 17:35 |
| jgrimm | which is our chef config example | 17:36 |
| jgrimm | no test for it today, which is why this was never noticed: https://bugs.launchpad.net/cloud-init/+bug/1678145 | 17:36 |
| rharper | smoser: plars tested the ds-identify change for maas cfg parsing, that's working as well | 17:43 |
| Odd_Bloke | nacc: \o/ | 17:44 |
| smoser | jgrimm, yeah.. so we can enable / add one. but as we've seen external dependencies suck | 17:45 |
| smoser | ie, curtin dependencies on launchpad or archive have a very non-zero failure rate. | 17:45 |
| smoser | adding a 3rd party dependency has potential for the same. | 17:46 |
| jgrimm | smoser, yep. totally aware, that's why i flagged it as an issue | 17:50 |
| nacc | Odd_Bloke: thanks for your help! | 17:50 |
| jgrimm | smoser, for now, i was just looking to add a test that we can use on demand, i'm more wary of having it always enabled. | 17:53 |
| smoser | right. | 17:53 |
| jgrimm | smoser, with zesty cloud-init uploaded, i'm assuming you are working on the corresponding xenial SRU? | 18:07 |
| smoser | i'm still working on this bond thing :) | 18:08 |
| smoser | but yeah, that'd be the plan | 18:08 |
| jgrimm | ok, yeah, we are gated on next steps with that SRU | 18:09 |
| powersj | jgrimm: seems you got a harder review than I did ;) | 20:02 |
| jgrimm | powersj, lol. its the nature of reviews. you can see something new every time you look in | 20:03 |
| smoser | rharper, still there? | 20:11 |
| smoser | https://bugs.launchpad.net/cloud-init/+bug/1677846 . i think i just need to ggive up | 20:11 |
| rharper | here | 20:13 |
| rharper | smoser: I thought we already pointed back at the OpenStack change there | 20:14 |
| rharper | just a dup, right? | 20:14 |
| smoser | yeah. its just that i think i'm going to give up. at lest for 'dvs' | 20:15 |
| smoser | and re-push upstream proposal | 20:15 |
| rharper | why? | 20:15 |
| smoser | the real rason for ping | 20:15 |
| smoser | http://paste.ubuntu.com/24290015/ | 20:16 |
| smoser | ^ that is the diff i have locally | 20:16 |
| smoser | have to remove use of 'assert_called()' | 20:16 |
| smoser | but wanted you to review that. | 20:16 |
| rharper | looking | 20:16 |
| smoser | taht is diff agastin what is in https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/321578 | 20:17 |
| smoser | does that provide rasonable test ? | 20:17 |
| smoser | the thing is kind of hard to test. wihtout mock-o-rama | 20:17 |
| * rharper wishes for assert_called(); I use len(mock_obj.call_arg_list) != 0 IIRC | 20:17 | |
| rharper | so, in the case that we see a duplicate, why is devs reset to empty list ? | 20:19 |
| rharper | OSError is for something besides the RuntimeError for duplicates I suspect | 20:21 |
| rharper | still not sure why the list would be reset in that case; documenting why we reset the list here would help (it's not obvious to me) | 20:21 |
| rharper | that said, the unittests look good | 20:23 |
| smoser | where do you see the list is reset ? | 20:27 |
| smoser | rharper, ^ ? | 20:29 |
| rharper | line 25- > 27 | 20:30 |
| rharper | on the OSError exception if errno.ENOENT | 20:30 |
| rharper | devs = [] | 20:30 |
| smoser | that has always been there. | 20:33 |
| rharper | code motion then | 20:33 |
| rharper | ok | 20:33 |
| smoser | if get_device_list() raises an exception | 20:33 |
| smoser | then it sets the list to [] | 20:33 |
| rharper | still seems odd to wipe the list, rather than return what it already had | 20:33 |
| smoser | ie, if you didn't have /sys/class/net | 20:33 |
| smoser | then you get "no devices" | 20:33 |
| smoser | there is no "already had" | 20:34 |
| rharper | I see, if devs was none | 20:34 |
| rharper | it just looks strange to set it to list, and then continue with a for name in devs which wont do anything, just return [] | 20:34 |
| rharper | oh, I see, we're returning a dict; well could return an empty dict | 20:35 |
| rharper | it's fine | 20:35 |
| rharper | it's harder to see with just the patch context; | 20:35 |
| smoser | yeah. | 20:35 |
| smoser | and you're right. return {} | 20:35 |
| smoser | would be the same | 20:35 |
| smoser | i dropped the 'devs=' param | 20:36 |
| smoser | as it was wierd. | 20:36 |
| smoser | Also drop the 'devs' argument to get_interfaces_by_mac. It was | 20:36 |
| smoser | non-obvious what the result should be if a device in the input | 20:36 |
| smoser | list was filtered out. ie should the following have an entry for | 20:36 |
| smoser | bond0 or not. get_interfaces_by_mac(devs=['bond0']) | 20:36 |
| rharper | yeah, not sure who we pre-loading it with a set of params | 20:36 |
| smoser | nothing used it like that. | 20:37 |
| rharper | s/we/would | 20:37 |
| rharper | yeah | 20:37 |
| rharper | maybe for easier unittesting =) | 20:37 |
| smoser | right | 20:37 |
| smoser | thats why i added it i'm sure | 20:37 |
| rharper | =_ | 20:37 |
| rharper | err, =) | 20:37 |
| jgrimm | smoser, thoughts on URL shortener that won't disappear (or desire to use/avoid in the project??) -> https://bugs.launchpad.net/cloud-init/+bug/1669727/comments/1 | 22:14 |
| jgrimm | smoser, fwiw... i'm trying to catch up to my long ago promise to sweep through the old bugs and do some housekeeping. long overdue, sorry | 22:45 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!