Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-05-04
19:59:03 mriedem gibi: i rebased and commented in https://review.openstack.org/#/c/419730/ for adding keypairs to InstancePayload - summary is i don't think we should lazy load instance.keypairs for every instance action, for the same reason I didn't want to do that for instance.tags
20:00:26 sdague dansmith: yep agreed, I want to make sure it looks sane
20:00:37 sdague hey, it's all about learning new things :)
20:02:13 dansmith sdague: I'm not complaining :)
20:03:11 sdague also, what's up with the linuxbridge job?
20:03:17 sdague that seem things to be hating me today
20:04:24 dansmith sdague: that was an impressively incomprehensible exclamation
20:04:45 openstackgerrit Ed Leafe proposed openstack/nova master: Don't delete allocation if instance being scheduled https://review.openstack.org/460177
20:04:55 edleafe bauzas: Here ya go ^^
20:05:33 dansmith what is this nonsense? http://logs.openstack.org/79/462679/1/check/gate-nova-tox-functional-ubuntu-xenial/c760042/testr_results.html.gz
20:05:37 bauzas edleafe: okay, I'll update the next top changes
20:07:06 melwitt dansmith: socket_options, duh
20:07:26 melwitt :D
20:10:49 cdent dansmith: might be mismatched versions of urllib3 and wsgi-intercept
20:11:03 dansmith cdent: it's broken is what it is
20:11:03 cdent I saw that a while back but thought it was better.
20:11:20 dansmith looks rather fundamental, so wondering if it's worth a recheck
20:11:25 cdent yessir, but I think it got better. are your deps maybe stale?
20:11:35 dansmith that patch is new from today
20:11:39 cdent hmmm
20:12:00 dansmith I guess edleafe already rechecked it
20:12:15 cdent i'll mess with it a bit
20:12:19 cdent (locally)
20:12:24 mriedem can't provide new key_name on rebuild?
20:17:28 melwitt mriedem: I don't think you can specify anything other than the image. there have been requests to allow new userdata and such, but the answer has been "just boot a new instance" if you're looking at re-imaging it anyway
20:18:04 mriedem do we rebuild the config drive during rebulid?
20:18:32 melwitt I don't think so
20:18:50 melwitt but not sure, not a config drive expert
20:19:24 mriedem i'm looking, just this keypair notification patch has me thinking about weird stuf
20:19:29 mriedem https://review.openstack.org/#/c/419730/
20:19:40 melwitt we definitely don't change anything on it. I don't know if it is "recreated" at all though
20:19:40 mriedem b/c i don't want to lazy-load instance.keypairs for every gd instance action notification
20:20:00 mriedem jaypipes: dansmith: ^ same predicament as with the instance.tags patch
20:20:08 mriedem for notifications that is
20:21:28 mriedem which would include every vm/task state change if you enabled that option...
20:21:50 mriedem CONF.notifications.notify_on_state_change i mean
20:22:23 melwitt preload all the things
20:22:29 melwitt jk
20:22:42 mriedem well someone proposed that for instance.tags,
20:22:44 mriedem preload by default
20:22:47 mriedem we nacked that
20:22:47 jroll just keep the whole db in RAM, wcpgw
20:23:10 melwitt see, jroll knows what I'm talkin about
20:23:13 openstackgerrit Alex Szarka proposed openstack/nova master: Transform instance.exists notification https://review.openstack.org/403660
20:23:14 mriedem i worry that some of the perf issues people are reporting in newton/ocata are due to all of the versioned notifications doing lazy-loads like crazy
20:24:02 jroll melwitt: :D
20:24:07 jroll feels like state change notifications shouldn't need tags, keypairs, etc
20:25:15 mriedem that's what i've said
20:25:19 mriedem and pushed back on
20:25:22 mriedem nor bdms...
20:25:35 mriedem we're basically throwing the kitchen sink at the notification consumer
20:26:00 mriedem because (1) we can and (2) we don't know what notification consumers even want/need
20:26:06 jaypipes mriedem: currently reviewing dansmith's target_cell patches. will get to those a little later (all the notification ones)
20:26:22 mriedem jaypipes: you don't really need to probably, just thinking out loud
20:26:25 mriedem i've already -1ed
20:26:39 jroll mriedem: and I just realized (3) notifications are considered lossy, so we can't assume consumers have state n-1 to diff properly
20:26:49 melwitt :\ a lot of people use notifications, I thought. so one would think we would have some idea what notification consumers want
20:26:55 jroll is the most sensible reason to me
20:27:20 mriedem melwitt: rebuild calls virt driver destroy and then spawn, so i'd think we're rebuilding the config drive...
20:27:33 mriedem but i guess you just get the same key as before
20:28:17 melwitt mriedem: yeah, I don't know what the term "rebuild" means in the context of config drive. I know we don't change it
20:28:25 mriedem melwitt: the only consumers i know of really are ceilometer, mistral and searchlight
20:28:47 mriedem not sure how much ceilometer is used, and since they didn't get the stuff they needed in the notifications years ago, they just hit the rest api
20:28:56 mriedem not sure about mistral or searchlight
20:29:44 melwitt notifications were supposed to be the answer to instance actions records. I have used those before in a previous life. and notifications, but just to know the history of what happened to an instance when the customer comes calling
20:33:43 mriedem melwitt: what i meant by rebuild was really recreate, since destroy_disks=True when driver.destroy is called during rebuild
20:33:52 mriedem so i think that will delete the config drive for the instance from the host,
20:33:56 mriedem and then spawn would recreate it
20:34:29 cdent edleafe, dansmith: I narrowed down the sitch on https://review.openstack.org/#/c/462679/ and left a comment there
20:34:46 cdent I can push the fix if desired
20:35:14 melwitt mriedem: I see what you mean. that makes sense with the requests too since it's being recreated anyhow and people think, why not allow metadata changes
20:36:23 mriedem right so i for whatever reason thought you could provide a new key during a rebuild,
20:36:42 mriedem and since i was -1ing that review saying scope the full keypair object to just a couple of notifications, like instance.create,
20:36:57 mriedem i was thinking instance.update too, but the key doesn't actually change, so we shouldn't need the full keypair in there
20:37:12 mriedem so i'm thinking we just put the full keypair in the instance.create notification and call it good
20:37:14 melwitt I see
20:37:17 mriedem and put instance.key_name in everything else
20:37:27 mriedem since that's just stored on the instance record and not lazy-loaded
20:37:56 melwitt IMHO it seems like notifications are a lot more fine-grained than I would have expected or thought needed
20:38:18 mriedem fine grained as in providing more detail than you'd expect?
20:38:27 mriedem or fine-grained as in different payloads for different notifications?
20:38:36 melwitt yeah, I guess on create you already have the key_name from the request, what else do you need
20:38:52 mriedem we also set instance.keypairs in the api during create
20:38:56 mriedem so we don't need to lazy-load it
20:39:14 jaypipes dansmith: if you have time: https://review.openstack.org/#/c/457782/
20:39:58 melwitt mriedem: I meant fine-grained like rich subobjects for everything
20:40:27 melwitt I didn't even know keypair was a field on instance
20:40:48 mriedem yeah that's the other argument is that the instance action payloads already have the full flavor and instance info_cache
20:40:50 melwitt I thought there was only the key name if anything
20:41:14 mriedem we started storing instance.keypairs when we moved keypairs to the instance_extras table
20:41:22 mriedem and by we i mean dan
20:41:29 mriedem well,
20:41:33 mriedem or maybe that was the move to the api db
20:41:33 melwitt ah, right. I forgot about that even though it was mentioned to me recently
20:41:35 mriedem i get confused
20:42:05 melwitt yeah I can't remember why. probably to avoid having to do a lookup in compute or something
20:57:00 edleafe cdent: dansmith: I'll push the fix.
20:57:32 dansmith ah
20:57:36 dansmith that was an odd failure
21:00:16 mriedem hemna: so looking at https://review.openstack.org/#/c/382097/47/etc/cinder/rootwrap.d/volume.filters and https://review.openstack.org/#/c/443951/10/etc/nova/rootwrap.d/compute.filters i thought we were supposed to push that common root helper stuff into os-brick?

Earlier   Later