caribou | Hello, could something explain the fact that all unittests pass in the jenkins context but when the pkg builds, one of them fail ? | 12:41 |
---|---|---|
caribou | and moreover, it builds fine here on X/B & C | 12:42 |
caribou | FYI https://jenkins.ubuntu.com/server/job/cloud-init-ci/91/console | 12:42 |
=== r-daneel_ is now known as r-daneel | ||
smoser | blackboxsw or rharper would like more feedback on https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/348005 | 14:32 |
rharper | y | 14:33 |
smoser | caribou: sorry give me 5 minutes i'll poke a bit. | 14:33 |
caribou | smoser: thanks. no real rush as I'm onto something else & won't get back at it until monday | 14:34 |
smoser | caribou: oh. well, the failure is simply because the build environment did not have udeveadm settle | 14:50 |
caribou | smoser: anything I need to do on my side ? | 15:46 |
smoser | caribou: sorry... rough day. | 17:23 |
smoser | i'll try to take a quick look | 17:23 |
smoser | i'll follow up on your MP | 17:23 |
smoser | blackboxsw: you typed: "just a nit" | 18:04 |
smoser | https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/348005 | 18:04 |
smoser | but .... what nit ? | 18:04 |
rharper | the whole thing ? | 18:04 |
rharper | =P | 18:04 |
blackboxsw | haha, it didn't save my comment. | 18:15 |
blackboxsw | smoser: comment attached | 18:16 |
blackboxsw | dropping unused assignemnt | 18:16 |
blackboxsw | 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:30 |
smoser | http://paste.ubuntu.com/p/xFjYFNggh2/ | 18:31 |
smoser | that is me testing... the lxdbr0 stuff. looks like we need to fix. | 18:31 |
=== r-daneel_ is now known as r-daneel | ||
rharper | smoser: =( | 18:34 |
rharper | looks like there is existing config that auto creates we'll need to sort out | 18:34 |
rharper | lxc profile list default or something dumps all of the set config | 18:35 |
smoser | yeah, fun | 18:38 |
rharper | it's friday, what else were you doing =P | 18:39 |
smoser | blackboxsw: yeah, i'd like to get that too. | 18:40 |
smoser | this is a pita | 19:05 |
blackboxsw | referring to ssh locale.... or lxc | 19:46 |
smoser | lxc | 19:46 |
blackboxsw | ehem, sorry for the bad review without an integration test run :/ | 19:47 |
smoser | my fault. i just assumed. | 19:49 |
smoser | and this is waht that leads too | 19:49 |
smoser | to | 19:49 |
blackboxsw | 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:02 |
blackboxsw | rharper: per openstack variable names comment, what use-case are you thinking about where a user might want | 20:33 |
blackboxsw | VALID_DMI_PRODUCT_NAMES or VALID_DMI_CHASSIS_ASSET_TAGS and not just the detect_openstack() function instead? | 20:33 |
rharper | 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 |
rharper | 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 |
rharper | rather than find where in the code an indiviual value is being used | 20:34 |
blackboxsw | ahh you are thinking DataSource base class.detect_valid_ds or something | 20:34 |
rharper | 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 | 20:35 |
smoser | https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/348005 | 21:04 |
smoser | that now passes bionic and xenial | 21:04 |
smoser | but doesnt have tests | 21:04 |
blackboxsw | return volley https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/347937 | 21:05 |
blackboxsw | 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 |
rharper | it doesn't really change that much | 21:07 |
rharper | the set of valid DMI product names is static, until we know otherwise | 21:07 |
blackboxsw | also, ds-identify and python Datasource.get_data detection aren't aligned currently in terms of logic :/ | 21:07 |
blackboxsw | the exception is probably Ec2 and openstack datasources which are fairly well aligned now | 21:08 |
rharper | 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 |
smoser | blackboxsw: can you just add a comment about /proc/1/environ that that is how nova-lxd identifies itself. | 21:08 |
smoser | (in the doc changes) | 21:08 |
blackboxsw | 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:08 |
smoser | and then... i really dont think you need to cmention chassis_asset_tag of OpenTelecom... | 21:09 |
blackboxsw | smoser: +1 | 21:09 |
rharper | I don't follow | 21:09 |
rharper | if they're not reporting the same thing, then where do they differ ? | 21:09 |
smoser | i'm fine with a 'Discovery' that just describes general intent, but does not contain the implementation in english text. | 21:09 |
rharper | the detection is the same, I think the behavior w.r.t strict mode results in different paths | 21:09 |
rharper | 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 |
rharper | no need for anything more than disclosing which values and from where we check | 21:10 |
blackboxsw | 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 |
blackboxsw | so datasource might have a subset of checks | 21:10 |
blackboxsw | which we ultimately should align I think w/ ds-identify behavior | 21:11 |
blackboxsw | but, "not there yet" ™ | 21:11 |
rharper | 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 |
rharper | and if that's the case, what scenario is that ? | 21:11 |
rharper | that smells like a bug | 21:11 |
rharper | 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 |
blackboxsw | 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:12 |
rharper | well, no, in non-strict mode, ds-identify still has to say maybe, which means we need to probe over network | 21:13 |
rharper | 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:14 |
rharper | 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 |
blackboxsw | 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 |
rharper | 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:15 |
rharper | and on a non-ds-identify OS, where strict=true, then the get_data() would say no without positive id, right ? | 21:16 |
blackboxsw | 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 |
rharper | so that should mirror what we get with ds-identify | 21:16 |
rharper | ok | 21:16 |
blackboxsw | 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 |
blackboxsw | detection. | 21:19 |
blackboxsw | wow typos. ok. I'll dig up a couple examples | 21:20 |
rharper | ok, so optimizing but we don't have differing results; which I certainly fine | 21:21 |
blackboxsw | ci needs fixing smoser on your lxc brnach | 21:21 |
blackboxsw | rharper: correct, sorry for the fuss, just optimization (early exit on known invalid datasource detection) | 21:22 |
rharper | ok | 21:22 |
smoser | fudged | 21:36 |
smoser | d key sticks down | 21:36 |
smoser | 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 |
smoser | test coverage is not 100% though. it doesnt test the lxc command error path. | 21:44 |
smoser | but running in x, b, c does test that for exit code = 1 | 21:44 |
rharper | we weren't testing lxc error path before either, right ? | 21:45 |
smoser | well before the rework i had 100% coverage on the newly added code | 21:45 |
rharper | oh, hehe | 21:45 |
smoser | but yeah, if lxc fails with other than 1, its going to bomb out | 21:46 |
smoser | which is kind of fine.. | 21:46 |
smoser | i have to run. i'll check back in in a few hours | 21:46 |
rharper | ok | 21:47 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!