| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-09-22 | |||
| 12:37:19 | sean-k-mooney | the sdk has 3 levels fo apis. 1 is a raw rest client that just handels auth, 1 is driect method for the indigivual apis and the final one is a high level api that abstrct the differnce between say the old glance image import and the new way using the correct one for the cloud its conneccting too | |
| 12:37:21 | brinzhang0 | Do we plan to replace the SDK? In W release or X release, or more further? | |
| 12:38:12 | sean-k-mooney | brinzhang0: another example is that the high level api provide a way to look up the ip you can ssh over taking into account floating ips and if the vm is directly connect to an external network | |
| 12:38:24 | sean-k-mooney | basically the functionality that shade provided | |
| 12:39:03 | sean-k-mooney | brinzhang0: replce the sdk? no adopt the SDK we should in thoery use it any time we need to call an new api in a different service | |
| 12:39:42 | sean-k-mooney | brinzhang0: for example we shoudl be interacting with cybrog via the sdk rahter then using a python-cyborgclient | |
| 12:40:06 | sean-k-mooney | im pretty sure we already do use the sdk to get a rest client and make direct rest call | |
| 12:40:22 | sean-k-mooney | if the sdk had cyborg supprot we would use that instead | |
| 12:41:04 | brinzhang0 | oh, that the sdk and novaclient will be all exist in Nova, it means we will maintain two tools | |
| 12:41:08 | sean-k-mooney | much of https://github.com/openstack/nova/blob/master/nova/accelerator/cyborg.py could live in the sdk | |
| 12:41:14 | sean-k-mooney | not all of it but much of it | |
| 12:41:36 | sean-k-mooney | brinzhang0: the sdk is a sperate repo | |
| 12:41:46 | sean-k-mooney | not part of nova | |
| 12:42:02 | sean-k-mooney | and novaclinet will eventually be deprecated and removed hopefully | |
| 12:42:22 | sean-k-mooney | just not in W or likely not in X either | |
| 12:42:25 | brinzhang0 | yeah, I know, but if we want to use sdk for the new api call, we need to deploy it, and add the api support in sdk | |
| 12:42:46 | sean-k-mooney | sure | |
| 12:43:46 | brinzhang0 | yes, now we interactiing with cyborg via cyborgclient | |
| 12:44:05 | sean-k-mooney | we do not use cyborgclient | |
| 12:44:27 | sean-k-mooney | and we should not | |
| 12:45:07 | sean-k-mooney | we use a raw rest client and make json queryies to the cyborg api | |
| 12:45:33 | sean-k-mooney | we have our own cyborg client https://github.com/openstack/nova/blob/master/nova/accelerator/cyborg.py#L81 | |
| 12:45:48 | sean-k-mooney | but its not the python-cyborgclient | |
| 12:46:11 | sean-k-mooney | we never use https://opendev.org/openstack/python-cyborgclient | |
| 12:46:38 | sean-k-mooney | and likely never will in nova | |
| 12:47:08 | brinzhang0 | sean-k-mooney: oh sorry, when we complete the nova-cyborg interaction we havenot completed the python-cyborgclient, so we cannot use the python-cyborgclient in that time | |
| 12:47:28 | sean-k-mooney | right but we also would not use it | |
| 12:47:38 | sean-k-mooney | we had already started adopting the sdk | |
| 12:47:43 | brinzhang0 | yes | |
| 12:47:56 | sean-k-mooney | so we would not add a dep on a new project sepcific client | |
| 12:48:01 | sean-k-mooney | we would jsut use the sdk | |
| 12:48:03 | brinzhang0 | we can use sdk instead of python-cyborgclient in future | |
| 12:48:21 | sean-k-mooney | yep we aldready use the sdk in some places by the way | |
| 12:48:41 | sean-k-mooney | anything you see "from openstack" thats using the sdk | |
| 12:48:44 | sean-k-mooney | https://github.com/openstack/nova/search?q=%22from+openstack%22 | |
| 12:49:15 | sean-k-mooney | or were we use get_sdk_adapter | |
| 12:49:36 | sean-k-mooney | that really shoudl be replacing all uses of the keystoneauth adapter eventually | |
| 12:50:36 | brinzhang0 | sean-k-mooney: thans, I will see it later :) | |
| 12:51:17 | sean-k-mooney | so far we are only using that for placement https://github.com/openstack/nova/blob/b1e34c594bc1b3a8c7c12e033e1e4576270e6be7/nova/scheduler/client/report.py#L231 and ironic https://github.com/openstack/nova/blob/99781d6fa96f7d776612bcd41c94f5b9a3724f3b/nova/virt/ironic/driver.py#L216 | |
| 12:51:39 | openstackgerrit | Federico Ressi proposed openstack/nova master: Log stack trace when fails launching a defined domain https://review.opendev.org/750652 | |
| 12:52:10 | openstackgerrit | Federico Ressi proposed openstack/nova master: Requires default python3-devel package on CentOS https://review.opendev.org/752073 | |
| 12:53:21 | brinzhang0 | I think placement only using sdk interacting with nova, and ironic now used python-ironicclient and sdk in Nova | |
| 12:55:21 | sean-k-mooney | placment only used the sdk yes https://github.com/openstack/nova/commit/2a2379a63cab4414676882c068d25163874cf08e and i think ironic only uses it too https://github.com/openstack/nova/commit/0176390531aa7bba56a12960334be4de2dd409ef | |
| 12:56:10 | sean-k-mooney | ya the driver and client wrapper https://github.com/openstack/nova/blob/99781d6fa96f7d776612bcd41c94f5b9a3724f3b/nova/virt/ironic/client_wrapper.py do not import ironic client | |
| 12:57:55 | brinzhang0 | sean-k-mooney: yeah, from the code record *yes*, placement and ironic interacting with nova only use sdk | |
| 12:59:07 | sean-k-mooney | keep in mind that the openstack sdk and openstack client are not the same thing | |
| 12:59:36 | sean-k-mooney | then now are both develoep and maaged via the opesntack-sdks team but they were seperate projects | |
| 12:59:54 | brinzhang0 | yeah, got it | |
| 13:01:53 | brinzhang0 | sean-k-mooney: In ironic we also inint the client, maybe the ironicclient also available https://github.com/openstack/nova/blob/99781d6fa96f7d776612bcd41c94f5b9a3724f3b/nova/virt/ironic/driver.py#L202 | |
| 13:02:32 | sean-k-mooney | that uses the sdk i think | |
| 13:02:49 | sean-k-mooney | that nova ironic wrapper | |
| 13:03:59 | sean-k-mooney | oh its using importutils https://github.com/openstack/nova/blob/99781d6fa96f7d776612bcd41c94f5b9a3724f3b/nova/virt/ironic/client_wrapper.py#L55-L63 | |
| 13:05:44 | sean-k-mooney | ya so it looks like you are right we have not fully removed the use of ironic client | |
| 13:05:45 | brinzhang0 | yes, so the ironicclient still avaliable | |
| 13:05:46 | sean-k-mooney | we should do that | |
| 13:05:58 | brinzhang0 | yep | |
| 13:44:14 | openstackgerrit | sean mooney proposed openstack/nova master: libvirt: delegate ovs plug to os-vif https://review.opendev.org/602432 | |
| 13:46:37 | sean-k-mooney | ignore ^ for now just updated it to add a depens on against a neutron patch | |
| 14:13:45 | CeeMac | lyarwood: https://bugs.launchpad.net/nova/+bug/1896621 | |
| 14:13:45 | openstack | Launchpad bug 1896621 in OpenStack Compute (nova) "instance corrupted after volume retype" [Undecided,New] | |
| 14:19:58 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Test for disabling greendns https://review.opendev.org/753072 | |
| 14:20:39 | artom | dansmith, ^^ brain was fried yesterday, so I decided now leave the improved commit message until this morning | |
| 14:23:55 | dansmith | artom: thanks, much better commit message :) | |
| 14:24:30 | dansmith | artom: I was thinking yesterday later... is there some reason we shouldn't just slap an early call to nova.monkeypatch in the unit test __init__ file? | |
| 14:25:12 | dansmith | we end up patched in the end anyway, right? if we just take the reins and do it as cmd/* does super early, like tests/unit/__init__.py then ... would that do it? | |
| 14:26:05 | artom | dansmith, true, worth a try, at any rate | |
| 14:26:38 | sean-k-mooney | dansmith: we did at one point i think and then removed it | |
| 14:26:58 | sean-k-mooney | https://github.com/openstack/nova/commit/ca1a2c42f5d98983b1fdff60993e6e884787a458 | |
| 14:27:00 | dansmith | sean-k-mooney: meaning some time in the past? was there a reason? | |
| 14:27:26 | dansmith | hrm | |
| 14:27:36 | sean-k-mooney | we indeneted to never need to monkey patch the unit tests | |
| 14:27:40 | sean-k-mooney | apparently | |
| 14:27:43 | dansmith | so we don't end up patched in the end I guess? | |
| 14:27:54 | dansmith | so, there's something else we could do, | |
| 14:28:28 | dansmith | which is split monkey_patch() into two things.. one that sets the params and imports eventlet, and one that does the monkey patching (the latter would call the former).. then we could just call the former in unit test init and be able to check for it | |
| 14:29:01 | dansmith | that would make our unit tests behave more like real life, if it ever mattered, without changing the monkeypatched-ness | |
| 14:29:39 | dansmith | because I was thinking that the rest of our unit tests are apparently running *with* greendns, but our prod code does not, which is a bit wonky | |
| 14:29:48 | dansmith | if we split it thusly, | |
| 14:30:12 | dansmith | we could also put a call to the set-and-import call in the functional init to make sure those tests also run with eventlet configured in the same way as runtime | |
| 14:30:51 | artom | dansmith, to be fair, unit tests should not depend on name resolution... should they? | |
| 14:31:02 | artom | Also, wouldn't that be changing our code to make unit tests happy? | |
| 14:31:23 | dansmith | wouldn't what be changing our code? | |
| 14:31:32 | artom | Splitting monkey_patch() into 2 | |
| 14:31:59 | dansmith | no, because the runtime code would be the same exact thing it is now | |
| 14:32:02 | artom | Also, in the case of greendns at least, we kinda need it to be monkeypatched to test it properly | |
| 14:32:14 | dansmith | it's totally cool to design code so it's easily testable right? | |
| 14:32:18 | artom | True | |
| 14:32:36 | artom | In fact, easy unit tests are usually an indication of good design | |
| 14:32:39 | dansmith | this is my point.. we're testing all our code with eventlet configured differently than we actually run it with | |
| 14:32:57 | dansmith | that's dumb and maybe there shouldn't be any behavior change as a result in the kind of tests we do, but.. there could be | |
| 14:33:13 | sean-k-mooney | dansmith: well i think the rational is unittest should never do io | |
| 14:33:19 | dansmith | so facilitating testing that it's configured as we expect, and running tests with it configured that way seems pretty sane | |
| 14:33:22 | sean-k-mooney | dansmith: so eventlets shoudl never context swtich | |
| 14:33:27 | dansmith | sean-k-mooney: sure I understand | |
| 14:33:50 | dansmith | sean-k-mooney: but all our functional tests used to be under unit/ and we still have plenty that do ... stuff :) | |
| 14:34:03 | artom | Yeah, functional tests definitely write to temp files | |
| 14:34:18 | sean-k-mooney | ya but they are monkeypatched | |
| 14:34:20 | artom | But then they import nova.monkey_path properly | |
| 14:34:54 | sean-k-mooney | the api used to not be monkeypatch in the past if it was run under uwsgi too | |
| 14:35:08 | sean-k-mooney | the conductor and comptue agent are always patched | |