| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-03 | |||
| 21:12:39 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add cross-cell resize policy rule and enable in API https://review.openstack.org/638269 | |
| 21:14:00 | mriedem | dansmith: i added a test for this cross-cell resize issue i ran into yesterday: https://review.openstack.org/#/c/643451/5/nova/tests/functional/test_cross_cell_migrate.py@499 - tl;dr is i'm going to need to hard destroy the instance in the db, instance.destroy() won't cut it | |
| 21:14:38 | mriedem | for example, resize to target cell and then revert back to source, then try to resize again to target will fail (that's that test) because there is a (soft) deleted instance record in the target cell db still | |
| 21:15:02 | mriedem | and the uuid unique constraint will prevent us from creating the instance in the target cell db on the 2nd attempt | |
| 21:15:25 | mriedem | same for rollbacks on failure (hard destroy in target cell db) and confirm resize (hard destroy from source cell db) | |
| 21:15:50 | mriedem | i think it's probably ok, although it sounds kind of scary - but you'll always have a copy of the instance in one of the db when the operation is over, so we don't lose anything | |
| 21:16:39 | mriedem | a real shitty alternative limitation/workaround is that it's just a no-go for cross-cell resizing that instance until the problem db is purged of the deleted instance | |
| 21:30:31 | melwitt | mriedem: thx | |
| 21:33:21 | efried | mriedem: http://logstash.openstack.org/#dashboard/file/logstash.json?query=message%3A%5C%22Deadlock%20found%20when%20trying%20to%20get%20lock%3B%20try%20restarting%20transaction%5C%22%20AND%20message%3A%5C%22UPDATE%20migrations%20SET%20updated_at%3D%25(updated_at)s%2C%20status%3D%25(status)s%20WHERE%20migrations.id%20%3D%20%25(migrations_id)s%5C%22 | |
| 21:33:55 | efried | looks... new? | |
| 21:34:50 | mriedem | efried: nope | |
| 21:35:05 | mriedem | https://bugs.launchpad.net/nova/+bug/1642537 | |
| 21:35:07 | openstack | Launchpad bug 1642537 in OpenStack Compute (nova) stein "finish_resize fails with DBDeadlock on migrations table" [Medium,In progress] - Assigned to Matt Riedemann (mriedem) | |
| 21:36:25 | dansmith | mriedem: you can't undelete it if you need to restore? | |
| 21:36:31 | efried | mriedem: not in e-r I guess? | |
| 21:37:53 | tonyb | Is there a configurable 'timeout' for server build operations? | |
| 21:39:06 | mriedem | efried: used to be i think | |
| 21:39:14 | mriedem | dansmith: no....but restore how/where? | |
| 21:39:29 | mriedem | like i said, you've have a copy of the instance and related records in the other cell db | |
| 21:39:47 | dansmith | mriedem: I mean undelete if you need to revert | |
| 21:39:58 | dansmith | mriedem: look it up with deleted=yes, set deleted=0, save | |
| 21:40:11 | dansmith | you'll have to fix up things we don't soft-delete, but that seems easier | |
| 21:40:44 | dansmith | I think you probably do want to hard-delete it when you confirm though, so you don't have to worry about the was-it-in-this-cell-before case each time, | |
| 21:40:45 | mriedem | that's not really the issue, unless i'm misunderstanding | |
| 21:40:50 | dansmith | but I dunno, while we're waiting for confirm... | |
| 21:40:52 | dansmith | okay | |
| 21:41:07 | dansmith | oh, I see, | |
| 21:41:21 | mriedem | when you revert, the instance is in both dbs, | |
| 21:41:22 | dansmith | you're talking about the instance in the target cell that got reverted back to source, a week later when you try to migrate again? | |
| 21:41:22 | melwitt | tonyb: I found this https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.instance_build_timeout | |
| 21:41:32 | mriedem | yeah | |
| 21:41:46 | mriedem | we literally can't have the instance.deleted!=0 in the cell db we're moving to | |
| 21:42:39 | tonyb | (undercloud) [root@director config-data]# grep -Erin instance_build_timeout nova | |
| 21:42:39 | tonyb | melwitt: Thanks | |
| 21:42:43 | mriedem | heh i didn't know instance_build_timeout existed | |
| 21:42:43 | tonyb | nova/etc/nova/nova.conf:983:#instance_build_timeout=0 | |
| 21:42:48 | tonyb | so I guess that isn't it | |
| 21:42:49 | mriedem | it's a periodic | |
| 21:43:18 | mriedem | looks like a hack for just setting instance status to ERROR for things that were hung or we didn't properly set to ERROR state on failure | |
| 21:43:31 | dansmith | mriedem: okay, I guess that seems obvious to me so I'm not sure why you're bringing it up as if it's a problem or something | |
| 21:43:49 | mriedem | dansmith: we just haven't hard hard destroy on stuff in the cell db records, except tags i guess | |
| 21:43:54 | mriedem | so it seems new and scary | |
| 21:43:58 | melwitt | tonyb: oh yeah, hm | |
| 21:44:01 | dansmith | mmkay | |
| 21:44:34 | mriedem | luckily theodoros already has a patch to add that support for his rebuild from cell0 series https://review.openstack.org/#/c/570202/ | |
| 21:44:42 | mriedem | which it turned out he later didn't need, but i will | |
| 21:47:14 | mriedem | anyway, i'll update the re-proposed spec to note it | |
| 22:06:41 | openstackgerrit | Matt Riedemann proposed openstack/nova master: api-ref: document ordering for instance actions and events https://review.openstack.org/649748 | |
| 22:09:18 | openstackgerrit | Matt Riedemann proposed openstack/nova master: api-ref: fix description of os-server-external-events 'events' param https://review.openstack.org/649750 | |
| 22:32:42 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove MIN_COMPUTE_MULTIATTACH conditions in API https://review.openstack.org/649757 | |
| 22:51:47 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add nova-status upgrade check for minimum required cinder API version https://review.openstack.org/649759 | |
| 23:01:14 | efried | mriedem: where does grenade source live? | |
| 23:03:32 | mriedem | http://git.openstack.org/cgit/openstack-dev/grenade/ | |
| #openstack-nova - 2019-04-04 | |||
| 00:32:54 | openstackgerrit | Merged openstack/nova master: Don't warn on network-vif-unplugged event during live migration https://review.openstack.org/642877 | |
| 00:37:24 | mriedem | this uh, seems not so necessary https://github.com/openstack/nova/commit/70c7ba7324cf47179d978b4073545e3837d4fd24#diff-12951ec32bc2b8cae6ea1933599a3dbcR334 | |
| 00:41:32 | mriedem | and potentially a security issue | |
| 00:43:22 | mriedem | efried: melwitt: ^ is new in stein so if it's something we shouldn't be exposing in the notifications we should probably think about that for rc2 | |
| 00:47:12 | sean-k-mooney | wait we send the db connection field in the cellmapping notifcation? | |
| 00:47:29 | mriedem | apparently | |
| 00:47:31 | mriedem | and transport_url | |
| 00:47:32 | sean-k-mooney | is that now the templated url? | |
| 00:47:33 | openstackgerrit | Merged openstack/nova master: Drop migrate_keypairs_to_api_db data migration https://review.openstack.org/649648 | |
| 00:47:40 | openstackgerrit | Merged openstack/nova master: Add functional test for the JsonFilter https://review.openstack.org/647811 | |
| 00:47:48 | openstackgerrit | Merged openstack/nova master: api-ref: document ordering for instance actions and events https://review.openstack.org/649748 | |
| 00:47:57 | openstackgerrit | Merged openstack/nova master: api-ref: fix description of os-server-external-events 'events' param https://review.openstack.org/649750 | |
| 00:48:08 | openstackgerrit | Merged openstack/nova master: Remove mox in unit/network/test_neutronv2.py (6) https://review.openstack.org/574113 | |
| 00:48:40 | sean-k-mooney | hehe a lot of nova patches were in the last job run | |
| 00:49:16 | sean-k-mooney | anyway if its the tempalted url its may not be an issue but ya if its the actully connetion sting that is bad | |
| 00:50:38 | mriedem | it does not need to be templated | |
| 00:50:58 | mriedem | https://docs.openstack.org/nova/latest/user/cells.html#template-urls-in-cell-mappings | |
| 00:51:52 | mriedem | even if it was templated, it would be the full url when pulled out of the CellMapping object attribute | |
| 00:52:06 | sean-k-mooney | :( | |
| 00:52:30 | sean-k-mooney | the patch mentioned it was transfroming the notificaiton to the versioned form | |
| 00:52:46 | sean-k-mooney | was this previouly sent via the unversioned endpoint | |
| 00:53:05 | mriedem | i doubt it | |
| 00:53:35 | sean-k-mooney | ok so it is a regression in stien and i agree it a security concern too. | |
| 00:53:59 | sean-k-mooney | how did you come across it | |
| 00:54:00 | mriedem | the unversioned notification sent RequestSpec.to_legacy_request_spec_dict() | |
| 00:54:18 | mriedem | tssurya -1ed a change of mine saying i should add more stuff to that notification since i added a new field to the Destination object | |
| 00:55:06 | mriedem | https://review.openstack.org/#/c/614035/ | |
| 00:55:08 | sean-k-mooney | mriedem: ah ok i similarly modifed a field in one of my patches and it ended up changin the image metadata notificaion | |
| 00:55:23 | sean-k-mooney | which i undersand in a way but i also was confused by | |
| 00:55:43 | sean-k-mooney | it seams that we not have to version some object when field in an objec support new values | |
| 00:56:00 | sean-k-mooney | previousl we did not version bump object for compostion | |
| 00:56:10 | sean-k-mooney | only when fields were added or removed | |
| 00:56:38 | mriedem | gibi_off: stephenfin: https://review.openstack.org/#/c/508506/28/nova/notifications/objects/request_spec.py@334 - if we don't need those i don't tihnk we should be sending them out of nova on the notification bus | |
| 01:02:16 | sean-k-mooney | mriedem: do we want to even include the cell here https://review.openstack.org/#/c/508506/28/nova/notifications/objects/request_spec.py@284 | |
| 01:02:55 | mriedem | idk, the legacy notification for select_destinations didn't send all of this stuff | |
| 01:02:58 | mriedem | so i don't know why it was all added | |
| 01:03:13 | mriedem | the legacy notification sent a legacy more minimal view of the request spec | |
| 01:03:23 | mriedem | this version is everything | |
| 01:03:57 | mriedem | sending the name/uuid of the cell is meh to me | |
| 01:04:18 | mriedem | sending the URLs with credentials to connect to the cell...that's a bad idea | |
| 01:04:20 | sean-k-mooney | ya i was just looking at what the to_legacy_request_spec_dict function did | |
| 01:04:59 | sean-k-mooney | i would proably change 'cell': fields.ObjectField('CellMappingPayload', nullable=True), | |
| 01:05:01 | mriedem | well i threw this on the RC2 etherpad so i guess can wait for others to look and provide an opinion | |
| 01:05:14 | sean-k-mooney | to 'cell': fields.UUIDFiled(nullable=True), | |
| 01:05:45 | sean-k-mooney | and then drop the CellMappingPayload object entirely | |
| 01:06:32 | sean-k-mooney | i think we require the host to be unique across all cells anyway though so the cell is uudi is not really needed | |
| 01:09:24 | mriedem | i'm dropping off | |