| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-03-27 | |||
| 13:45:56 | gibi | there is at least a separate patch that renames the existing update policy | |
| 13:46:16 | dansmith | okay I started on that and yeah, okay | |
| 13:46:17 | gibi | this is the policy rename https://review.opendev.org/#/c/715356/ | |
| 13:46:48 | gibi | the rest is in a stack starting at https://review.opendev.org/#/c/693828/19 | |
| 13:46:59 | dansmith | yep, I got it | |
| 13:47:09 | dansmith | this needs a microversion rebase anyway | |
| 13:47:29 | dansmith | gibi: it might be good to start lining up any other patches that have a microversion so we're not spending lots of time re-versioning things | |
| 13:47:45 | gibi | true, and good point | |
| 13:48:12 | owalsh | sean-k-mooney: hey, re https://bugs.launchpad.net/nova/+bug/1677913 | |
| 13:48:14 | gibi | at least I haven't approved any new microversion bumping patch today | |
| 13:48:14 | openstack | Launchpad bug 1677913 in OpenStack Compute (nova) "percent must be escaped in live_migration_uri" [Low,In progress] - Assigned to Stephen Finucane (stephenfinucane) | |
| 13:49:01 | owalsh | sean-k-mooney: problem is that nova is doing string interpolation e.g uri % dest | |
| 13:49:22 | owalsh | which is fine for qemu+ssh://%s:123/ | |
| 13:49:26 | sean-k-mooney | owalsh: yes but you should not be url encoding extra params unless they are unsupportd | |
| 13:49:35 | sean-k-mooney | e.g. once that libvirt does not know about | |
| 13:49:50 | owalsh | sean-k-mooney: and url endeded extra params contain % | |
| 13:49:52 | sean-k-mooney | if they are part of the standard set you dont need to encode them | |
| 13:50:21 | sean-k-mooney | owalsh: then ooo should have escaped them properly | |
| 13:50:46 | owalsh | sean-k-mooney: nope, we had to double escape them | |
| 13:51:09 | owalsh | i.e %% so the nova sting interpolation replaces it with a single % | |
| 13:51:10 | sean-k-mooney | owalsh: so you had to put 4 % | |
| 13:51:26 | sean-k-mooney | owalsh: yes im saying that is the correct thing to do | |
| 13:51:58 | sean-k-mooney | owalsh: nova reuired you to provide a vail python sting that will be interperalted | |
| 13:52:10 | sean-k-mooney | so ooo should have escaped the % | |
| 13:53:45 | sean-k-mooney | owalsh: looking at the standard extra paramater i dont think any of them shoudl reuire a % | |
| 13:53:47 | sean-k-mooney | https://libvirt.org/uri.html#Remote_URI_parameters | |
| 13:53:54 | sean-k-mooney | which one was ooo trying to use | |
| 13:53:55 | dansmith | gibi: gmann it looks like the put expects the client to remove the 'device' from the put? meaning if it does a get on the thing, then it has to remove 'device' before it can put that back... | |
| 13:54:28 | dansmith | gibi: gmann that makes no sense to me, but hopefully it's okay to change that in the new microversion to just be ignored so you can get/put the thing REST-style? | |
| 13:54:35 | owalsh | sean-k-mooney: erm, it's been years.. probably ssh key path | |
| 13:54:50 | sean-k-mooney | owalsh: why would that contain a % | |
| 13:55:30 | sean-k-mooney | while technically legal in a file name it would not be common or expect practic to see a % in a path | |
| 13:55:38 | owalsh | sean-k-mooney: libvirt docs say "Note that parameter values must be URI-escaped" | |
| 13:55:50 | sean-k-mooney | no read the full sentence | |
| 13:56:07 | owalsh | pretty sure it means all parameter values | |
| 13:56:19 | sean-k-mooney | Any others are passed unmodified through to the back end | |
| 13:56:44 | sean-k-mooney | it only applies to the parmater not supproted in the table below as far as i know | |
| 13:57:14 | owalsh | that would be weird TBH, not a valid uri | |
| 13:57:27 | sean-k-mooney | a uri is not a url | |
| 13:57:27 | gibi | dansmith: I think that is an artifact of the implementation, the device gets automatically calculated | |
| 13:57:32 | gmann | dansmith: volume attachment PUT right ? | |
| 13:57:35 | nightmare_unreal | mriedem: ping | |
| 13:57:39 | sean-k-mooney | they do not have the same constratits on formating | |
| 13:58:26 | dansmith | gibi: I know in create, but on update you should be able to GET, modify, PUT, without having to delete things.. things that aren't update-able that are unchanged are just retained | |
| 13:58:28 | dansmith | gmann: yes | |
| 13:58:42 | gmann | dansmith: yeah we restrict it on schema side- https://github.com/openstack/nova/blob/e78343dcff7feb0975ffd10ff41ab4368611c285/nova/api/openstack/compute/schemas/volumes.py#L95 | |
| 13:59:01 | owalsh | sean-k-mooney: wasn't just ooo that hit this FWIW, I'll try to dig up the info from back then | |
| 13:59:27 | dansmith | gmann: exactly.. I'm saying we should add that back in in the new microversion | |
| 13:59:28 | gibi | dansmith: you are right about the generic PUT behavior. However I'm not sure if there is a swap volume nova will keep the device name the same | |
| 13:59:54 | dansmith | gibi: I know, you can't *change* it, you should just be allowed to pass it | |
| 14:00:28 | gibi | it might be that you pass the same value in via PUT (swap case) but as a result the value of the device field changes by nova | |
| 14:00:41 | dansmith | sure, fine | |
| 14:00:47 | dansmith | but right now, it's not allowed by schema | |
| 14:01:06 | dansmith | so in the new microversion I want it to be allowed, just not change-able, so you can PUT what you GET | |
| 14:01:37 | mriedem | allowing a field that you can't change in a PUT request is weird | |
| 14:01:48 | dansmith | eh? | |
| 14:02:11 | gibi | so if cinder will not provide the device then nova will reject? | |
| 14:02:11 | dansmith | that's the whole point of PUT, that you're PUTting the whole thing back, otherwise it'd be a PATCH | |
| 14:02:18 | dansmith | gibi: no,we can keep it optional | |
| 14:02:26 | gibi | dansmith: OK, in that case I'm fine | |
| 14:02:30 | gibi | with the change | |
| 14:02:35 | dansmith | cool | |
| 14:02:40 | sean-k-mooney | owalsh: dont waste time looking it up. the patch is going to proceed anyway. its way to late to change the direction at this point since we alerady added other config options | |
| 14:03:16 | gmann | other PUT are same like PUT server etc | |
| 14:03:23 | dansmith | mriedem: to be clear, I can enforce that you gave me the same value I gave you (i.e. you didn't change it), the client just shouldn't have to trim out the result of the GET to be able to PUT it back | |
| 14:03:24 | owalsh | sean-k-mooney: ack. I'm pretty sure I ran all of that past danpb at the time, and he probably wrote those docs | |
| 14:03:26 | dansmith | gmann: exactly | |
| 14:03:29 | sean-k-mooney | owalsh: i just dont think this was a bug. it was a feature request and the original bug was in ooo | |
| 14:03:46 | owalsh | sean-k-mooney: nova doc bug at the very least | |
| 14:04:01 | sean-k-mooney | owalsh: yes that too | |
| 14:04:33 | gmann | dansmith: and 400 if non-updatable things are passed with change in value ? | |
| 14:04:42 | dansmith | gmann: yep | |
| 14:05:56 | gmann | ok then it will not create confusion to client that what he/she requested for change are done or not. that I was thinking. so accepting all and 400 for non-updatable if requested lgtm | |
| 14:06:12 | dansmith | cool | |
| 14:06:19 | gmann | and you mean for all PUT to make consistent not just attachment one ? | |
| 14:07:12 | dansmith | I don't understand the question | |
| 14:07:41 | gmann | i mean other APIS like PUT servers etc, other PUT i have to check what they accept. | |
| 14:07:52 | dansmith | I think all our PUTs should behave that way, I'm sure not all of them do, but when we have a chance (like this) to make it behave this way, then we should | |
| 14:08:40 | gmann | yeah, then it make consistent. Aslo we replicate the GET response to PUT at least for server we do so doing same in request make sense also. | |
| 14:09:00 | dansmith | yes | |
| 14:09:53 | openstackgerrit | jayaditya gupta proposed openstack/nova master: Support for --overwrite flag for nova-manage placement heal_allocations command https://review.opendev.org/715395 | |
| 14:10:39 | mriedem | nightmare_unreal: pong | |
| 14:11:23 | nightmare_unreal | mriedem: can you leave comment for my patch : https://review.opendev.org/#/c/715395/ | |
| 14:11:39 | nightmare_unreal | mriedem: I think i have made some mistake with test case | |
| 14:11:57 | nightmare_unreal | if you can review it :) that will be helpful | |
| 14:12:22 | nightmare_unreal | also what does greynade-py3 checks/do | |
| 14:13:41 | mriedem | grenade job is for upgrade testing, | |
| 14:14:16 | mriedem | so for ussuri changes it installs train devstack, runs smoke tests, saves off some resources (servers/volumes), then upgrades to master devstack, runs smoke tests and makes sure those saved resources are still available | |
| 14:14:31 | mriedem | https://docs.openstack.org/grenade/latest/readme.html | |
| 14:15:46 | gmann | dansmith: gibi added in API cleanup etherpad. L81: https://etherpad.openstack.org/p/nova-api-cleanup | |
| 14:16:51 | gibi | gmann: ack, thanks | |
| 14:22:19 | openstackgerrit | sean mooney proposed openstack/nova master: [WIP] cyborg evacuate support https://review.opendev.org/715326 | |
| 14:25:41 | mriedem | nightmare_unreal: comments inline | |
| 14:25:54 | nightmare_unreal | thank you :D | |
| 14:26:00 | mriedem | nightmare_unreal: remember to run `tox -e flake8` before pushing up your changes | |
| 14:26:07 | mriedem | err, | |
| 14:26:10 | mriedem | tox -e fast8 | |
| 14:26:39 | nightmare_unreal | I'll remember that . So far I was running only tox -e pep8 , tox-e py36 | |
| 14:27:10 | stephenfin | or do 'pre-commit install --allow-missing-config' | |
| 14:27:19 | stephenfin | then it'll run automatically whenever you make a commit | |