| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-19 | |||
| 14:03:29 | dansmith | and that's good for early revs I think | |
| 14:03:46 | gibi | OK, I think I see the use case of the templating now. | |
| 14:03:47 | efried | but yes, you make a good case for future expansion of the identification section. | |
| 14:03:54 | dansmith | I'm saying that's a good reason for having identification be a section and not a key | |
| 14:03:55 | dansmith | right | |
| 14:04:30 | gibi | we might need a warning for ironic admins about using the yaml for node specific config | |
| 14:04:44 | dansmith | like I said, a ..note: somewhere for sure | |
| 14:04:49 | gibi | OK | |
| 14:05:01 | gibi | let me read PS13 through, but I think I'm +2 on the spec | |
| 14:05:06 | efried | Thanks gibi | |
| 14:10:35 | artom | dansmith, dunno if you saw efried's adding of yourself to https://review.opendev.org/#/c/671471/, could I pester you some more for it? | |
| 14:11:02 | gibi | efried: does changing 'ensure' to 'additional' keeps the intended behavior idepmotent? I mean if I restart a compute serivce it won't try to re-create the inventory if already exists | |
| 14:11:13 | dansmith | artom: I did not | |
| 14:11:46 | dansmith | gibi: it's still idempotent language to me | |
| 14:11:50 | efried | gibi: yes, it's just a bikeshed of the word being used. I think I explained the rationale in the comment on PS12 | |
| 14:12:14 | efried | gibi: it's more than just "restart a compute service" -- this thing runs on every periodic | |
| 14:12:15 | gibi | dansmith: I more after the behavior. if that is idepmontent then I'm happy | |
| 14:12:33 | gibi | efried: OK. then I'm happy :) | |
| 14:12:37 | dansmith | gibi: same behavior, more descriptive (IMHO) wording | |
| 14:12:43 | efried | It makes me happy to make you happy, gibi | |
| 14:13:08 | gibi | dansmith: OK. I accept the language change blindly as I'm not a native speakre | |
| 14:13:22 | dansmith | heh | |
| 14:13:36 | gibi | ... and try to learn in the process | |
| 14:13:47 | efried | don't try to learn too much from s/ensure/additional/ | |
| 14:13:52 | efried | that way lies madness | |
| 14:14:18 | dansmith | efried: stop mocking me | |
| 14:14:20 | efried | you can learn things about Dan, but not about English | |
| 14:14:23 | efried | jinx | |
| 14:14:43 | efried | kisses | |
| 14:14:44 | gibi | :) | |
| 14:19:49 | gibi | I'm +2 on the provider config spec | |
| 14:20:24 | gibi | and I guess it is good time to inform you that I will disappeare for two weeks of vacation starting in an hour | |
| 14:22:52 | efried | phew, then I'm glad I nabbed you for that +2. Have a great vacation gibi! | |
| 14:23:02 | gibi | efried: thanks! | |
| 14:28:46 | cdent | enjoy your time off gibi | |
| 14:28:59 | gibi | cdent: thanks | |
| 14:31:51 | dansmith | artom: you have to fix your many typos anyway, so I think you should move the code to a single check to LOG.warning on startup and then I'd be +2 if not for needing some diversity on it | |
| 14:32:16 | artom | dansmith, in _init_host? | |
| 14:32:27 | dansmith | that'd be startup yeah :) | |
| 14:32:27 | artom | Yeah, that'd make sense | |
| 14:32:46 | sean-k-mooney | if you put it in _init_host i would perfonally make it stop the agent booting | |
| 14:32:57 | dansmith | no | |
| 14:33:04 | artom | It wouldn't stop it | |
| 14:33:10 | artom | But... it might take a long time | |
| 14:33:12 | dansmith | this should be a LOG.warning | |
| 14:33:13 | sean-k-mooney | im fine with it just being a warning to | |
| 14:33:21 | artom | Not sure if that's acceptable | |
| 14:33:35 | dansmith | artom: also a reno, just in case anyone is relying on this to do their homework for them | |
| 14:34:20 | dansmith | artom: how long does this take? I'm not sure why it'd be slow really | |
| 14:34:29 | artom | The bug's env has > 1000 nics, they timed it to over 2 minutes | |
| 14:34:40 | artom | ========== | |
| 14:34:41 | artom | $ time python -c "import nova.compute.utils; nova.compute.utils.get_machine_ips()" | |
| 14:34:41 | artom | real 2m32.642s | |
| 14:34:41 | artom | user 0m4.358s | |
| 14:34:41 | artom | sys 2m27.918s | |
| 14:34:41 | artom | ========== | |
| 14:34:59 | efried | wow | |
| 14:35:14 | dansmith | which thing takes long? listing the nics themselves? | |
| 14:35:20 | dansmith | because that really shouldn't take that long | |
| 14:35:26 | efried | yeah, is there not a quicker way to find out if my IP is on the system? | |
| 14:35:35 | efried | ifconfig -a | grep ?? | |
| 14:35:53 | artom | efried, could be that "dumb", yeah. Would we be cool with this? | |
| 14:36:03 | dansmith | if it's the list, then not much can be done, but if it's our processing, we could limit to 100 nics and bail after, or something | |
| 14:36:05 | artom | ifconfig isn't on every system nowadays, but `ip` is | |
| 14:36:12 | efried | considering what we're shooting for here, I would be totally fine with something that "dumb". | |
| 14:36:14 | dansmith | no, don't do ip|grep | |
| 14:36:27 | efried | if we were relying on this for real functionality that would be one thing | |
| 14:36:30 | dansmith | this has been in place for four years and one person has complained, | |
| 14:36:34 | efried | but this is a best-effort sanity check | |
| 14:36:40 | dansmith | so I think once at startup which is way less impactful should be fine | |
| 14:36:47 | artom | dansmith, well, thousands of nics aren't exactly common :) | |
| 14:36:53 | dansmith | right | |
| 14:37:07 | dansmith | which is why it's not a problem to do this for the 99.999% I think | |
| 14:37:15 | efried | and as dansmith noted in the original, it's possible for it to fail and everything still be alright. | |
| 14:37:17 | artom | I'm just worried there might be something that implicitly relies on compute service starting up in less than a few seconds | |
| 14:37:33 | dansmith | artom: we have tons of things that are long-running in compute startup, | |
| 14:37:35 | dansmith | so that's not a concern | |
| 14:37:54 | dansmith | we block compute startup until we can talk to conductor in both directions, we clean up failed migrations, do disk IO, etc, etc | |
| 14:38:35 | artom | dansmith, ack, sounds like we have a way forward with moving the warning to _init_host then | |
| 14:42:07 | artom | (dansmith, btw, I suspect it's https://github.com/openstack/nova/blob/master/nova/compute/utils.py#L977 that's the time sink, not https://github.com/openstack/nova/blob/master/nova/compute/utils.py#L975, but that's moot, and unless we ask the reporter we'll never know) | |
| 14:42:44 | dansmith | artom: yeah I just don't know _why_ it'd be slow, unless it's trying to reverse lookup all the addresses too | |
| 14:43:10 | artom | Yeah, good question... | |
| 14:43:24 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Followup for provider config file spec https://review.opendev.org/671749 | |
| 14:43:35 | artom | If it was doing reverse lookups I think it'd be slower, because a single loopkup takes forever to timeout | |
| 14:43:36 | efried | dansmith: Put the ironic NOTE in a fup so as not to lose gibi's +2 ^ | |
| 14:44:18 | dansmith | artom: that call is implemented in C even | |
| 14:47:31 | alex_xu | stephenfin: sean-k-mooney thanks in person | |
| 14:47:57 | alex_xu | sean-k-mooney: buyin your proposal about 1 extraspec and mask | |
| 14:52:10 | alex_xu | artom: ah...claim depends on migrate_data, just aware that, I need to dig into your patch more | |
| 14:52:40 | artom | alex_xu, hey, if you find a way to make it work, I'm all for it :) | |
| 14:53:28 | alex_xu | at least understand the dependence...and I pretty sure people will hate that I added a interface call get_vpmems :) | |
| 14:55:04 | artom | alex_xu, yeah, that'd make 2 libvirt-specific interfaces that no other driver uses | |
| 14:55:22 | alex_xu | yea | |
| 14:56:51 | artom | Maybe consolidate them into something like "get_driver_specific_migrate_data"? | |
| 14:56:58 | artom | Not super happy with that either | |
| 14:57:49 | efried | fwiw, I think we need *something* like this ^ | |
| 14:58:08 | efried | although... I thought we already *had* driver-specific migrate data | |
| 14:58:20 | artom | Exactly | |
| 14:58:29 | artom | It's more like... claims-dependant driver-specific | |
| 14:58:32 | artom | Which is a mouthful | |