| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-13 | |||
| 13:45:41 | mriedem | Shilpa: ok well nova doesn't use taskflow. harlowja was the primary maintainer but i'm not sure if he's around anymore. oslo team would be your best bet i'd think, otherwise maybe #openstack-dev | |
| 13:45:58 | mriedem | efried loves taskflow though | |
| 13:46:49 | Shilpa | mriedem: ok, hope he will help me there | |
| 13:47:37 | mriedem | tssurya: my concern is if tooling is passing filter parameters by default, | |
| 13:47:50 | mriedem | normally in the API we filter out search options that the user isn't allowed to use without it being an error | |
| 13:48:18 | mriedem | so let's say i'm a non-admin and i pass some stuff which normally just gets filtered out in the remove_invalid_options method | |
| 13:48:25 | mriedem | essentially making search_opts = {} | |
| 13:48:41 | tssurya | like "deleted" and "tenant" ? | |
| 13:48:46 | mriedem | if i'm using 2.69, then empty search_opts would mean down_cell_support=True | |
| 13:48:59 | mriedem | tssurya: non-admins can't use the tenant filter | |
| 13:49:10 | tssurya | yea I know | |
| 13:49:12 | mriedem | servers are filtered by default from the project_id in the request context | |
| 13:49:27 | tssurya | which is why I was asking if that's what you mean by invalid options | |
| 13:50:04 | mriedem | so let's say i'm a non-admin and i'm using --deleted, here are some scenarios | |
| 13:50:25 | mriedem | 1. nova list --deleted with v2.1 - the deleted filter would be excluded w/o any errors in the api and i should list my servers | |
| 13:50:51 | mriedem | 2. nova list --deleted with v2.69 and list_records_by_skipping_down_cells=True (default), the down_cell_support flag would be set to False and i wouldn't get any results (if my servers are in a down cell) | |
| 13:51:06 | mriedem | 3. nova list --deleted with v2.69 and list_records_by_skipping_down_cells=False, if my servers are in a down cell i'll get a 500 | |
| 13:51:45 | mriedem | my point was if we calculated the down_cell_support flag *after* calling remove_invalid_options to filter the search_opts, then #3 becomes #2 | |
| 13:52:00 | mriedem | actually no that's not right, | |
| 13:52:14 | mriedem | i'd get partial results | |
| 13:53:14 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Restore connection_info after live migration rollback https://review.openstack.org/551349 | |
| 13:53:16 | mriedem | to summarize, i think the down_cell_support variable should be set based on the search opts after we've filtered that set in remove_invalid_options to increase the chances of someone getting partial results | |
| 13:53:17 | lyarwood | mdbooth: ^ btw, feel free to respin the commit message if you have time. | |
| 13:53:55 | tssurya | mriedem: ok, I get what you mean | |
| 13:54:21 | mriedem | the tricky thing is if you do that, you have to explicitly look for paging and sorting parameters | |
| 13:54:30 | mriedem | because of https://review.openstack.org/#/c/591657/39/nova/api/openstack/compute/servers.py@1235 | |
| 13:54:38 | tssurya | yea | |
| 13:54:55 | mriedem | but i dont think that's so hard, we could just create a constant for ('sort_key', 'sort_dir', 'limit', 'marker') and use that in both places | |
| 13:55:34 | mriedem | and your _is_cell_down_supported method can check, "if any(PAGING_SORTING_PARAMS in reg.GET) | |
| 13:57:01 | tssurya | got it, let me dig more and incorporate that | |
| 13:57:17 | mriedem | err, if set(PAGING_SORTING_PARAMS) & set(list(req.GET.keys())) | |
| 13:57:20 | mriedem | something like that | |
| 13:57:33 | mriedem | does that make sense though? | |
| 13:57:45 | tssurya | it makes sense yea | |
| 13:57:59 | openstackgerrit | Hamdy Khader proposed openstack/os-vif master: Add create_ovs_port field in VIFPortProfileOpenVSwitch profile https://review.openstack.org/636061 | |
| 13:58:01 | tssurya | but I still don't get why the policy things are mixed with ignoring flags | |
| 13:58:06 | mriedem | cool. my goal is to maximize the chance that someone gets partial results if they have servers in a down cell | |
| 13:58:29 | mriedem | which policy things? | |
| 13:58:37 | tssurya | I mean if --deleted is an only admin option | |
| 13:58:39 | tssurya | it shuld barf | |
| 13:58:42 | tssurya | should* | |
| 13:58:46 | tssurya | saying you are not admin | |
| 13:58:56 | mriedem | legacy backward compat | |
| 13:58:59 | tssurya | just lke tenant or user | |
| 13:59:03 | tssurya | ahhh ok ok | |
| 13:59:06 | mriedem | same reason you can do GET /servers?foo=bar | |
| 13:59:36 | mriedem | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/servers.py#L606 | |
| 13:59:54 | openstackgerrit | Stephen Finucane proposed openstack/python-novaclient master: Microversion 2.68: Remove 'forced' live migrations, evacuations https://review.openstack.org/635131 | |
| 13:59:59 | mriedem | the query parameter schema is whitelisted but also allows users to pass anything for backward compat, | |
| 14:00:11 | mriedem | except the stuff in https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/servers.py#L512 | |
| 14:00:30 | mriedem | those are 404ed here https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L125 | |
| 14:01:34 | tssurya | mriedem: wow its crazy, but I understand at least why it was done, thanks for patiently explaining that | |
| 14:01:48 | tssurya | I'll work on ironing out the wrinkles | |
| 14:02:19 | mriedem | cool. otherwise from my manual testing (once i worked around those other bugs with the service version checking) it was working pretty well | |
| 14:02:57 | stephenfin | mriedem: Not sure you already discussed this again with kashyap, but any serious objections to doing the libvirt min version bump now? https://review.openstack.org/#/c/632507/ gibi and I are happy but I recall some objections previously | |
| 14:03:12 | kashyap | stephenfin: Heh, was _just_ about to write a message here. | |
| 14:03:26 | kashyap | stephenfin: Not quite objections, mriedem was not sure, based on some discussion w/ an operator he had | |
| 14:03:53 | kashyap | Ideally, I should have done this at the _start_ of the cycle, but completely lost track of it | |
| 14:04:24 | mriedem | looking, but you should probably ask the PTL | |
| 14:04:27 | adrianc | sean-k-mooney: Hi, regarding sriov live-migration patches, id like to address some of the comments in https://review.openstack.org/#/c/620115/21, however i saw you are planning to upload a new PS for the series, will it be soon ? as id like to avoid merge conflicts etc... | |
| 14:04:58 | kashyap | mriedem: Yeah, will do. I thought melwitt was still catching up after being on PTO (?) | |
| 14:05:28 | mriedem | stephenfin: kashyap: the concern from an operator was they were upgrading from mitaka to i think queens and the minimum bump in between was just for maintenance, not related to any functional changes, and the computes they were upgrading didn't have that minimum libvirt (they were using older centos i think) | |
| 14:06:01 | mriedem | so this operator was asking me if there was a reason for the bump, or if they could just revert the change to keep the older centos compute node but still upgrade nova | |
| 14:06:24 | sean-k-mooney | adrianc: i got pulled into some other stuff so go ahead. | |
| 14:06:28 | kashyap | I see. Yeah, probably we should better advertize _why_ we do bumps and at what intervals? | |
| 14:06:48 | kashyap | mriedem: ^ Like me writing a reminder note to the mailing list about why we do, and give a gentle heads-up. | |
| 14:06:55 | mriedem | i just think in years past we weren't real aggressive about doing it every release | |
| 14:07:09 | sean-k-mooney | adrianc: ill work on adressing the base patches but ill mainly be adressing comment/commit messages so there whould be no conflicts | |
| 14:07:27 | kashyap | mriedem: Yes, I now kept a reminder on my phone to do the start of the cycle | |
| 14:07:33 | adrianc | sean-k-mooney: would you like me to address some of the nits in https://review.openstack.org/#/c/624842/ as well, or ill leave them to you ? | |
| 14:08:59 | mriedem | so looking at https://review.openstack.org/#/c/558171/ the last time we did a bump was pike, is that correct? | |
| 14:09:05 | sean-k-mooney | adrianc: am if you want to adress them i can focous on the os-vif brctl removal which is the other thing im context switch with | |
| 14:09:17 | mriedem | we advertized the next minimums in rocky | |
| 14:09:25 | mriedem | so really this would be the first required minimum bump since pike | |
| 14:09:34 | kashyap | mriedem: Yes, I did the bump last April-May (https://review.openstack.org/#/q/topic:bump_min_libvirt_and_qemu+(status:open+OR+status:abandoned+OR+status:merged) | |
| 14:09:36 | mriedem | giving at least a year to prepare | |
| 14:09:47 | kashyap | mriedem: Indeed | |
| 14:09:53 | mriedem | (knowing people skip several years of openstack releases now) | |
| 14:10:07 | sean-k-mooney | adrianc: i dont think there is anything controvertiol in any of the coments on the seirs so if you have time to make the requested change go for it | |
| 14:10:38 | mriedem | looks like we don't have zkvm 3rd party CI anymore... | |
| 14:10:45 | kashyap | Release-wise, we're just about to reach M3 (Mar 04-08), and the GA is aroud 12-Apr — I feel we're still okay | |
| 14:11:05 | adrianc | sean-k-mooney: Ack, ill start from the base patch then :) | |
| 14:11:11 | kashyap | mriedem: Yeah, was wondering this morning about it | |
| 14:11:17 | mriedem | https://wiki.openstack.org/wiki/ThirdPartySystems/IBM_zKVM_CI | |
| 14:11:33 | kashyap | Today I spent 4 hours digging into s390x and their fun serial console stuff | |
| 14:11:49 | openstackgerrit | Alexandra Settle proposed openstack/nova master: Adding cross refs for config options in scheduler filter guide https://review.openstack.org/636635 | |
| 14:11:52 | mriedem | kashyap: so how about you send a reminder to the mailing list and also copy zkvm-ci@linux.vnet.ibm.com and bjzhjing@linux.vnet.ibm.com | |
| 14:11:56 | kashyap | MarkusZ left a note in the code with a pointer to a bug ... and it turned out it's a bug _I_ filed 4 years ago :P | |
| 14:12:17 | mriedem | markus_z is long gone from openstack | |
| 14:12:21 | kashyap | mriedem: I guess you mean now, right? | |
| 14:12:35 | mriedem | kashyap: yeah, i'd say do that now and if we don't hear any strong objections by end of week we go ahead | |
| 14:12:40 | kashyap | mriedem: I know, I just found it sadly funny that when I decided to learn more from the bug ... apparently I filed it. | |
| 14:12:45 | kashyap | mriedem: Alright, let me draft it right away. | |
| 14:13:04 | kashyap | mriedem: I spent a painful 8 days on-and-off arriving at the root cause. But hey, fun :-) | |
| 14:13:37 | kashyap | Thanks for your time. Appreciate it | |
| 14:13:38 | mriedem | you've grown as a person i'm sure | |
| 14:14:18 | kashyap | I did; I grew in "(positive) complexity" :D | |
| 14:15:42 | kashyap | What tags would I use? [ops][nova] suffices I guess? | |
| 14:15:48 | kashyap | s/suffices/suffice/ | |
| 14:16:34 | mriedem | yeah | |