| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-20 | |||
| 19:37:24 | mriedem | if [glance]/api_servers is set, we just build a static string based on that | |
| 19:37:29 | sean-k-mooney[m] | mriedem: so the api call to glance when sending the notificaiton is from creating the glance client object? | |
| 19:37:33 | mriedem | but if it's not set, we go through the ksa magik | |
| 19:37:45 | efried | Which *should* be using cached values, I thought. | |
| 19:37:55 | mriedem | maybe it is | |
| 19:37:55 | efried | i.e. we're not actually hitting the API 4000 times. | |
| 19:38:02 | mriedem | that would be nice to know | |
| 19:38:46 | efried | mriedem: Yeah, we build the adapter every time, but the session & auth are cached. | |
| 19:39:22 | sean-k-mooney[m] | efried: well if the call to the api is coming from createing the client object perhaps we should be storing that between requests so that we dont have to keep creating it | |
| 19:39:48 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_certs object https://review.openstack.org/489408 | |
| 19:39:48 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_certs to instance_extra https://review.openstack.org/537897 | |
| 19:39:50 | openstackgerrit | Jackie Truong proposed openstack/nova master: Implement certificate_utils https://review.openstack.org/479949 | |
| 19:39:52 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_image_certificates to REST API https://review.openstack.org/486204 | |
| 19:40:12 | mriedem | sean-k-mooney[m]: efried is saying we're not hitting the api every time | |
| 19:40:19 | mriedem | just the first time to get the service catalog entry | |
| 19:40:29 | mriedem | and it's the identity api in this case | |
| 19:40:41 | sean-k-mooney | mriedem: yes but we are still creating the client 4000 and hitting the cache 3999 times right? | |
| 19:40:47 | mriedem | alternatively, don't put an image ref url in the notification payload, just the image uuid | |
| 19:40:52 | mriedem | sean-k-mooney[m]: yeah | |
| 19:41:28 | mriedem | the notification payload attempts to mimic the GET /servers/detail API which returns the image id and bookmark link to the image | |
| 19:41:35 | mriedem | using the same thing we're hitting here | |
| 19:41:35 | sean-k-mooney | so if we create teh client once and resuit we definetlly dont hit the api and we get rid of 3999 calls to the client constructor? | |
| 19:42:29 | mriedem | sure, but that's lower priority atm | |
| 19:42:37 | mriedem | the thing now is how to fix this periodic | |
| 19:42:48 | efried | It'd be interesting to see if I can cache the adapter and stuff will still work. | |
| 19:43:06 | sean-k-mooney | mriedem: image uuid might be better in general then the url | |
| 19:43:21 | mriedem | sean-k-mooney: at least for the notification yeah i agree | |
| 19:43:47 | mriedem | we can change that with versioned notifications, but this also gets shoved into the legacy notifications and changing those is like breaking an api | |
| 19:43:49 | sean-k-mooney | glance can have multiple image urls correct? is there any guarntee the consumer of the url can reach that backend | |
| 19:44:38 | mriedem | well, it's over rpc so if you've configured nova to hit internal glance api endpoints, your notification consumer probably can too | |
| 19:44:57 | mriedem | if that consumer actually needs to get the image details, idk | |
| 19:45:11 | mriedem | or why the consumer can't just take the image id and form it's own image api request, ... | |
| 19:45:29 | sean-k-mooney | mriedem: sure but with the uuid they can query for which ever one they actully need so that is more generally useful i think | |
| 19:45:30 | mriedem | it's not like we return links to volumes and ports in the notification either | |
| 19:45:38 | mriedem | sean-k-mooney: i agree | |
| 19:45:42 | melwitt | yeah, the only other thing I can think of for fixing the periodic task is if we could somehow seed periodic tasks with the service catalog instead of the empty get_admin_context() one, but I don't know what would be involved there | |
| 19:46:10 | melwitt | I guess it anyway won't be able to get any info from glance even if it had the catalog, it would just fail later, right? | |
| 19:46:34 | mriedem | the notification path here isn't actually doing a GET to glance | |
| 19:46:47 | efried | Yeah, I'm not worried about that bit at all. | |
| 19:46:49 | mriedem | it's getting the service catalog from keystone via ksa | |
| 19:46:53 | melwitt | meaning, it won't do that even if it has the catalog? | |
| 19:46:54 | mriedem | but w/o a token | |
| 19:47:05 | melwitt | okay | |
| 19:47:27 | mriedem | so for now, the easiest thing to do probably do is in this notification code, handle EndpointNotFound and just set the image_ref_url to the image id | |
| 19:47:36 | efried | are context hashable? | |
| 19:47:45 | mriedem | they are serializable | |
| 19:47:46 | dansmith | edleafe: hmm, so member_of with multiple aggregates is doing an "or" of all the ones you pass, right? is that what we really want? | |
| 19:47:51 | openstackgerrit | Merged openstack/nova master: [libvirt] Add _get_XXXpin_cpuset() https://review.openstack.org/527631 | |
| 19:47:54 | efried | so probably | |
| 19:48:00 | mriedem | efried: i'd rather not do crazy cache shit with contexts | |
| 19:48:02 | openstackgerrit | Merged openstack/nova master: api-ref: add a note in DELETE /os-services about deleting computes https://review.openstack.org/553598 | |
| 19:48:14 | mriedem | we already have some craziness with contexts and periodics today pulling off the local thread storage | |
| 19:48:22 | mriedem | which totally effs with request id log tracing | |
| 19:48:23 | efried | mriedem: I was just going to experiment, cool yer jets. | |
| 19:48:33 | mriedem | i will not cool my jets or hold my horses | |
| 19:48:51 | melwitt | slow your roll | |
| 19:48:57 | mriedem | this is the part that blows up https://github.com/openstack/nova/blob/master/nova/notifications/base.py#L397 | |
| 19:48:58 | openstackgerrit | Chris Dent proposed openstack/nova master: Use nova.db.api directly https://review.openstack.org/543262 | |
| 19:49:54 | sean-k-mooney | mriedem: is instance.image_ref the image id? | |
| 19:51:18 | melwitt | gdi I tried to set the importance at the same time as mriedem and set it differently. will set it back | |
| 19:53:34 | openstackgerrit | Eric Young proposed openstack/nova master: Support extending attached ScaleIO volumes https://review.openstack.org/554679 | |
| 19:54:17 | mriedem | sean-k-mooney: yes | |
| 19:54:28 | melwitt | mriedem, efried: are one of you going to propose the patch or shall I? | |
| 19:54:38 | mriedem | i can push a patch | |
| 19:54:43 | melwitt | k | |
| 19:54:44 | mriedem | if LP would not timeout on me | |
| 19:55:15 | openstackgerrit | Michael Still proposed openstack/nova master: Move xenapi xenstore_read's to privsep. https://review.openstack.org/552241 | |
| 19:55:15 | openstackgerrit | Michael Still proposed openstack/nova master: Move configurable mkfs to privsep. https://review.openstack.org/551921 | |
| 19:55:16 | openstackgerrit | Michael Still proposed openstack/nova master: Sync xenapi and libvirt on what flags to pass e2fsck. https://review.openstack.org/554078 | |
| 19:55:16 | openstackgerrit | Michael Still proposed openstack/nova master: Move xenapi disk resizing to privsep. https://review.openstack.org/552242 | |
| 19:55:17 | openstackgerrit | Michael Still proposed openstack/nova master: Move image conversion to privsep. https://review.openstack.org/554437 | |
| 19:55:17 | openstackgerrit | Michael Still proposed openstack/nova master: Move xenapi partition copies to privsep. https://review.openstack.org/553605 | |
| 19:55:18 | openstackgerrit | Michael Still proposed openstack/nova master: We no longer need rootwrap. https://review.openstack.org/554438 | |
| 19:55:18 | openstackgerrit | Michael Still proposed openstack/nova master: We don't need utils.trycmd any more. https://review.openstack.org/554439 | |
| 19:56:35 | sean-k-mooney | mriedem: one other tought. you said if you get an endpoint not found you would set image_ref_url to the image id. any reason to not always use the image id? | |
| 19:57:01 | mriedem | sean-k-mooney: i said above, it changes the api | |
| 19:57:10 | mriedem | in this case, meh | |
| 19:57:18 | mriedem | we can change the payload for the versioned notifications later, | |
| 19:57:22 | mriedem | but this also goes in the legacy notifications | |
| 19:57:24 | jaypipes | guh, this day turned into a giant disaster. | |
| 19:57:38 | sean-k-mooney | mriedem: yes but we could have a microverion for that no? oh this is an unversioned notification | |
| 19:57:39 | mriedem | jaypipes: i have a photo that might make your day better | |
| 19:57:43 | efried | jaypipes: Sokay, I still haven't gotten through those two patches yet. | |
| 19:57:56 | jaypipes | efried: no worries, duder. today's pretty much a goner for me. | |
| 19:58:17 | sean-k-mooney | jaypipes: how did the dentist go. | |
| 19:58:20 | jaypipes | efried: suffice to say it involves me cleaning up a giant pile of dog shit in the back of my car in the rain. | |
| 19:58:29 | jaypipes | efried: ^ not related to the dentist | |
| 19:58:36 | mriedem | but at least your teeth are clean | |
| 19:58:37 | efried | jaypipes: Geez, you try to do something nice for the dog... | |
| 19:58:51 | jaypipes | indeed. | |
| 19:58:53 | mriedem | unless there was...splatter | |
| 19:59:21 | sean-k-mooney | mriedem: ew, let hop not for jaypipes sake | |
| 19:59:24 | efried | jaypipes: You want a quick feeling of satisfaction, https://review.openstack.org/#/c/545111/ ought to be eligible for quick-approve :) | |
| 19:59:29 | sean-k-mooney | *hope | |
| 19:59:55 | mriedem | efried: i'll take a look again at that after this | |
| 19:59:57 | jaypipes | my dogs have become a nested provider tree with inventory of DOG_SHIT_KG | |
| 20:00:07 | efried | rofl | |
| 20:00:08 | mriedem | CUSTOM_DOG_SHIT_KG? | |
| 20:00:11 | tssurya | lol | |
| 20:00:14 | jaypipes | yes, sorry. | |