Earlier  
Posted Nick Remark
#openstack-nova - 2024-07-18
15:05:41 sean-k-mooney nova has 2 existign apis both fo which have test coverage
15:06:12 sean-k-mooney the console url show rpvoded a url that is templated based on `spice.kerbside_base_url`
15:06:25 bauzas_ okay, then I'll accept the spec but asking for a FUP
15:06:48 sean-k-mooney the url provided to the use rwhen they do show has a console token which kebside will use to call /os-console-auth-tokens
15:06:58 sean-k-mooney so im expeicting 2 seperte tests
15:07:16 sean-k-mooney one to cover the new console type for console url show
15:07:35 sean-k-mooney and one to test the addtion of a tls_port for spice-direct in /os-console-auth-tokens
15:53:01 opendevreview Merged openstack/nova-specs master: Re-propose using extend volume completion action for 2024.2 https://review.opendev.org/c/openstack/nova-specs/+/917133
16:05:43 bauzas sean-k-mooney: dansmith: gibi: I know this is late, but could you please look at https://review.opendev.org/c/openstack/nova-specs/+/922201 if you have time today ?
16:05:58 bauzas I can also try to look at it before leaving
16:09:35 dansmith there's a ton of unresolved comments on that?
16:12:25 bauzas_ I think fwiesel replied on sean's comments
16:12:29 bauzas_ but sure, I can look at those
16:13:07 dansmith but there's a bunch that reference lines not even in the file anymore and so they're stacked at the top
16:13:18 bauzas_ due to an older revision
16:13:30 bauzas_ but I can clean them up or stick them
16:13:35 bauzas_ lemme look before you go in
16:16:40 sean-k-mooney if the lazy loadign was optional and off by defualt. i could maybe see triling this but im still not entirly sure this the best approch
16:27:11 bauzas sean-k-mooney: can you then just reply to Fabian's comments ?
16:27:31 bauzas I agree with the fact that I wouldn't rush to merge that spec given your concerns
16:27:47 bauzas I'd rather propose that spec to be discussed at the next PTG
16:27:52 bauzas fwiesel ^
16:31:16 dansmith he does specify it being optional, but not sure about default
16:31:44 dansmith but I agree, I'm really not sure about the claims made in there, as this seems like it would be a gateway to increasing load
16:32:26 sean-k-mooney i woudl actully prefer to reduce the load on neutron by reviving https://review.opendev.org/c/openstack/nova/+/786348
16:32:27 dansmith it may be better in the one case where you want to re-fetch one thing, but it seems like it opens the ability (and perhaps even required behavior) of generating many more small requests at the behest of a client that may want to DoS the metadata server
16:33:05 dansmith I thought the plan was just to cache that stuff in with metadata instead of always fetching it from neutron, tbh
16:33:23 sean-k-mooney well thats what the older patch is doing
16:33:24 dansmith idk that we need to store it in info_cache directly and I can see reasons why that would be bad and confusing
16:33:38 dansmith the older patch is storing it in info_cache, rihgt?
16:33:41 sean-k-mooney its adding the security groups to the network info cache
16:33:53 sean-k-mooney which speed up both the metadta api and server show
16:33:57 dansmith right, but what I'm saying I thought the plans was, is to just cache it in the built metadata
16:34:12 dansmith yeah I know, but if you immediately add a floating and then server show and it's not there, that's going to suck
16:34:18 sean-k-mooney oh am i was not in the ptg session so i dont know
16:34:22 dansmith which I think is why we're not doing that today, IIRC
16:34:46 sean-k-mooney we do already cache all the metadata once built today btu its time based with no way to triger the invalidation
16:34:51 dansmith network-changed doesn't dispatch for adding a floating right?
16:35:20 sean-k-mooney it might but i dont think applciationcation should be relaying on the metadata contnet to be up to date
16:35:47 sean-k-mooney we do get network-chaged for a number of things but i dont knwo of a list that is written down anywhere to check
16:35:52 dansmith I was talking about server show
16:36:06 dansmith I don't think applications can rely on anything other than metadata for that info right?
16:36:25 sean-k-mooney they shoudl really call neutorn if the need the athoritive info
16:36:38 sean-k-mooney but if they dont know they are on openstack then no
16:36:41 sean-k-mooney this is all they have
16:36:46 dansmith applications inside guests can't do that unless they have creds, which we also don't provide them with :)
16:37:13 dansmith like some thing that needs to provision on boot and know it's external IP so it can offer it up to clients needs to ask metadata for that info
16:37:46 sean-k-mooney yep ops have asked for that JWS tokens speficically
16:38:51 sean-k-mooney dansmith: so the boot case shoudl be up to date today beacue its the first request but yes if it change during the boot
16:39:11 sean-k-mooney i.e. a floting ip was added while it was booting it could be racy
16:40:25 sean-k-mooney what would be nice at somepoint is to be able to povide a short lived bootsrap took like kubernbetes does but thats a much bigger feature
16:41:11 sean-k-mooney dansmith: may main concern with the lazy loading spec is it does not explain how its going to do this
16:42:00 sean-k-mooney liekk is the plan ot create a url router within the metadata code to fetch only the conent for that file via decompoing things into lots of funcitons
16:42:51 sean-k-mooney or do some kind of properlty based laze loading on a class
16:44:19 sean-k-mooney there is really just https://review.opendev.org/c/openstack/nova-specs/+/922201/9/specs/2024.2/approved/lazy-metadata-loading.rst#79
16:53:38 dansmith sean-k-mooney: yeah, if there's a race in the boot, add floating, cloud-init sequence it could be wrong
17:08:13 sean-k-mooney so to me the metadata api comes under the banner of "no more proxy apis" in nova
17:08:55 sean-k-mooney i.e. its not a data socue that shoudl be proxiying up to date info form other service its just a cached view of some useful info
17:09:22 sean-k-mooney so the idea that applciations are writen to poll it at a rate
17:09:29 dansmith idk, I don't think that's reasonable if there is literally no other way to get the info and we're holding the guest in our hand
17:09:35 sean-k-mooney that is enough to cause load issues is kind of bizar to me
17:09:43 dansmith like, if we start provisioning them with credentials and a directory, then I'm all for it
17:09:59 dansmith (provisioning with credentials that can be renewed I should say)
17:10:23 sean-k-mooney ya so at some point i think that should be done like how pods in k8s get service credital tokens to be able to talk to k8s
17:10:46 sean-k-mooney but short term if we can optimise it then ok im just concerned by the lack of detial on how
17:10:48 dansmith right
17:10:53 dansmith people have been asking for that for a long time
17:11:12 opendevreview Sylvain Bauza proposed openstack/nova master: cpu: Only check governor type on online cores https://review.opendev.org/c/openstack/nova/+/924427
17:11:17 sean-k-mooney ya i think its a reason able thing to od at least if the user opts into it
17:11:33 sean-k-mooney but its not what the spec is adressing
17:12:10 bauzas sean-k-mooney: I had to drop due to a bugfix, but please just capture your concerns on that lazy-loading metadata spec so you can move on
17:12:28 bauzas and we'll revisit that spec at the PTG
17:16:55 sean-k-mooney bauzas: i rased most of them in the spec already, mainily i dont know how they are planning to implemnte the lazy loading and im concured it will break cloud-init or cirros-init due to the fact they dont retry by default and some recuest take longer then other to formulate
17:17:04 bauzas ++
17:17:27 sean-k-mooney bauzas: bascily cloud-init and cirros-init today only retry the first request and assuem the rest will be fast and work first try by default
17:17:39 sean-k-mooney cloud init cna be configured to retry its just not the default
17:18:13 sean-k-mooney so if this is on by defualt im concerned it woudl regression previous bugs we had where vms would not get an ssh key inejcted ectra
17:20:14 dansmith sean-k-mooney: yeah, that's exactly my concern about making this less obvious by having some requests work and some not because they're all independent on the backend
17:20:22 dansmith I missed your comment about that but +100
17:20:49 dansmith I think if we want to defeat caching in certain circumstances we *should* make cloud-init declare when it wants uncached data or not,
17:20:53 dansmith and otherwise we're just guessing
17:21:06 dansmith but splitting this into N backend requests doesn't seem to improve things at all to me
17:26:02 sean-k-mooney i was wondering if there was soem way to use etag or just support triggerting the cache invalidation form the clinet side i.e. cluod init
17:26:11 sean-k-mooney or curl
17:26:32 sean-k-mooney i dont know what the standard convetion is for that at a http level but im sure there is one
17:30:16 sean-k-mooney sound like there is a semi standardisaed HTTP PURGE method that cloudflare, varnish and squid all support to do this
17:31:31 dansmith cache-control, I mentioned it in my review
17:31:49 sean-k-mooney cache-contol is slightly different
17:32:07 sean-k-mooney that says if caching layers are allowed to cache it or not and for how long
17:32:22 dansmith it's usable on request as well
17:32:25 sean-k-mooney that works with etag to allow the caching server to know if the content changed
17:32:31 sean-k-mooney oh really
17:32:39 dansmith https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control
17:32:46 sean-k-mooney so you can send a cache-contole no-store or similr
17:32:49 dansmith max-age, no-cache, etc would be perfect here I think
17:33:03 sean-k-mooney ya i tought that was responce only
17:33:09 sean-k-mooney but if it can be in the request
17:33:11 sean-k-mooney then yes
17:34:28 sean-k-mooney ya so that seams quite reasonabel to look into
17:56:16 opendevreview Merged openstack/nova-specs master: Propose a new API microversion for a spice-direct console. https://review.opendev.org/c/openstack/nova-specs/+/915190

Earlier   Later