| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-04 | |||
| 16:26:52 | jgwentworth | I would think so, rbd == ceph | |
| 16:26:55 | superdan | mriedem: do we not store network_request(s) today? | |
| 16:27:02 | mriedem | https://docs.openstack.org/nova/latest/configuration/config.html#libvirt.images_type | |
| 16:27:08 | mriedem | superdan: not in the request spec no | |
| 16:27:09 | mriedem | nor bdms | |
| 16:27:14 | superdan | mriedem: I mean at all | |
| 16:27:18 | mriedem | correct | |
| 16:27:36 | superdan | mriedem: don't we need that on rebuild/evac? or do we get it from neutron or something? | |
| 16:27:51 | mriedem | we have the network info cache / neutron | |
| 16:27:54 | superdan | surely not just the info_cache at that point...? | |
| 16:27:57 | mriedem | you can't request new ports on rebuild/evac | |
| 16:27:59 | superdan | hrm | |
| 16:28:20 | superdan | mriedem: so, storing it in the reqspec.. that may potentially include port uuids that you booted with right? | |
| 16:28:30 | mriedem | the original network request list woudn't have things attached to the instance after it was created either | |
| 16:28:39 | mriedem | superdan: port id or network id yeah | |
| 16:28:39 | superdan | mriedem: what happens if/when I attach a new port and detach the old one.. now the reqspec has a stale from-two-years-ago port uuid in it right? | |
| 16:29:07 | mriedem | superdan: yeah, same could be said about the security groups right? | |
| 16:29:20 | superdan | yes, although that seems less problematic to me, | |
| 16:29:33 | mriedem | the spec says they aren't going to persist this anyway, just include it in the request spec is a middleman to get from the api to the scheduler | |
| 16:29:34 | superdan | probably because port_uuids are somewhat ephemeral | |
| 16:29:48 | mriedem | i'm fine with not persisting it | |
| 16:30:04 | superdan | mriedem: okay in general I think it's confusing to put things in DB-persisted objects that we don't store in the DB | |
| 16:30:18 | mriedem | but i was thinking for modeling, NetworkRequest object gains a RequestGroup field, and then RequestSpec gains a (non-persisted) NetworkRequestList field | |
| 16:30:20 | superdan | someone sets something there and calls save(), no error, so expects it's saved now, but isn't | |
| 16:30:45 | superdan | what is requestgroup? | |
| 16:30:49 | mriedem | ok, gibi has an alternative in the spec that it doesn't go into the request spec at all, and it's passed as a separate param to select_destinations() | |
| 16:31:02 | mriedem | it's in the placement lib - contains things about a granular request group | |
| 16:31:03 | jmccarthy | Ah sorry ! I was looking at a template, not the final rendered file | |
| 16:31:11 | superdan | oh, that right | |
| 16:31:19 | mriedem | so they'd have one of those per port | |
| 16:31:32 | jmccarthy | For my deployed nova.conf there is no entry there for images_type at all (so I guess default?) | |
| 16:31:35 | superdan | mriedem: well, a new rpc argument to the scheduler just for network stuff is also kinda odd, which I guess was your point | |
| 16:31:36 | mriedem | if it doesn't go in the request spec because we don't persist it, then it's a new param to select_destinations() | |
| 16:32:16 | mriedem | jgwentworth: so i don't think my idea will work..drats | |
| 16:32:56 | mriedem | jgwentworth: my idea was just to check https://github.com/openstack/nova/blob/4b0d0ea9f18139d58103a520a6a4e9119e19a4de/nova/virt/libvirt/imagebackend.py#L57 in driver._cleanup_resize and if True, do the snapshot removal stuff that is rbd-specific | |
| 16:33:01 | jgwentworth | *sad trombone* | |
| 16:33:14 | mriedem | but to get that value, we have to init the imagebackend object which is the thing that recreates the gd disk.info file | |
| 16:33:41 | mriedem | so, we could make a module-level dict of image types that support clone and use that, | |
| 16:33:57 | mriedem | or pass a flag to imagebackend init to tell it to not touch the filesystem | |
| 16:34:04 | mriedem | former seems easier | |
| 16:34:19 | mriedem | or hell, just: if CONF.libvirt.images_type == 'rbd': in _cleanup_resize | |
| 16:34:30 | mriedem | that's essentially what we'd be doing if we check SUPPORTS_CLONE | |
| 16:37:04 | jgwentworth | do you mean change the check for is_volume_backed to essentially "is_shared_storage"? | |
| 16:38:58 | mriedem | that entire block goes away | |
| 16:39:07 | jmccarthy | <back asap - afk> | |
| 16:39:13 | mriedem | i.e. basically undo this https://review.openstack.org/#/c/437356/3/nova/virt/libvirt/driver.py | |
| 16:39:17 | mriedem | and fix the logic | |
| 16:39:24 | jgwentworth | oh, I see, yeah | |
| 16:39:34 | jgwentworth | instead of using root_disk.exists() | |
| 16:39:50 | mriedem | "if os.path.exists(inst_base) and not root_disk.exists():" was added for qcow2/flat/ploop because those will recreate the instance dir and disk.info on init, | |
| 16:40:01 | mriedem | when all we needed the imagebackend for was the remove_snap call, | |
| 16:40:04 | mriedem | which is rbd-specific | |
| 16:40:07 | jgwentworth | right | |
| 16:40:12 | jgwentworth | gotcha | |
| 16:40:19 | mriedem | so if we just don't create the imagebackend object in the first place, we avoid the init et al | |
| 16:40:22 | mriedem | \o/ | |
| 16:40:30 | jgwentworth | yeah, seriously | |
| 16:40:44 | mriedem | i of course will require jmccarthy to test the patch in his setup | |
| 16:41:00 | jgwentworth | oh, because you have to use it to do the remove_snap | |
| 16:41:27 | openstack | Launchpad bug 1666831 in OpenStack Compute (nova) ocata "Nova recreates instance directory after migration/resize" [Low,Fix committed] - Assigned to Lee Yarwood (lyarwood) | |
| 16:41:27 | mriedem | yup and that's pointed out in https://bugs.launchpad.net/nova/+bug/1666831 | |
| 16:41:34 | mriedem | "root_disk is used to remove rdb snapshots, but during execution of self.image_backend.by_name() nova recreates instance directory." | |
| 16:41:50 | jgwentworth | so how can we remove_snap without the imagebackend object? | |
| 16:41:53 | mriedem | so in his case, he wasn't even using rbd, he was using qcow2 | |
| 16:42:01 | mriedem | we can, we'll get it if the backend supports clone | |
| 16:42:11 | mriedem | remove_snap is only implemented for image backends that support clone | |
| 16:42:15 | mriedem | and that's only rb | |
| 16:42:15 | jgwentworth | ohhhh k | |
| 16:42:17 | mriedem | *rbd | |
| 16:43:29 | jgwentworth | yeah, makes sense | |
| 16:44:45 | openstackgerrit | Merged openstack/nova master: Convert websocketproxy to use db for token validation https://review.openstack.org/333990 | |
| 16:45:53 | mriedem | and because i added that tempest test, we should be testing a resize of a volume-backed instance on shared local storage since we have the NFS job in nova's experimental queue | |
| 16:45:55 | mriedem | plus CEPH | |
| 16:48:05 | jgwentworth | *mind blown* | |
| 16:54:19 | jmccarthy | back | |
| 17:07:58 | jmccarthy | mriedem: I can test any stuff no probs ! It might take a little while, but hopefully not too long | |
| 17:12:09 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: check image type before removing snapshots in _cleanup_resize https://review.openstack.org/566367 | |
| 17:12:10 | mriedem | jmccarthy: jgwentworth: ^ | |
| 17:12:21 | jmccarthy | mriedman: Nice ! | |
| 17:12:37 | jmccarthy | mriedman: Let me see about working that in | |
| 17:21:40 | openstackgerrit | Merged openstack/nova master: Add multi-cell negative test for cold migration with target host https://review.openstack.org/524027 | |
| 17:21:49 | openstackgerrit | Merged openstack/nova master: Update layout docs for running console proxies https://review.openstack.org/557489 | |
| 17:37:54 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: remove old rbd snapshot removal error handling https://review.openstack.org/566369 | |
| 17:38:44 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: remove old rbd snapshot removal error handling https://review.openstack.org/566369 | |
| 17:39:00 | openstackgerrit | Jay Pipes proposed openstack/nova master: rework how we pass candidate request information https://review.openstack.org/566166 | |
| 17:39:17 | idlemind | jgwentworth thx i'm using lvm via iscsi (simple stuff for now) | |
| 17:49:17 | jmccarthy | mriedman: Ok I thought this might happen - I'm going to need a while longer, unfortunately it's a long weekend here - I'll update the bug asap but it may not be until Tuesday | |
| 17:51:23 | jmccarthy | mriedman: I appreciate your quick efforts ! I'm going to keep at it another while | |
| 18:15:23 | mriedem | jmccarthy: sure np | |
| 18:15:35 | pipesinpain | mriedem, jgwentworth, johnthetubaguy, alex_xu: your eyeballs on https://review.openstack.org/#/c/565565/ would be appreciated. | |
| 18:21:06 | arvindn05 | mriedem: can you look over the spec amendment? https://review.openstack.org/#/c/560718/ | |
| 18:21:36 | arvindn05 | once its approved, hoping the scheduler patch can be upstreamed | |
| 18:53:05 | jgwentworth | pipesinpain: ack, it's on my list | |
| 18:59:15 | openstackgerrit | Chris Dent proposed openstack/nova master: Optional separate database for placement API https://review.openstack.org/362766 | |
| 18:59:16 | openstackgerrit | Chris Dent proposed openstack/nova master: Isolate placement database config https://review.openstack.org/541435 | |
| 18:59:17 | openstackgerrit | Chris Dent proposed openstack/nova master: WIP: Ensure that os-traits sync is attempted only at start of process https://review.openstack.org/553857 | |
| 19:00:17 | openstackgerrit | Chris Dent proposed openstack/nova master: WIP: Add PLACEMENT_DB_ENABLED=True to the nova-next job https://review.openstack.org/564067 | |
| 19:06:45 | pipesinpain | jgwentworth: thx Melanie. | |
| 19:07:27 | openstackgerrit | Merged openstack/nova master: Base test module/class for functional placement db https://review.openstack.org/564590 | |
| 19:19:51 | eandersson | mriedem, I was gonna do a quick pull request to change the log "Successfully synced instances from host '%s'." to DEBUG, but noticed that it's still doing log translation | |