| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-09-06 | |||
| 16:36:48 | stephenfin | efried: Sorry, was on a call (and follow-ups). Yeah, I think you mentioned that issue before to me, hence why I brought it up again | |
| 16:37:18 | efried | stephenfin: It didn't sound familiar, so if I brought it up, I purged the memory. | |
| 16:37:52 | efried | stephenfin: See latest comment, not sure where to go from here. Back to PS2, or put really awkward words in, or <something else>...? | |
| 16:38:18 | stephenfin | efried: But, as you've summarized on the review, using that default is the correct thing to do. Confusingly, the default reflects the _processed_ result, i.e. once oslo.config has gone and merged all those disparate 'alias' values into one list | |
| 16:38:59 | stephenfin | efried: I'm also not 100% sure that the bug upstream is a bug as it's hiding the above fact from the user | |
| 16:39:29 | efried | Well, the bug just complains that you break if you use square brackets. Which is true. | |
| 16:39:31 | stephenfin | efried: The changes they've made to the help text are valid and should be retained. It's just the changes to default that need to be removed | |
| 16:39:43 | stephenfin | efried: In nova.conf? | |
| 16:39:49 | efried | yeah | |
| 16:40:06 | efried | it breaks when we try to process it. | |
| 16:40:12 | efried | not an oslo problem, a nova problem. | |
| 16:40:14 | stephenfin | efried: Yeah, then our docs are wrong. We never supported use of list values for that option | |
| 16:40:22 | stephenfin | Yup, a nova docs problem | |
| 16:41:08 | stephenfin | Fixing this by adding processing of lists would be a feature, not a bug fix, as you've pointed out. Fixing it by correcting the docs, on the other hand, is a must do | |
| 16:43:09 | stephenfin | efried: That make sense? | |
| 16:43:59 | efried | stephenfin: Yeah, I get it. And their add definitely helps. But don't you feel like we should mention that you *can't* use a square-bracket list, despite the fact that the default makes it look like you can? | |
| 16:44:31 | stephenfin | efried: The default in the code but not the docs. I think a comment would be a-ok | |
| 16:45:36 | stephenfin | I wouldn't put it into the docstring since that's going to show up in the HTML docs and nova.conf templates, neither of which will actually have that default | |
| 16:47:01 | efried | stephenfin: But only because there's a bug? | |
| 16:47:20 | stephenfin | efried: Is it a bug or is it a feature? :) | |
| 16:47:25 | efried | stephenfin: The default being [] is something that *should* show up in the docs | |
| 16:47:31 | efried | because that's how it shows up in code. | |
| 16:48:16 | stephenfin | efried: Right, but recall what I said above. The default for these MultiStrOpt opts applies to the final value, not each individual entry | |
| 16:48:43 | stephenfin | Not a great design decision in oslo.config, if you ask me, but not something one could fix without breaking the world | |
| 16:49:17 | efried | ? | |
| 16:49:17 | efried | alias = '' | |
| 16:49:17 | efried | stephenfin: sorry, I don't follow. You mean that by saying the default is '' it's actually saying that omitting it is equivalent to specifying | |
| 16:49:53 | efried | cause... it ain't, I'm pretty sure. If you did that, you would either get '' or [''] | |
| 16:50:15 | efried | whereas omitting it you get [] | |
| 16:50:21 | efried | f idk | |
| 16:50:45 | efried | maybe I'm the one thinking too hard about this, and it's obvious to admins | |
| 16:51:00 | melwitt | I was thinking it was a bug that a list doesn't work and that we would fix it. did someone not want to fix it? | |
| 16:51:04 | efried | but seeing default=[] makes me think that I would use [...] to specify multiple values. | |
| 16:51:23 | efried | melwitt: Yeah, they fixed that in PS2 but I shot it down because we don't want to do more work on pci code. | |
| 16:51:25 | stephenfin | efried: Yeah, which is why the docs lie | |
| 16:51:30 | melwitt | (I just commented on the review but now see it's being discussed still here) | |
| 16:51:48 | stephenfin | melwitt: We don't want/need to do that. It's a feature. The bug is in the documentation, IMO | |
| 16:51:51 | efried | melwitt: or as stephenfin put it, fixing it to take a list would be a "feature". | |
| 16:52:26 | melwitt | oh, okay. stephenfin definitely knows more than me about the intention when it was created, so if a list was not meant to work, then I agree, doc bug | |
| 16:52:57 | melwitt | I had thought because of the examples in the config help, it was supposed to work as a list | |
| 16:53:04 | stephenfin | `MultiStrOpt`s are meant to be defined multiple times in a nova.conf. No reason to support defining it multiple ways. More JSON is the last thing anyone needs | |
| 16:53:21 | stephenfin | melwitt: Clearly so did whoever wrote the docs (probably me, tbf) :) It's wrong though | |
| 16:53:37 | melwitt | I see | |
| 16:54:31 | stephenfin | ouch | |
| 16:55:28 | stephenfin | efried: Personally, I'd stick in a comment saying "this means the option is undefined" and leave it at that. There are bigger battles to be fought | |
| 16:55:38 | stephenfin | I'll see if I can clear up the oslo.config docs post-PTG | |
| 16:56:03 | efried | yup, agreed; just want to be encouraging here :) | |
| 16:56:59 | stephenfin | and with that I take my leave. See y'all in Denver o/ | |
| 17:24:01 | cfriesen | do we have any idea when the stein spec freeze would end up? | |
| 17:26:54 | cfriesen | never mind, found it | |
| 17:28:18 | melwitt | cfriesen: I think we're going to discuss that as part of the rocky retro, but I expect it'll be at s-1 | |
| 17:30:13 | melwitt | last time we made it later (2nd milestone) because of runways, but we're going to reassess now that we know how runways went last time http://lists.openstack.org/pipermail/openstack-dev/2018-August/133757.html | |
| 17:42:27 | mriedem | nicolasbock: did you get an answer? | |
| 17:45:18 | mriedem | nicolasbock: if you have an evacuated instance with allocations against multiple resource providers (compute nodes) in placement, and one is against the old source host you evacuated *from*, you should be able to cleanup those bogus allocations using the OSC placement CLI | |
| 17:45:35 | mriedem | https://docs.openstack.org/osc-placement/latest/cli/index.html#resource-provider-allocation-show | |
| 17:59:00 | mriedem | jaypipes: super fun questions about metadata api and config drive in https://review.openstack.org/#/c/599598/ | |
| 18:29:03 | nicolasbock | mriedem, Ah thanks! | |
| 18:30:19 | mriedem | nicolasbock: w/o knowing your full situation, that's my first guess - let me know if that doesn't align with what you're dealing with | |
| 18:30:54 | nicolasbock | mriedem, Thanks. Is there a command line client for placement? | |
| 18:31:04 | nicolasbock | Or is that done directly through the API via curl? | |
| 18:31:23 | mriedem | that's the osc-placement plugin | |
| 18:31:26 | mriedem | in the docs i linked | |
| 18:31:37 | mriedem | https://docs.openstack.org/osc-placement/latest/ | |
| 18:31:54 | mriedem | https://pypi.org/project/osc-placement/ | |
| 18:32:52 | nicolasbock | Thanks! | |
| 19:02:47 | mriedem | so grenade hasn't been working in stein yet just fyi https://review.openstack.org/#/c/591594/ | |
| 19:03:04 | mriedem | good news is you can skip level from queens to stein! | |
| 19:03:10 | mriedem | FFU MISSION ACCOMPLISHED! | |
| 19:03:20 | jroll | congrats | |
| 19:15:53 | mriedem | mnaser: you've been summoned to https://review.openstack.org/#/c/600157/ | |
| 19:16:03 | mriedem | discussion about how extracted placement is going to deal with config | |
| 19:16:21 | mnaser | hi | |
| 19:16:29 | mriedem | right now, as written, deployers would need to copy/rename nova.conf to placement.conf *or* write a fresh placement.conf with the needed stuff that was in nova.conf before | |
| 19:16:43 | mriedem | there is no "if nova.conf: use it; else placement.conf" | |
| 19:17:00 | mriedem | we'll likely be doing the latter in grenade | |
| 19:18:43 | mnaser | well | |
| 19:18:49 | mnaser | we've always deprecated things | |
| 19:19:14 | mnaser | so i don't see why this would be any different | |
| 19:19:33 | mnaser | if anything this is way more critical than any other config disappearing | |
| 19:21:16 | mriedem | you mean as in deprecated but still rely on nova.conf in extracted placement, and drop support for placement using nova.conf in T? | |
| 19:22:00 | mnaser | mriedem: load in order of /etc/placement.conf => /etc/nova/nova.conf in S with a giant warning | |
| 19:22:01 | mnaser | drop that in T | |
| 19:22:33 | mriedem | i believe that's what jaypipes asked for on the review | |
| 19:23:36 | jaypipes | correct. | |
| 19:24:13 | mnaser | i really hope to try and loop in other deployment tools into this | |
| 19:24:15 | mnaser | i.e. tripleo | |
| 19:24:35 | mriedem | beyond config, the other major thing is obviously the db | |
| 19:25:08 | mriedem | if placement does use nova.conf in stein (use/supports as a bridge), it would rely on the [api_database]/connection config | |
| 19:25:57 | mnaser | yeah and also | |
| 19:26:00 | mriedem | looks like that code is still there | |
| 19:26:01 | mnaser | wasnt placement_database a thing | |
| 19:26:07 | mnaser | but i think that was removed cause it wasn't wired | |
| 19:26:09 | mriedem | in openstack/placement, to support that | |
| 19:26:11 | mnaser | i cant remember if it was fixed | |
| 19:26:22 | mriedem | CONF.placement_database is a thing in rocky | |
| 19:26:29 | mriedem | optional thing | |
| 19:26:31 | mnaser | ok so that's back | |
| 19:26:37 | mriedem | yes | |
| 19:26:42 | mnaser | so the logic to determine the db source would be useful there | |
| 19:26:48 | mriedem | if not set, it still uses the nova_api db | |