Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-05-04
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?
21:00:27 mriedem doesn't doing a rootwrap filter in cinder and nova kind of defeating the purpose?
21:02:07 mriedem heh it's not even used in the volume driver code in nova
21:02:12 melwitt mriedem: I found there is a bug in oslo.log too regarding the unicode thing https://review.openstack.org/#/c/462735
21:03:12 mriedem melwitt: nice catch
21:04:04 melwitt :)
21:04:56 johnsom Hi nova folks. Is there a known issue where cirros instances come up ACTIVE in nova show, but there is no console.log content?
21:05:25 mtreinish johnsom: when I hit that locally on my cloud it was because the glance image was empty...
21:06:33 johnsom Yeah, two of us just restacked today and cirros and our image both won't come up
21:07:09 mtreinish johnsom: I would check the image size in glance. If it's zero bytes that's probably it
21:07:10 johnsom Well, glance is showing content for the cirros image... hmmm
21:07:24 mtreinish hmm, ok maybe it's something else :)
21:07:31 johnsom The console logs aren't in this new fanlged journalctl logs are they?
21:07:57 edleafe crap, looks like cdent has left
21:07:59 mtreinish johnsom: that's independent. THat's just service logs, because the services are running under systemd
21:08:12 mtreinish console logs are grabbed from the virt driver
21:08:18 edleafe I don't see where he is referring to in his comment. dansmith?
21:08:30 johnsom Yeah, I wouldn't have expected it. Especially because there is a zero byte console.log file sitting there
21:08:45 mtreinish johnsom: I would check the n-cpu log
21:09:00 edleafe oh, nvm
21:09:17 edleafe I had the unit test test_resource_tracker.py open, not the functional
21:09:22 openstackgerrit Alex Szarka proposed openstack/nova master: Refactor instance-exists comment and test https://review.openstack.org/462738
21:10:51 dansmith edleafe: I didn't look close, I just read what he said and thought "mocks, okay, complicated"
21:11:03 edleafe heh
21:11:18 openstackgerrit Alex Szarka proposed openstack/nova master: Transform instance.exists notification https://review.openstack.org/403660
21:11:23 openstackgerrit Alex Szarka proposed openstack/nova master: Refactor instance-exists comment and test https://review.openstack.org/462738
21:11:31 johnsom Yeah, I started there. No ERROR records and it shows 2 vcpus allocated. It looks normal to me. Ok, I will dig in. I just didn't want to burn a bunch of time if this was a known issue.
21:12:13 dansmith edleafe: assume he means this: https://pastebin.com/x6MJut7P

Earlier   Later