| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-23 | |||
| 18:41:19 | mriedem | efried: yeah i think i can do that | |
| 18:41:39 | efried | thanks mriedem | |
| 18:42:06 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: [WIP] Support adding the reason behind a server lock https://review.opendev.org/648662 | |
| 18:42:29 | efried | eandersson: Hmph, I see why you did that; perhaps I need a separate bug for min/max | |
| 18:42:45 | efried | as currently stacked, min/max would have to be partial and yours would have to be closes | |
| 18:42:46 | eandersson | Nah I think your bug is the real fix | |
| 18:42:57 | eandersson | Mine is just a fallback bug fix | |
| 18:43:28 | eandersson | but I am fine with separate bug reports as well | |
| 18:43:41 | efried | eandersson: Mine should really have a bug that says, "ksa endpoint lookup never worked because of min_version" | |
| 18:44:05 | efried | Your bug is about region_name not being respected, which is true regardless of my fix. | |
| 18:44:42 | efried | eandersson: You want to open the other or should I? | |
| 18:45:22 | efried | I mean, credit's to you for finding the problem (and pretty much for tracking down the cause too) so... | |
| 18:47:16 | eandersson | Haha | |
| 18:47:23 | eandersson | Actually yours fixes my bug 99.9% | |
| 18:47:25 | eandersson | of the time | |
| 18:47:52 | eandersson | The only time your fix does not fix my bug is if ServiceNotFound is raised | |
| 18:48:09 | efried | which probably would mean that ironicclient would raise the same | |
| 18:48:17 | eandersson | Very likely | |
| 18:49:07 | efried | I can live with that. I can change mine to Partial-Bug and yours can be Closes-Bug. Or we can swap the order of the fixes. But I don't think we're allowed to have Partial-Bug *after* Closes-Bug. mriedem, a ruling? | |
| 18:49:19 | eandersson | The only difference is that the ironicclient is less opiniated (in it's current implementation) | |
| 18:49:27 | eandersson | Sure | |
| 18:49:35 | efried | ...I guess we can break them apart too. There's actually no reason for them to be in a series, I don't think. | |
| 18:50:32 | mriedem | efried: don't think that matters too much, but partial-bug will change the assignee, whereas related-bug wouldn't | |
| 18:50:37 | mriedem | so i'd use related-bug or separate bugs | |
| 18:51:24 | efried | they're truly both fixing part of the bug. | |
| 18:51:39 | eandersson | I assumed that partial-bug wouldn't change the assignee | |
| 18:51:41 | efried | but okay, if the assignee is the only bit of process that matters | |
| 18:51:46 | eandersson | The more you know | |
| 18:52:17 | efried | and we can always change the assignee | |
| 18:52:27 | efried | so effit, let's go as is. | |
| 18:52:31 | eandersson | Sounds good | |
| 18:53:33 | efried | mriedem: series at https://review.opendev.org/#/c/654457/ when you get a chance -- ironic ksa stuff never worked (/me sheepish); but ironicclient covered for us; but wasn't respecting region_name when it did, we were just getting lucky most of the time. | |
| 18:53:51 | efried | what other core can tackle ksa stuff? | |
| 18:53:57 | tssurya | mriedem: silly question - does the addition of an existing key (locked) to filters/sorting have anything to do with microversions ? as in we just allow them without any checks from the moment we implement them right ? | |
| 18:54:15 | eandersson | It was also partially broken on the ironic client side https://github.com/openstack/python-ironicclient/commit/c038f1db67e7809513d5535648d15c3590b191d5 | |
| 18:54:40 | mriedem | tssurya: there is a whitelist on filters and sorting params, | |
| 18:54:54 | tssurya | right, | |
| 18:54:55 | mriedem | specifying filters that aren't in the whitelist are just ignored for now (gmann has a spec to make that an error), | |
| 18:55:03 | mriedem | specifying sort params that aren't in the whitelist is a 400 | |
| 18:55:22 | mriedem | so you can't add locked as a filter/sort param without a microversion bump | |
| 18:55:39 | mriedem | efried: ack | |
| 18:55:59 | mriedem | tssurya: that's how i verified you couldn't just filter/sort on the instance.hidden field in my change | |
| 18:56:01 | tssurya | mriedem: ah because of change in behaviour for sort params? | |
| 18:56:07 | mriedem | yes | |
| 18:56:19 | mriedem | i thought we said in your spec we weren't going to start filtering/sorting on locked? | |
| 18:56:35 | tssurya | oh we did ? :D | |
| 18:56:43 | mriedem | oh nvm | |
| 18:56:45 | mriedem | "Filtering/Sorting: The locked key will be added to the existing list of valid sorting/filtering keys so that instances can be filtered/sorted based on this field." | |
| 18:56:45 | tssurya | the last iteration was that we would | |
| 18:56:51 | mriedem | i might have been thinking of locked_by | |
| 18:56:54 | tssurya | yea | |
| 18:57:06 | tssurya | okay since I have the bump anyways might as well add this too | |
| 18:57:10 | mriedem | yeah so you'll need a microversion | |
| 18:57:10 | tssurya | thanks mriedem | |
| 18:57:14 | mriedem | np | |
| 19:08:33 | mriedem | gmann: you might want to comment on this https://review.opendev.org/#/c/648919/4/specs/train/approved/int-replace-default-value-of-the-flavor-swap.rst@77 | |
| 19:09:00 | mriedem | it's a small inconsistency cleanup in the api spec so people have said it should be merged into your general cleanup spec, but that's just adding more stuff to your already somewhat large api change | |
| 19:23:29 | mriedem | efried: questions in https://review.opendev.org/#/c/654457/ | |
| 19:23:43 | efried | ... | |
| 19:24:21 | efried | eandersson: ^ | |
| 19:25:12 | mriedem | why not squash those 2 changes together? | |
| 19:27:11 | efried | They're pretty unrelated. | |
| 19:27:47 | efried | one fixes how we do ksa stuff, the other fixes how we construct the ironicclient | |
| 19:29:00 | mriedem | it seems the answer to my question here https://review.opendev.org/#/c/654457/2/nova/virt/ironic/client_wrapper.py@131 though is 'ironicclient will try to sort it out' but needs the region_name passed along as well (if you've got multiple regions and are configuring nova-compute to talk to different ones) | |
| 19:31:08 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: [WIP] Support adding the reason behind a server lock https://review.opendev.org/648662 | |
| 19:31:51 | efried | mriedem: Yes. | |
| 19:32:00 | efried | Though I think that was actually not the case until recently. | |
| 19:32:16 | efried | I think the logic to "figure it out" was broken until https://github.com/openstack/python-ironicclient/commit/c038f1db67e7809513d5535648d15c3590b191d5 ? | |
| 19:33:58 | efried | mriedem: so, do you want these squashed? Am I being silly asserting that they're doing different things? | |
| 19:37:05 | mriedem | well, it looks like it might be better to keep them separate because of how ironicclient handles the kwargs in older releases, it looks like region_name was only since Ifc7b45d047c8882a41021e1604b74d17eac2e6e8 in rocky and before that it was os_region_name | |
| 19:37:15 | mriedem | so backports to queens for eandersson's change could be a bit trickier | |
| 19:37:21 | mriedem | especially since we don't have integration testing for this stuff | |
| 19:41:24 | mriedem | ok +2 on the bottom and +W on the top | |
| 19:41:39 | efried | thanks mriedem. Any idea who else should look at the bottom one? | |
| 19:41:52 | mriedem | https://review.opendev.org/#/q/topic:bug/1825583+(status:open+OR+status:merged) | |
| 19:41:54 | mriedem | cleans that up | |
| 19:41:57 | mriedem | you guys and your silly tp's | |
| 19:42:00 | mriedem | *tb's | |
| 19:42:09 | mriedem | efried: ummm, melwitt? | |
| 19:42:40 | artom | Wheel of cortune | |
| 19:42:46 | efried | nice | |
| 19:43:18 | artom | That's basically the extent of my contribution here. Puns. | |
| 19:49:58 | eandersson | Not backporting mine beyond Rocky is fine, as the main patch will fix most cases. | |
| 19:50:13 | eandersson | And endpoint_override is a valid work around. | |
| 20:00:11 | mriedem | and you're on rocky so you don't have to care about queens :) | |
| 20:13:11 | mriedem | it's sad that something as simple as "pass az to unshelve" https://review.opendev.org/#/c/624689/ is this complicated | |
| 20:16:15 | mriedem | sorrison: i think this old bug https://bugs.launchpad.net/nova/+bug/1453629 has been fixed since pike due to https://review.opendev.org/#/c/446053/ - are you on pike or later and can confirm? | |
| 20:16:20 | openstack | Launchpad bug 1453629 in OpenStack Compute (nova) "Creating neutron ports uses incorrect instance availability zone" [Low,Confirmed] | |
| 20:18:52 | colby_ | Hey All. Is there any way to have shelved_offloaded instances not count toward cpu hours/ram hour usage? | |
| 20:19:56 | aspiers | efried: if resources:foo=1 doesn't currently work with image properties, I'm unsure how it would work when translating another image property gimme_foo=true into it within request_filter.py | |
| 20:21:20 | aspiers | efried: wouldn't we have to add support to ResourceRequest.from_image_props() at least? | |
| 20:22:21 | sean-k-mooney | aspiers: you are not allowed to request resoruce from an image | |
| 20:23:06 | sean-k-mooney | and i dont think that should be part of teh SEV spec if we intoduce it | |
| 20:24:18 | aspiers | sean-k-mooney: is the term "resource" overloaded here? | |
| 20:24:53 | sean-k-mooney | i assume resouce:foo was a placment resouce class request | |
| 20:24:55 | aspiers | sean-k-mooney: by the way it's named, ResourceRequest.from_image_props() considers an image property requesting a trait as a resource request | |
| 20:25:02 | sean-k-mooney | that is not allowed in an image | |
| 20:25:37 | aspiers | so in that naming, a "resource" can also refer to something wider which can include traits, not just something from a resource class | |
| 20:25:47 | sean-k-mooney | is that in the nova code? | |
| 20:25:51 | aspiers | yes | |