| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-02-06 | |||
| 14:56:33 | bauzas | cool, that's what I hoped | |
| 14:56:37 | bauzas | a synchronous 40x | |
| 14:56:53 | sean-k-mooney | proably a 400 bad request | |
| 14:56:57 | bauzas | because we also need to make it for the translation mechanism | |
| 14:57:03 | sean-k-mooney | a 4xx for sure | |
| 14:57:03 | bauzas | ideally, same exception | |
| 14:57:30 | brinzhang | efried: how about now? :) | |
| 14:57:32 | sean-k-mooney | ya so this is all handeled in the hardware.py module | |
| 14:57:46 | sean-k-mooney | and its reused in several places | |
| 14:58:23 | efried | brinzhang: for which? The spec review? | |
| 14:58:56 | sean-k-mooney | bauzas: its all handeled by https://github.com/openstack/nova/blob/014c1ab864cec6213750c5ef57d8080ae8cd973d/nova/virt/hardware.py#L1742 | |
| 14:59:00 | brinzhang | efried: I added a new policy to the os-instance-action https://review.opendev.org/#/c/706179/2/nova/policies/instance_actions.py@27, but while run the unit test in cmd.test_policy, it was failed, and the "passing_rules " cannot get the new rule 'os_compute_api:os-instance-actions:show_details | |
| 14:59:24 | bauzas | sean-k-mooney: thanks, looking | |
| 14:59:38 | bauzas | okay, will look | |
| 14:59:54 | brinzhang | efried:https://github.com/openstack/nova/blob/master/nova/tests/unit/cmd/test_policy.py#L119 the check function is here | |
| 15:00:18 | Sundar | efried, dansmith, sean-k-mooney: Re. https://review.opendev.org/#/c/631243/55/nova/accelerator/cyborg.py@86, I am adding checks for keystone exceptions here. However, if this 'get device profile' call fails, there will be no further calls to Cyborg for that instance. So, there is no point in adding redundant exception checks to other calls for | |
| 15:00:18 | Sundar | creating ARQs, etc. Do you agree? | |
| 15:00:48 | sean-k-mooney | bauzas: the importnt bit for you is https://github.com/openstack/nova/blob/014c1ab864cec6213750c5ef57d8080ae8cd973d/nova/compute/api.py#L925 in _validate_and_build_base_options | |
| 15:01:15 | dansmith | Sundar: you should assume every call to another service could fail | |
| 15:01:29 | bauzas | sean-k-mooney: anyway, I'll write something in the spec and you'll chime in | |
| 15:01:54 | efried | brinzhang: I'm afraid I have no clue about this. I would have to dig in fresh, and I don't have time to do that right now. However, I think gmann might be able to address your questions much more easily. | |
| 15:01:54 | sean-k-mooney | bauzas: cool | |
| 15:02:06 | efried | brinzhang: not sure where he is... | |
| 15:02:25 | bauzas | basically, my concern is 'I want 8 vCPUs, 7GB of RAM and hw:numa_nodes=2', accordingly the RAM has to be provided by two different memory pages that are in each NUMA nodes | |
| 15:02:29 | bauzas | sean-k-mooney: ^ | |
| 15:02:54 | bauzas | meaning that the translation mechanism will split this memory amount in twice (or more) | |
| 15:02:58 | stephenfin | I want to use an unreleased feature in neutronclient in a nova patch. For 'Depends-On' to work, I need to to add 'python-neutronclient' to 'required-projects' in '.zuul.yaml', right? | |
| 15:03:02 | sean-k-mooney | ya you dont need to wory about that really | |
| 15:03:06 | brinzhang | efried: ok, I will wait him, and I pinged him another question too, I will wait for him ,thanks | |
| 15:03:34 | sean-k-mooney | if the requst_spec has a numa_toplogy objct its a numa instnace and all of that has already been calualted | |
| 15:03:41 | efried | bauzas: yes, that sounds correct to me | |
| 15:03:48 | sean-k-mooney | so you just need to read the value form the numa toplogy object | |
| 15:04:22 | efried | stephenfin: yes, that sounds correct. But we wouldn't want to merge that change. | |
| 15:04:28 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Use neutronclient's port binding APIs https://review.opendev.org/706295 | |
| 15:04:31 | stephenfin | efried: Yup, just a WIP for now | |
| 15:04:53 | efried | stephenfin: and that only works for zv3 jobs btw, not for legacy (IIUC) | |
| 15:04:59 | sean-k-mooney | Sundar: im goign to redeploy with the latest patches are they all updated | |
| 15:05:30 | Sundar | dansmith: Yes, but we are talking of failures like EndpointNotFound, MissingAuthPlugin, and Unauthorized. If the first call succeeds, is the second one going to fail for one of these reasons? | |
| 15:05:36 | sean-k-mooney | you said you tought i was using an old set of cybog pataches when you looked at the error i got on monday but that was the latest set that were up at that point | |
| 15:05:44 | Sundar | sean-k-mooney: Great, thanks | |
| 15:05:59 | dansmith | Sundar: no, this is a distributed system.. you should never make that assumption | |
| 15:06:17 | mriosfer | Sean: if host got two sockets , is recomended to "force" hw:numa_nodes=2 ? If vcpus are more 2 / 4 / 6 / 8 etc... | |
| 15:06:26 | efried | Sundar: If you wanted to trap those failures (you can use the superclass ClientException) and convert them to DeviceProfileError("Couldn't communicate with cyborg!") I think that would be okay. | |
| 15:06:42 | dansmith | Sundar: calls from nova to cyborg (and back) may be routed to different api endpoints on subsequent calls, in different states of broken, misconfigured, or transient upgrade | |
| 15:06:47 | openstackgerrit | Stephen Finucane proposed openstack/nova master: WIP: Use neutronclient's port binding APIs https://review.opendev.org/706295 | |
| 15:06:52 | sean-k-mooney | mriosfer: that depens on your workloads but i do for larger vms | |
| 15:06:54 | dansmith | efried: agreed | |
| 15:07:06 | stephenfin | kashyap: np, I figured it would just be easier do it myself real quick. No skin off my back | |
| 15:07:12 | mriosfer | Sean : larger VM from 8 vcpu? | |
| 15:07:15 | mriosfer | or more | |
| 15:07:25 | sean-k-mooney | for small vms of 4-8 CPUs i will often keep them with hw:numa_nodes=1 | |
| 15:07:39 | sean-k-mooney | ya over 8 i tend to always split | |
| 15:07:48 | openstackgerrit | Brian Rosmaita proposed openstack/nova master: Do not inherit cinder_ image properties https://review.opendev.org/706298 | |
| 15:07:49 | sean-k-mooney | to hw:numa_nodes=2 | |
| 15:08:04 | stephenfin | dansmith: That neutron fix yesterday was incomplete, btw. ralonsoh and I worked out the fix this morning if you care to look again https://review.opendev.org/#/c/706213/ | |
| 15:08:22 | dansmith | stephenfin: I'm reading now | |
| 15:08:26 | stephenfin | ta | |
| 15:08:52 | sean-k-mooney | but that is based on host core counts of 10-16 cores per socket. if you have 32 cores per socket then you might adjust what you consider big | |
| 15:08:57 | mtreinish | stephenfin: thanks for the reminder, I'll re-review that PR today or tomorrow | |
| 15:08:58 | dansmith | stephenfin: where is the code that actually uses this? | |
| 15:09:23 | stephenfin | dansmith: Just nova/api/openstack/compute/floating_ips.py | |
| 15:09:52 | dansmith | stephenfin: "port_details" is not in that file | |
| 15:10:00 | dansmith | do you mean it just passes through the structure to the api requester? | |
| 15:10:04 | Sundar | efried, dansmith: Got it. Yes, I would handle service failures in distributed systems in general. I asked because I don't see these specific checks everywhere e.g. https://github.com/openstack/nova/blob/master/nova/image/glance.py . But, I agree it is better to do it everywhere. | |
| 15:10:29 | stephenfin | dansmith: sure it is - line 40 | |
| 15:11:10 | mtreinish | stephenfin: the only thing is we're in prep for 3.0.0 (which removes 2.7). So that PR probably won't get released until then which we're shooting for mid-March | |
| 15:11:30 | dansmith | stephenfin: % grep -c port_details nova/api/openstack/compute/floating_ips.py | |
| 15:11:31 | dansmith | 0 | |
| 15:12:11 | stephenfin | dansmith: It's definitely there :) https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/floating_ips.py#L40 | |
| 15:12:34 | dansmith | oh is this a recent change? /me rebases | |
| 15:13:01 | sean-k-mooney | dansmith: stephenfin started usign it as part of the nova net removal | |
| 15:13:07 | sean-k-mooney | so yes | |
| 15:13:15 | dansmith | yeah I see | |
| 15:13:15 | stephenfin | Yup, it was picked up in one of the "Stop using nova-network objects as containers" patches | |
| 15:13:42 | sean-k-mooney | so floating ips are not ports and do not normally have port details | |
| 15:13:42 | stephenfin | mtreinish: That's okay. To be honest, the more crucial fix is the subunit fix and I need lifeless for that | |
| 15:13:46 | dansmith | okay just wanted to see what used it and that it was okay with None | |
| 15:14:02 | sean-k-mooney | there is a neutorn extnion to expose the port_details of the prot that they are attached too when attached | |
| 15:14:13 | sean-k-mooney | but its optional | |
| 15:14:19 | sean-k-mooney | apparently | |
| 15:14:35 | stephenfin | mtreinish: https://github.com/testing-cabal/subunit/pull/40 for reference (I think I referenced it the stestr PR) | |
| 15:17:31 | sean-k-mooney | stephenfin: when did we deprecate teh proxy api | |
| 15:18:08 | dansmith | stephenfin: so because this broke the neutron gate, I assume we have tempest tests that actually use this proxy api, right? | |
| 15:18:17 | stephenfin | sean-k-mooney: nova/api/openstack/api_version_request.py tells me 2.36 | |
| 15:18:50 | stephenfin | dansmith: correct. the reason we didn't see it was because we test using ML2-OVS or ML2-LB (I don't know which) in the gate, not ML2-OVN | |
| 15:18:56 | sean-k-mooney | well it was only added to neutron in 2018 | |
| 15:18:57 | sean-k-mooney | https://github.com/openstack/neutron-lib/commit/24a0877d1d234830a36794388bb342d3d91b9230 | |
| 15:19:05 | sean-k-mooney | which is well after we had deprecated it | |
| 15:19:17 | dansmith | stephenfin: yeah I get that. are the tests _testing_ the proxy or using it out of convenience? | |
| 15:19:19 | sean-k-mooney | 2.36 was newton | |
| 15:19:48 | stephenfin | sean-k-mooney: Yup, it's not the proxy API that was wrong, it was how I updated things to stop using nova-network o.vo's | |
| 15:20:29 | sean-k-mooney | stephenfin: what im saying is the proxy api should not conatin that info | |
| 15:20:54 | sean-k-mooney | we deprecated itn in 2016 that extention was only added in 2018 | |
| 15:21:15 | dansmith | sean-k-mooney: I'm guessing we had it in there before via other mechanisms | |
| 15:21:18 | sean-k-mooney | so there should be noting relying on it for fip port details | |
| 15:21:28 | sean-k-mooney | ya proably | |
| 15:21:34 | dansmith | sean-k-mooney: looking at the older code I was originally looking at, it was still putting those things in there but in a different way | |
| 15:21:38 | stephenfin | dansmith: That I don't know. Maybe ralonsoh knows since he had a patch up against neutron to disable the broken job yesterday | |
| 15:21:53 | sean-k-mooney | the correct way to look this up before was you get all the ports attach to the vm then get the fip for each port | |
| 15:22:05 | openstackgerrit | Brin Zhang proposed openstack/nova master: Add SYSTEM_READER role to servers actions API https://review.opendev.org/706179 | |