| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-11-08 | |||
| 11:33:07 | bauzas | so, any forgery hiding traits sounds preferable | |
| 11:33:22 | sean-k-mooney | i think its fair to say that if you have very complex resouce and traits request in flavor that opens you up to some upgrade impacts that you woudl otehrwise not be exposed too | |
| 11:33:30 | bauzas | whether we should factorize the forgeries into one piece is debatable | |
| 11:33:50 | sean-k-mooney | perhasp something to consier in the future | |
| 11:34:00 | sean-k-mooney | but i dint think its required right now | |
| 11:34:06 | bauzas | yeah | |
| 11:34:54 | bauzas | also, have we ever tried to sanitize os-traits before ? | |
| 11:35:49 | bauzas | sounds now nearly impossible to me | |
| 11:36:13 | sean-k-mooney | we are not allowed to remove traits form os-traits | |
| 11:36:17 | sean-k-mooney | ever | |
| 11:36:24 | sean-k-mooney | placement does not support that today | |
| 11:36:29 | bauzas | I reached to that conclusion | |
| 11:36:55 | sean-k-mooney | it could be done with some mettadata but basically we need to resver the triat ids in the db | |
| 11:40:23 | bauzas | anyway, I need to deserve time for lunch and writing my own spec | |
| 11:56:33 | opendevreview | Merged openstack/nova-specs master: Add libvirt-dev-alias spec https://review.opendev.org/c/openstack/nova-specs/+/892806 | |
| 12:19:52 | opendevreview | sean mooney proposed openstack/nova master: add pyproject.toml to support pip 23.1 https://review.opendev.org/c/openstack/nova/+/899753 | |
| 12:22:06 | Continuity | hey all, I have submitted a PR (https://review.opendev.org/c/openstack/nova/+/900252) and I'm struggling to understand and fix the build errors. Mostly around openstack-tox-cover, openstack-tox-py38 and nova-tox-py310-with-sqlalchemy-2x. It seems to be a case of, works fine on my local machine :D | |
| 12:22:12 | Continuity | anyone have any pointers | |
| 12:28:46 | sean-k-mooney | Continuity: the pep8 failures look valid https://zuul.opendev.org/t/openstack/build/914ce175822a4ada8271b3d5ae9d6877/log/job-output.txt#4269-4282 | |
| 12:30:11 | sean-k-mooney | Continuity: we will also need a bug or specless blueprint to proceed with this feature | |
| 12:31:06 | sean-k-mooney | the unit test failrue also look valid https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_3b1/900252/2/check/openstack-tox-py38/3b1dc22/testr_results.html | |
| 12:31:42 | sean-k-mooney | requests.exceptions.MissingSchema: Invalid URL 'POST': No scheme supplied. Perhaps you meant https://POST? | |
| 12:33:19 | Continuity | sean-k-mooney: yeah I agree they look correct, thats the issue, when I run the tox commands locally they pass :S | |
| 12:33:22 | sean-k-mooney | Continuity: the fact you are gettign fewer error in 3.10 vs 3.8 is concerning | |
| 12:33:34 | Continuity | and yes i need to drop a BP in as well | |
| 12:33:41 | sean-k-mooney | as it imply your relaying on a behaivor in requests that is diffent between the two versions | |
| 12:34:07 | Continuity | not intentionally, but its possible. I will dig a litter deeper | |
| 12:35:23 | sean-k-mooney | in general in not sure how i feel about adding http basic auth to this feature | |
| 12:35:38 | sean-k-mooney | escially since we have not tempest testing of this at all | |
| 12:36:22 | sean-k-mooney | Continuity: we will have to see how others feel about that and if a spec is required and ci coverave beyond unit test. i woudl like to see at least some functionl coverate for this i think | |
| 12:36:38 | sean-k-mooney | but maybe even devstack/tempest testing | |
| 12:37:26 | Continuity | fair enough, basic auth felt like a good step between having to shoehorn keystoneauth into and endpoint that may already exist and nothing. | |
| 12:37:38 | sean-k-mooney | basically devstack woudl jsut have to stand up a fake webservice with http_auth configured that returns some static data to show the auth actully works | |
| 12:37:49 | Continuity | yeah i can deffo take a look at that | |
| 12:38:12 | Continuity | let me get a BP created at least then we can invite conversation.. | |
| 12:38:37 | sean-k-mooney | sure. im not agaisnt basic auth in general but i dont want to see us supproting more auth protocl in the future | |
| 12:38:44 | sean-k-mooney | i.e. oath or saml tokens | |
| 12:38:59 | sean-k-mooney | if we needed anything more advanced then you shoudl use keystone | |
| 12:39:13 | Continuity | yeah | |
| 12:39:15 | Continuity | i agree | |
| 12:39:37 | Continuity | it would get massively messy otherwise. | |
| 12:40:52 | sean-k-mooney | are you missign POST here https://review.opendev.org/c/openstack/nova/+/900252/3/nova/api/metadata/vendordata_dynamic.py#116 | |
| 12:41:24 | sean-k-mooney | oh no | |
| 12:41:32 | sean-k-mooney | your using session.post | |
| 12:42:04 | sean-k-mooney | is there a reason your not using res = self.session.request(url, 'POST', | |
| 12:42:40 | sean-k-mooney | ah sessionis a difent object | |
| 12:42:56 | sean-k-mooney | dependin on if you use _load_ks_session or _load_request_session | |
| 12:43:47 | sean-k-mooney | one is using a keystone auth client vs a requests client directly | |
| 12:48:14 | Continuity | yeah | |
| 12:48:19 | Continuity | might be the wrong approach ... | |
| 12:48:45 | Continuity | i know that that ks_sessions is built ontop of requests, but had issues picking it apart | |
| 12:48:53 | sean-k-mooney | it is yes | |
| 12:48:56 | Continuity | and was trying to not muddy the two | |
| 12:49:05 | sean-k-mooney | using request directly is fine | |
| 12:49:15 | sean-k-mooney | but can you try and use the same method assumign it exits | |
| 12:49:19 | sean-k-mooney | i commented on the patch | |
| 12:49:31 | sean-k-mooney | you are not mockign the funciton that you are calling in the unit test correctly | |
| 12:49:43 | sean-k-mooney | you are using session.post but mocking session.request | |
| 12:50:04 | sean-k-mooney | if we can just use session.request in both cases that woudl be preferable | |
| 12:50:15 | sean-k-mooney | or session.post in both cases | |
| 12:55:30 | Continuity | right, thanks | |
| 12:55:52 | Continuity | sean-k-mooney: appreciate the pointers. I will take another look | |
| 14:36:46 | dansmith | bauzas: I'm around now | |
| 14:39:12 | bauzas | sean-k-mooney: cool, let's see if sean-k-mooney can also be around | |
| 14:39:39 | bauzas | heh, sorry, I meant dansmith | |
| 14:40:17 | dansmith | bauzas: if you just want sean-k-mooney twice and not me, that's fine | |
| 14:41:04 | bauzas | dansmith: nah, was saying let's see if sean-k-mooney and you have time for discussing about the bug :) | |
| 14:41:11 | dansmith | I know :) | |
| 14:41:25 | bauzas | 'sup, sean ? | |
| 14:42:27 | bauzas | dansmith: also fwiw, I eventually accepted to use a trait for the virtio-packedring thing if *to be clear* we use a prefilter that we can delete | |
| 14:42:57 | bauzas | because I want to be clear that standard traits shouldn't be directly used in flavors | |
| 14:43:59 | bauzas | this way, we could do something else for hypervisory-only verifications if we want without holding the spec itself that we already accepted before | |
| 14:44:11 | dansmith | standard traits shouldn't be used in flavors? what does that mean? | |
| 14:47:00 | bauzas | they *could* be used if we don't have prefilters or request spec transformation but, | |
| 14:47:31 | bauzas | for the other ones that have a prefilter, nova shouldn't support the fact to call directly Placement with a resource request using those traits | |
| 14:48:15 | bauzas | by saying "shouldn't support", I think people could do it, but in case computes no longer provide the traits, it wouldn't be a bug | |
| 14:48:36 | bauzas | like we do with, say, virt driver API which is an internal API | |
| 14:48:42 | dansmith | okay I guess I'm confused - one of the primary reasons for this stuff in the first place was so you could have a flavor that guaranteed you some CPU flag like sse3, which is a standard trait | |
| 14:48:59 | bauzas | that's the exception I agree | |
| 14:49:17 | dansmith | that's the _rule_ IMHO :) | |
| 14:49:17 | bauzas | hypervisor specific traits like sse3 are okay to me to be directly used in flavors | |
| 14:49:40 | bauzas | we wouldn't change the computes to no longer pass those traits | |
| 14:49:42 | dansmith | okay I don't see that we can, should, or need to draw the line around things like this new one.. if it's a trait it's a trait | |
| 14:50:18 | bauzas | but for other traits that we create, like the one we discussed yesterday, I'm torn to say 'sure, the compute will always provide it" | |
| 14:50:22 | sean-k-mooney | bauzas: i generally agree with dan the reason traits exist is to request hardware capabliteis like sse3 | |
| 14:50:48 | sean-k-mooney | but we realised that not all capabliteis are hardware capabliets | |
| 14:51:11 | dansmith | right, this new capability is "virtual hardware capability" not unlike a cpu flag, IMHO | |
| 14:51:16 | sean-k-mooney | which is why the compute namespace exists to model the capablity of a hyperviors to emulate/provide a functionaltiy | |
| 14:51:41 | sean-k-mooney | right so https://github.com/openstack/os-traits/tree/master/os_traits/compute | |
| 14:51:49 | sean-k-mooney | is for software cpablies | |
| 14:52:05 | sean-k-mooney | and https://github.com/openstack/os-traits/tree/master/os_traits/hw is for host capablities | |
| 14:52:24 | dansmith | exactly | |
| 14:53:03 | bauzas | yeah, so my point is that some of the traits can directly be used, for sure, like the hardware ones | |
| 14:53:12 | sean-k-mooney | where i kind of sit in this debate is if we have an extra spec to enable a functionlatiy that should automatically result in requeest the capablity | |
| 14:53:25 | bauzas | but for some traits we only use internally for scheduling, those shouldn't be directly used | |
| 14:53:53 | sean-k-mooney | i.e. you should not need to request hw:viommu=true and traits:viommu=required | |
| 14:54:21 | sean-k-mooney | the former shoudl result in the latter being added automatically | |
| 14:54:27 | bauzas | if one day, we think that COMPUTE_NET_VIRTIO_PACKED is not the right trait to provide for supporting the virtio_packed thing, we could be able to no longer provide it | |
| 14:54:54 | opendevreview | Danylo Vodopianov proposed openstack/nova-specs master: Re-propose using VirtIO PackedRing Configuration support for 2024.1 https://review.opendev.org/c/openstack/nova-specs/+/895924 | |
| 14:55:14 | sean-k-mooney | yep and since we dont expect anywone to directly request it (just use the flavor extra spec) that shoudl be fine | |