[12:41] Hello, could something explain the fact that all unittests pass in the jenkins context but when the pkg builds, one of them fail ? [12:42] and moreover, it builds fine here on X/B & C [12:42] FYI https://jenkins.ubuntu.com/server/job/cloud-init-ci/91/console === r-daneel_ is now known as r-daneel [14:32] blackboxsw or rharper would like more feedback on https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/348005 [14:33] y [14:33] caribou: sorry give me 5 minutes i'll poke a bit. [14:34] smoser: thanks. no real rush as I'm onto something else & won't get back at it until monday [14:50] caribou: oh. well, the failure is simply because the build environment did not have udeveadm settle [15:46] smoser: anything I need to do on my side ? [17:23] caribou: sorry... rough day. [17:23] i'll try to take a quick look [17:23] i'll follow up on your MP [18:04] blackboxsw: you typed: "just a nit" [18:04] https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/348005 [18:04] but .... what nit ? [18:04] the whole thing ? [18:04] =P [18:15] haha, it didn't save my comment. [18:16] smoser: comment attached [18:16] dropping unused assignemnt [18:30] smoser: rharper if we are cutting a cosmic branch today for release, I'd like to see https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/347937 landed if we could [18:31] http://paste.ubuntu.com/p/xFjYFNggh2/ [18:31] that is me testing... the lxdbr0 stuff. looks like we need to fix. === r-daneel_ is now known as r-daneel [18:34] smoser: =( [18:34] looks like there is existing config that auto creates we'll need to sort out [18:35] lxc profile list default or something dumps all of the set config [18:38] yeah, fun [18:39] it's friday, what else were you doing =P [18:40] blackboxsw: yeah, i'd like to get that too. [19:05] this is a pita [19:46] referring to ssh locale.... or lxc [19:46] lxc [19:47] ehem, sorry for the bad review without an integration test run :/ [19:49] my fault. i just assumed. [19:49] and this is waht that leads too [19:49] to [20:02] yeah running commands on an existing install which had lxc seemed to make sense as a validation point, but I should've actually run through the whole system during an install to see what fell out [20:33] rharper: per openstack variable names comment, what use-case are you thinking about where a user might want [20:33] VALID_DMI_PRODUCT_NAMES or VALID_DMI_CHASSIS_ASSET_TAGS and not just the detect_openstack() function instead? [20:34] not the user, any other part of cloud-init that would want to know what values OpenStackDS says are valid product names to positively identify OS [20:34] my point was mostly, if we find out that we have another PRODUCT name or asset tag, there's a single place to add it to a list [20:34] rather than find where in the code an indiviual value is being used [20:34] ahh you are thinking DataSource base class.detect_valid_ds or something [20:35] yes, that'd be the next path; we've not yet migrated the logic from ds-identify into the DS classes themselves for a path which doesn't use ds-identify [21:04] https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/348005 [21:04] that now passes bionic and xenial [21:04] but doesnt have tests [21:05] return volley https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/347937 [21:07] I addressed smoser's and rharper's comments, with exception of the docs reporting what DMI info we look at on openstack. I don't really know which way to go here. I like the discovery detail, but it is something that will change frequently, and it'll leave us with some significant gaps in docs if detection moves beyond the docs. [21:07] it doesn't really change that much [21:07] the set of valid DMI product names is static, until we know otherwise [21:07] also, ds-identify and python Datasource.get_data detection aren't aligned currently in terms of logic :/ [21:08] the exception is probably Ec2 and openstack datasources which are fairly well aligned now [21:08] they don't have to mirror each other in implementation but certainly we don't want one to say we're on OS when we're not [21:08] blackboxsw: can you just add a comment about /proc/1/environ that that is how nova-lxd identifies itself. [21:08] (in the doc changes) [21:08] rharper: right, but I was wondering what we end up documenting in rtd in those cases. the ds-identify detection behavior, or the python detection [21:09] and then... i really dont think you need to cmention chassis_asset_tag of OpenTelecom... [21:09] smoser: +1 [21:09] I don't follow [21:09] if they're not reporting the same thing, then where do they differ ? [21:09] i'm fine with a 'Discovery' that just describes general intent, but does not contain the implementation in english text. [21:09] the detection is the same, I think the behavior w.r.t strict mode results in different paths [21:10] right, I just meant to document that, we currently positively recognize the following values DMI_PRODUCT = [1, 2] as users of the OpenstackDatasource [21:10] no need for anything more than disclosing which values and from where we check [21:10] rharper: right, it's the strict mode setup that allows some datasource detection to be a little looser in get_data() when returning False [21:10] so datasource might have a subset of checks [21:11] which we ultimately should align I think w/ ds-identify behavior [21:11] but, "not there yet" ™ [21:11] I'm still not really following, it sounds like you're suggesting that the datasource as is now would say yes when ds-identify says no [21:11] and if that's the case, what scenario is that ? [21:11] that smells like a bug [21:12] effectively, non-strict mode returns maybe when we cannot positivley identify it; the DS is going to do the same (it'll run the network probe if it can't positively know via the DMI values, etc) [21:12] rharper: well that's what this branch just fixes, openstack datasource determined that it wasn't valid on ec2 platforms because OpenStack.get_data happened to probe the metadata service to find that it wasn't openstack... whereas ds-identify detects that quickly via DMI product_name/chassis_asset_tag checks [21:13] well, no, in non-strict mode, ds-identify still has to say maybe, which means we need to probe over network [21:14] so, AFAICT things are still aligned, we're adding support for strict mode in the Ds class itself; but not setting it to strict mode only, which is just fine [21:15] I mean, it can be in cosmic, I think; but IIRC this is onlyi nXenial which isn't in strict mode; so we know we're probing, and this is a detect short-circuit to prevent probing when we absolutely know we're on OS [21:15] rharper: yes this issue was only in Xenial and I think you are right it's due to non-strict vs strict mode [21:15] I don't see any divergent behavior here, so maybe it's just some phrasing; but the branch you have looks fine, I think does the right thing on xenial (non-strict mode) ; [21:16] and on a non-ds-identify OS, where strict=true, then the get_data() would say no without positive id, right ? [21:16] yeah, I'm making hand-wavy suggestions, I'll have to find concrete examples we can talk about monday. (I thought I had come across a couple) [21:16] so that should mirror what we get with ds-identify [21:16] ok [21:19] rharper: end result in all cases I think the python code does mirror results from ds-identify. I think there may be some gaps where the datasource does more work than it needs to to vet environments as DS_NOT_FOUND that ds-identify does. Some of that behavior is because the get_data() actually has does addtional probing of the metadata services etc. But some of that work could be avoided with a tighter exit NOT_FOUND [21:19] detection. [21:20] wow typos. ok. I'll dig up a couple examples [21:21] ok, so optimizing but we don't have differing results; which I certainly fine [21:21] ci needs fixing smoser on your lxc brnach [21:22] rharper: correct, sorry for the fuss, just optimization (early exit on known invalid datasource detection) [21:22] ok [21:36] fudged [21:36] d key sticks down [21:44] rharper, blackboxsw https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/348005 i'm happy with that now, and have tested in x, b, c [21:44] test coverage is not 100% though. it doesnt test the lxc command error path. [21:44] but running in x, b, c does test that for exit code = 1 [21:45] we weren't testing lxc error path before either, right ? [21:45] well before the rework i had 100% coverage on the newly added code [21:45] oh, hehe [21:46] but yeah, if lxc fails with other than 1, its going to bomb out [21:46] which is kind of fine.. [21:46] i have to run. i'll check back in in a few hours [21:47] ok