| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-11-08 | |||
| 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 | |
| 14:55:30 | bauzas | anyway, I don't want us to discuss that much now | |
| 14:55:31 | sean-k-mooney | we can just document "hay dont set this nova will do it for you" | |
| 14:55:51 | bauzas | agreed | |
| 14:56:33 | dansmith | well, it's that reason why I think it's not *worth* a trait really, and a service version is enough to get through this current hoop we have | |
| 14:56:54 | bauzas | dansmith: that's exactly why I meh'd to the spec | |
| 14:57:05 | dansmith | but I'm not wed to either alternative as they both have pros and cons | |
| 14:57:07 | bauzas | if we only do this in a prefilter, we can modify it later | |
| 14:58:18 | bauzas | like, say, by one time, we create some other trait that says "heh, look, I'm a libvirt host" (I know, I know, this is something some people disagree but that's just an example), then we could remove that trait, provided we support both traits during a SLURP time | |
| 14:58:53 | bauzas | again, don't nitpick that example, we could be smarter | |
| 14:59:18 | dvo-plv | sean-k-mooney, bauzas Under words "user should not use trait, only nova". Should I add this here https://review.opendev.org/c/openstack/nova/+/876075/25/releasenotes/notes/packed-virtqueue-filter-43a376674cb5b345.yaml ? | |
| 14:59:50 | bauzas | dvo-plv: I don't think we need | |
| 15:00:01 | sean-k-mooney | dvo-plv: not in the release notes | |
| 15:00:09 | sean-k-mooney | but it could be a note in the feature doc | |
| 15:00:10 | dansmith | I don't think you should say it at all | |
| 15:00:22 | sean-k-mooney | that also ok | |
| 15:00:25 | dansmith | if it's a trait there's no point in us trying to get people to not use it | |
| 15:00:29 | dansmith | that's just confusing | |
| 15:00:29 | sean-k-mooney | provided we dont refence it in our doc | |
| 15:00:34 | dansmith | (and won't work) | |
| 15:00:41 | sean-k-mooney | i.e. just mention the extra spec not the trait in the doc | |
| 15:00:45 | dansmith | I'm confused | |
| 15:00:50 | dansmith | why are we not mentioning it? | |
| 15:01:22 | sean-k-mooney | to use this feature you do not need to set the trait manually in the flavor ro the image | |
| 15:01:36 | sean-k-mooney | you just need to set the flavor extra spec | |
| 15:01:42 | dansmith | but why? | |
| 15:01:46 | sean-k-mooney | the trait is an internal detail and does not need to be in the doc | |
| 15:01:57 | dansmith | then we should _not_ be using a trait, IMHO | |
| 15:02:19 | sean-k-mooney | becuase the prefilte adds it based on teh extra spec | |