| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-11-08 | |||
| 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 | |
| 15:02:47 | dansmith | the only way I think a trait makes sense is if you want to be able to say "I asked for packed queue in hw_ and I want you to FAIL if you can't find me a host that supports that" | |
| 15:03:03 | sean-k-mooney | that what it will do | |
| 15:03:28 | sean-k-mooney | the prefilter will check if the flavor has the packed virt queue extra spec and add a required trait | |
| 15:03:39 | sean-k-mooney | so it will fail if we dont find a host | |
| 15:03:40 | dansmith | but you're talking about a prefilter to always do that | |
| 15:03:46 | sean-k-mooney | yes | |
| 15:04:00 | dansmith | what other hw_ things do we enforce at prefilter time that way? | |
| 15:04:04 | sean-k-mooney | unless we just dont do the trait at all | |
| 15:04:09 | sean-k-mooney | sev | |
| 15:04:12 | sean-k-mooney | vtpm | |
| 15:04:42 | sean-k-mooney | we technialy do thoese in teh ResouceRequest.from_request_spec function | |
| 15:04:45 | dansmith | "sev" and "tpm" are not in request_filter | |
| 15:04:45 | sean-k-mooney | rahter then a prefilter | |
| 15:05:16 | sean-k-mooney | if you want a prefilter example cyborg | |