Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-05-04
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 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:28:39 mriedem superdan: port id or network id yeah
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 mriedem yup and that's pointed out in https://bugs.launchpad.net/nova/+bug/1666831
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: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 jgwentworth ohhhh k
16:42:15 mriedem and that's only rb
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
19:20:05 eandersson Can I remove the log translation in the same commit?
19:24:12 mriedem eandersson: yes
19:24:18 mriedem we don't translate logs anymore
19:24:38 eandersson Can I remove the log translation for all the entries in that file? :D or will that make it too difficult to see what changed
19:24:51 eandersson I can also follow up with a new pull request for that :D
19:25:39 mriedem i'd keep those separate
19:27:48 openstackgerrit Erik Olof Gunnar Andersson proposed openstack/nova master: Changing scheduler sync event from INFO to DEBUG https://review.openstack.org/566392

Earlier   Later