| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-10-01 | |||
| 14:54:45 | openstackgerrit | Stephen Finucane proposed openstack/python-novaclient stable/train: Stop silently ignoring invalid 'nova boot --hint' options https://review.opendev.org/685980 | |
| 14:55:30 | openstackgerrit | Stephen Finucane proposed openstack/python-novaclient stable/stein: Stop silently ignoring invalid 'nova boot --hint' options https://review.opendev.org/685981 | |
| 14:55:51 | openstackgerrit | Stephen Finucane proposed openstack/python-novaclient stable/rocky: Stop silently ignoring invalid 'nova boot --hint' options https://review.opendev.org/685983 | |
| 14:57:26 | mriedem | stephenfin: i initially thought 'fixes' but could see 'upgrade' as well | |
| 14:57:45 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add a way to spy on function calls in test https://review.opendev.org/685949 | |
| 14:59:01 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Replace mock with spy in test https://review.opendev.org/685950 | |
| 14:59:02 | openstackgerrit | Stephen Finucane proposed openstack/python-novaclient stable/train: Stop silently ignoring invalid 'nova boot --hint' options https://review.opendev.org/685980 | |
| 14:59:32 | openstackgerrit | Stephen Finucane proposed openstack/python-novaclient stable/stein: Stop silently ignoring invalid 'nova boot --hint' options https://review.opendev.org/685981 | |
| 15:00:10 | openstackgerrit | Stephen Finucane proposed openstack/python-novaclient stable/rocky: Stop silently ignoring invalid 'nova boot --hint' options https://review.opendev.org/685983 | |
| 15:25:59 | openstackgerrit | Merged openstack/nova master: ec2: Move ec2utils functions to their callers https://review.opendev.org/662505 | |
| 15:32:42 | stephenfin | gibi: Since you looked at the functional change, care to send this reno on its way, please? https://review.opendev.org/685986 | |
| 15:33:49 | gibi | stephenfin: sure, looking | |
| 15:33:56 | stephenfin | Thanks | |
| 15:34:36 | artom | gibi, a couple of -1s on your spy stack, but they're really more for visibility/discussion | |
| 15:34:37 | gibi | stephenfin: that was an easy one :) | |
| 15:35:09 | stephenfin | gibi: hang on, pretty sure I've a 1kloc patch sitting around here somewhere ;) | |
| 15:35:27 | gibi | artom: yeah, that is basically an experiment, so any discussion starter is warmly welcome. I will check back to that patch later | |
| 15:35:44 | gibi | stephenfin: I'm sure you have one :) | |
| 15:35:54 | stephenfin | I probably do :( | |
| 15:38:19 | artom | gibi, ack, no rush :) | |
| 15:41:25 | openstackgerrit | Merged openstack/python-novaclient stable/train: Update .gitreview for stable/train https://review.opendev.org/683625 | |
| 15:41:43 | efried | mriedem, stephenfin: Can I get some help/advice moving forward with unauthed versioned discovery? https://review.opendev.org/685181 | |
| 15:42:20 | efried | I've got this working, but there are a couple of points I'd like to discuss. | |
| 15:42:21 | mriedem | still no alex or john around huh | |
| 15:42:36 | efried | Alex is on vacation. John I haven't seen in a while. | |
| 15:43:03 | stephenfin | I can try, but this is not my wheelhouse | |
| 15:43:17 | efried | oh, sure you're fine removing an unused middleware though | |
| 15:43:24 | efried | :P | |
| 15:43:25 | efried | so first | |
| 15:43:57 | stephenfin | deleting code is a significantly easier endeavour ;) | |
| 15:44:09 | efried | mordred: your advice would be helpful here too | |
| 15:44:39 | efried | http://host:port/v2 and /v2.1 used to do a 302 redirect to .../v2/ and /v2.1/ (with the trailing slash). The trailing-slash version then returned the version document with a 200. | |
| 15:44:57 | efried | Now /v2, /v2.1, /v2/, and /v2.1/ all just return the version document with 200. | |
| 15:45:15 | efried | Is that a behavior change that could possibly cause a problem for anyone? | |
| 15:45:48 | efried | perhaps gmann could also advise? | |
| 15:46:26 | mordred | efried: it shouldn't cause a problem for SDK - we treat the version document payload as authoritative | |
| 15:46:54 | mordred | but - if there are people who are doing blind url appending - maybe that's a behavior change that would break them? | |
| 15:47:05 | mriedem | i want to say novaclient has had to deal with the trailing slash stuff to get the version document and avoid the 302 | |
| 15:47:08 | efried | what do you mean, blind url appending? | |
| 15:47:19 | mriedem | i think sdague added that and alex is probably familiar with it | |
| 15:47:40 | efried | mriedem: so worst case novaclient is doing an extra thing that's no longer necessary, rite? | |
| 15:48:09 | mriedem | probably, details are in here https://github.com/openstack/python-novaclient/blob/003ac57d9af74aa4658a7bf6cc6b6b3bafa58c11/novaclient/v2/versions.py#L36 | |
| 15:48:28 | efried | mordred: /v2/stuff will still go into the real API, not hit version discovery | |
| 15:48:43 | mordred | ah - yeah | |
| 15:49:18 | mordred | I mean - then in that case, I think it's no problem - because what a person is looking for is the version document | |
| 15:49:23 | mordred | and they'll be getting it | |
| 15:49:38 | mriedem | i also remember this craziness that andreykurilin fixed https://github.com/openstack/python-novaclient/blob/003ac57d9af74aa4658a7bf6cc6b6b3bafa58c11/novaclient/v2/versions.py#L83 | |
| 15:49:40 | mordred | I agree with "novaclient might be doing an extra step it doesn't need to do anymore" part | |
| 15:49:42 | stephenfin | efried: I assume there's a very good reason that it's not possible to retain the 302 redirect? | |
| 15:50:12 | mordred | stephenfin: bunny rabbits | |
| 15:50:14 | efried | mriedem: yeah, so that novaclient blip would become unnecessary - but only for deployments actually running this code, so I'm not gonna like rip it out right away or anything. | |
| 15:50:45 | mordred | yah- that's why we're fixing sdk to DTRT with older nova too | |
| 15:50:45 | mriedem | more like ever | |
| 15:51:02 | mriedem | api-paste is config and deployments can do whatever, even if it's dumb | |
| 15:51:05 | mriedem | see the comments in there about rax | |
| 15:51:49 | efried | stephenfin: I suppose with some extra hacking I could figure out a way to re-add the 302, but if it's not problematic to remove it, this is way simpler, less code, etc. | |
| 15:52:19 | stephenfin | gotcha | |
| 15:52:21 | efried | mriedem: that second novaclient blip should also continue to work fine. | |
| 15:52:22 | mriedem | i have no idea if external tooling things would rely on that 302 actually happening | |
| 15:52:32 | efried | yeah, that would be just... crazy I would think. | |
| 15:52:39 | efried | but it's what I'm asking. | |
| 15:52:47 | efried | Second thing: | |
| 15:53:19 | efried | Do we have any need to retain full compatibility for [api]auth_strategy=noauth2 in the wild? | |
| 15:55:22 | efried | I had to change NoAuthMiddleware a little bit because it used to be relying (hardcoded) on the auth_url being the versioned endpoint -- i.e. http://server:port/v2 or .../v2.1 -- because it would return that in the X-Server-Management-Url header. | |
| 15:55:40 | efried | But the whole point of the change was to make those URLs *not* go through auth middleware | |
| 15:56:35 | efried | so I had to make it so you use a different auth_url, and NoAuthMiddleware (still hardcoded) massages it appropriately so you still get the right X-Server-Management-Url header back. | |
| 15:57:27 | mriedem | noauth is for tests only | |
| 15:58:04 | efried | good, that's what I wanted to hear. | |
| 15:58:06 | mriedem | https://docs.openstack.org/nova/latest/configuration/config.html#api.auth_strategy | |
| 15:58:10 | mriedem | it's also in the config option help | |
| 15:58:42 | efried | Yeah, I just didn't know if the fact that we mention it in the conf help means we kinda have to support it and not break existing uses | |
| 15:59:05 | efried | Cool, so the last thing: | |
| 15:59:18 | efried | and this is obvious, because it's the point of the patch: | |
| 15:59:37 | efried | You no longer go through auth to get the versioned discovery documents | |
| 15:59:51 | mriedem | WHAT?! | |
| 16:00:09 | efried | I think it's possible this means there are some headers you used to get back that you wouldn't anymore. | |
| 16:02:11 | efried | We seem to have tests for things we care about, like Accept. | |
| 16:02:38 | efried | I suppose it would be duly diligent of me to go find out exactly what that diff looks like... | |
| 16:05:01 | efried | thanks for the talk mriedem mordred stephenfin | |
| 16:10:57 | openstackgerrit | Merged openstack/python-novaclient stable/train: Update TOX/UPPER_CONSTRAINTS_FILE for stable/train https://review.opendev.org/683626 | |
| 16:11:00 | openstack | bug 1845322 in python-novaclient train "'--hint' argument to 'nova boot ' should be a key-value pair" [Low,In progress] https://launchpad.net/bugs/1845322 - Assigned to Stephen Finucane (stephenfinucane) | |
| 16:11:00 | openstackgerrit | Merged openstack/python-novaclient master: Add release note for bug 1845322 https://review.opendev.org/685986 | |
| 16:12:33 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add Selection.availability_zone field https://review.opendev.org/685807 | |
| 16:12:34 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Handle get_host_availability_zone error during reschedule https://review.opendev.org/685997 | |
| 16:12:35 | openstack | bug 1781286 in OpenStack Compute (nova) "CantStartEngineError in cell conductor during reschedule - get_host_availability_zone up-call" [Medium,Triaged] https://launchpad.net/bugs/1781286 - Assigned to Matt Riedemann (mriedem) | |
| 16:12:35 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add functional regression test for bug 1781286 https://review.opendev.org/685998 | |
| 16:26:41 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Do not print default dicts during heal_allocations https://review.opendev.org/686001 | |
| 16:29:45 | gibi | mriedem: finally ticked off this TODO as well ^^ | |
| 16:32:02 | mriedem | ack, would be nice to see that in https://review.opendev.org/#/c/669879/ but i won't make you rebase it | |
| 16:32:23 | mriedem | maybe time to nudge efried and/or stephenfin to come back on ^ | |
| 16:32:29 | mriedem | though stephen had comments | |
| 16:33:27 | mriedem | unrelated, while writing a resize reschedule test i realize that if we fail the resize claim we temporarily put the instance into ERROR state while rescheduling, and it will end up that way if rescheduling fails, which seems wrong b/c we didn't actually do anything with the guest, we just failed a claim | |
| 16:33:31 | mriedem | dansmith: agree? ^ | |
| 16:34:15 | dansmith | agree with what? that leaving it in error state is wrong? | |
| 16:34:19 | mriedem | yeah | |
| 16:34:33 | mriedem | we should just fail the resize, mark the migration as error, inject a fault, etc | |
| 16:34:38 | mriedem | but the vm_state should still be active or stopped | |
| 16:35:50 | dansmith | I dunno, I guess I thought it was always a design goal that ERROR is your indication that something failed, and you start or stop the instance to get it back to normal state | |
| 16:36:26 | mriedem | that's a pretty big hammer for a user when resize fails b/c they lost a claim race | |
| 16:36:30 | mriedem | and the guest is otherwise untouched | |