| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-03-13 | |||
| 13:48:49 | kashyap | And thanks for all the outstanding (as in, excellent, not "remaining") work! ;-) | |
| 13:49:11 | kashyap | mnaser: Hey, yes... | |
| 13:49:42 | kashyap | mnaser: That issue known due to a libvirt regression, as lyarwood got a patch merged to solve it in Nova -- which is the right thing to do _anyway) | |
| 13:50:05 | kashyap | mnaser: See this one: | |
| 13:50:32 | stephenfin | lyarwood, smcginnis: Think you folks could hit this backport from elod today? It fixes issues with failing tests seen in other backports :) https://review.opendev.org/#/c/712751/ | |
| 13:50:36 | kashyap | mnaser: https://review.opendev.org/#/c/708745/ ("libvirt: Provide the backing file format when creating qcow2 disks") | |
| 13:50:44 | kashyap | mnaser: It's merged in master, Train backport in-progress. | |
| 13:50:45 | stephenfin | (like this one https://review.opendev.org/#/c/711670/) | |
| 13:51:02 | kashyap | Oh, lyarwood already answered it; silly me. I should read the scrollback in full before spamming the channel. | |
| 14:02:59 | lyarwood | kashyap: looking | |
| 14:03:04 | lyarwood | kashyap: ah this one | |
| 14:03:07 | brinzhang | stephenfin: https://review.opendev.org/#/c/712651/ could please check this patch of novaclient? I add a PATCH ``volume-patch`` CLI, debug it, and not found where is wrong | |
| 14:03:23 | kashyap | lyarwood: Yeah, sorry for the noise. | |
| 14:05:45 | lyarwood | stephenfin: ack I'll take a swing later today | |
| 14:10:28 | stephenfin | brinzhang: You need to add a 'patch_servers_1234_os_volume_attachments_Work' method to 'FakeSessionClient' in 'novaclient/tests/unit/v2/fakes.py' | |
| 14:11:04 | stephenfin | with a response mocking what you'd see from nova-api | |
| 14:13:17 | brinzhang | stephenfin: cools, thanks, yep, I will try, I missing that response in 'FakeSessionClient' | |
| 14:17:01 | sean-k-mooney | stephenfin: i cant cross link to source code form nova docs right | |
| 14:17:02 | brinzhang | stephenfin: thanks, I will complete its unit tests, and then update ^^ | |
| 14:17:36 | stephenfin | sean-k-mooney: Only if the code is autodoc'd somewhere, and I think only the notifier stuff falls in that bracket | |
| 14:18:02 | stephenfin | I _think_ you should still be able to use e.g. :py:method:`nova.foo.bar` but it won't resolve to anything | |
| 14:18:05 | sean-k-mooney | stephenfin: ok in that case i think ill have to link to opendev/github | |
| 14:18:47 | sean-k-mooney | stephenfin: context is https://review.opendev.org/#/c/693460/18/doc/source/admin/managing-resource-providers.rst,unified@66 | |
| 14:19:07 | sean-k-mooney | just trying to figure out how i would go ablout liking to the schem file for the provider.yaml validation | |
| 14:19:47 | sean-k-mooney | i think an external link is the only way to do it so ill figure out what the opendev path would be | |
| 14:19:49 | stephenfin | you could just '.. include' it | |
| 14:20:15 | sean-k-mooney | oh i didnt know you could do that am ill try | |
| 14:24:37 | brinzhang | johnthetubaguy: I resolved the os-instance-actions policy issue, and granular the GET API, pls review again. https://review.opendev.org/#/c/706470/ | |
| 14:55:01 | dansmith | brinzhang: assume you saw this right? https://review.opendev.org/#/c/712697/ | |
| 14:55:54 | brinzhang | dansmith: yeah, I have seen it, but I am not very understand why | |
| 14:56:14 | brinzhang | dansmith: so I am not update my patch :( | |
| 14:56:24 | dansmith | brinzhang: understand why what? | |
| 14:58:12 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Use virDomainBlockCopy to swap volumes with Libvirt >= 5.10.0 https://review.opendev.org/696834 | |
| 14:58:34 | dansmith | brinzhang: your patch needs to be rebased on top of that, but otherwise there isn't much you need to do I think | |
| 14:58:36 | brinzhang | dansmith: no, I am still confusing of that change ... | |
| 14:59:40 | brinzhang | I don't seem to grasp the main reason for doing this. | |
| 14:59:57 | brinzhang | I think that's why I am confusing | |
| 15:00:20 | dansmith | brinzhang: the main reason for doing the "format_message()" instead of just "str(e)" ? | |
| 15:00:21 | sean-k-mooney | brinzhang: to not leak sensitive infomation to end users when there are errors | |
| 15:01:09 | brinzhang | <sean-k-mooney> yeah, I know, but can you give me a example? | |
| 15:01:13 | dansmith | yes, | |
| 15:01:26 | brinzhang | I think I need a sample to understand this well | |
| 15:01:30 | dansmith | we had a ceph exception which was something like "Failed to connect to 12.34.56.78" | |
| 15:01:44 | kashyap | lyarwood: I just noticed I didn't hit send on my new comment; done. (See the regression mentioned there.) | |
| 15:01:44 | brinzhang | s/example/sample | |
| 15:01:45 | dansmith | which gets exposed out of the API and users get to see sensitive details like the internal ceph ip address | |
| 15:02:08 | dansmith | brinzhang: I think there was even a case where a credential got leaked.. mriedem might remember the bug(s) to point to | |
| 15:03:32 | brinzhang | dansmith: now we can get the exception only, and cannot get "Failed to connect to 12.34.56.78", right? | |
| 15:04:08 | dansmith | brinzhang: yeah so if the exception is not known (i.e. not inherited from NovaException) we just get the exception *name*, so FailedToConnectToCeph or ConnectionFailure or something like that | |
| 15:04:20 | brinzhang | In other words, we cannot get the details message for the sensitive | |
| 15:04:24 | dansmith | brinzhang: we can *log* it in the compute log for the admin, but we don't want to expose it to the user | |
| 15:04:26 | dansmith | right | |
| 15:05:51 | brinzhang | dansmith: ok, let me taste your changes again | |
| 15:05:56 | dansmith | lol | |
| 15:06:13 | dansmith | brinzhang: um, global pandemic going on right now.. probably best not to lick anything | |
| 15:06:28 | lyarwood | kashyap: ack, we aren't going to hit that FWIW | |
| 15:06:36 | lyarwood | kashyap: we don't use the shallow copy flag | |
| 15:06:37 | brinzhang | dansmith: haha | |
| 15:06:43 | dansmith | brinzhang: :) | |
| 15:06:47 | lyarwood | kashyap: but thanks for raising that | |
| 15:07:29 | brinzhang | dansmith: Thanks to a guy named Guo WeiPeng, I have been in quarantine for another 15 days at home | |
| 15:07:36 | kashyap | lyarwood: Yeah, was reading the details; just wanted to think through if there are any other places we need to bear in mind. | |
| 15:07:47 | dansmith | brinzhang: well, the definitely don't lick *my* patch :) | |
| 15:07:53 | brinzhang | dansmith: so I have too much time to talk with you | |
| 15:08:16 | dansmith | brinzhang: heh, well, I work from home all the time, so I'm pretty much on quarantine normally :) | |
| 15:08:58 | brinzhang | dansmith: you are happiness, it's my dream :) | |
| 15:09:11 | kashyap | Wauw, Dan _is_ Happiness; that's something. | |
| 15:09:59 | dansmith | heh | |
| 15:13:13 | openstackgerrit | Brin Zhang proposed openstack/nova master: Store instance action event exc_val fault details https://review.opendev.org/694428 | |
| 15:14:51 | dansmith | brinzhang: we might want to make sure we have some functional tests that raise both nova and non-nova exceptions in such a way that we can examine them from the API | |
| 15:17:07 | brinzhang | dansmith: I saw you add non-nova test exception in https://review.opendev.org/#/c/712697/1/nova/tests/unit/objects/test_objects.py@1003 | |
| 15:17:36 | brinzhang | You mean, I should add some nova and non-nova exception for os-instance-actions API? | |
| 15:17:36 | mriedem | dansmith: brinzhang: i can look up the cve, but we were exposing credentials to the rbd backing a compute host via instance faults | |
| 15:17:57 | dansmith | brinzhang: yeah, but in a functional test I think | |
| 15:17:57 | mriedem | that's why i referred to the nova.compute.utils code that handles faults | |
| 15:18:07 | dansmith | mriedem: ack yeah | |
| 15:18:08 | mriedem | you could build on the functional test i wrote for ^ | |
| 15:19:02 | mriedem | dansmith: brinzhang: https://review.opendev.org/#/c/674821/ | |
| 15:20:01 | melwitt | kashyap: fyi I added you to this review about adding aarch64 cpu model | |
| 15:20:12 | melwitt | https://review.opendev.org/709494 | |
| 15:20:15 | kashyap | melwitt: Hiya | |
| 15:20:29 | kashyap | melwitt: Will look; thanks for the heads-up | |
| 15:21:43 | melwitt | cool thanks | |
| 15:22:09 | brinzhang | mriedem: dansmith: Looks like I need to add a functional test file for instance_actions, such as nova/tests/functional/test_instance_action.py | |
| 15:22:37 | mriedem | *shrug* there are lots of existing functional tests that make assertions using instance actions, | |
| 15:22:56 | mriedem | i'm not sure there is a module dedicated to instance actions outside of the api samples | |
| 15:23:18 | dansmith | yeah, just another case in one of those, or just add to test_server_faults, IMHO.. it's mostly the same thing | |
| 15:23:43 | mriedem | umm https://github.com/openstack/nova/blob/master/nova/tests/functional/test_instance_actions.py | |
| 15:23:54 | mriedem | or just use the existing module :) | |
| 15:24:12 | dansmith | heh yeah | |
| 15:24:31 | brinzhang | mriedem: yeah, I saw another api sample tests in nova\tests\functional\api_sample_tests\test_instance_actions.py | |
| 15:24:46 | dansmith | *gasp* | |
| 15:24:49 | dansmith | backslashes! | |
| 15:25:04 | mriedem | brinzhang: api samples are generally not really for this type of testing | |
| 15:25:20 | mriedem | they are more about happy path positive test scenarios with minimal fixture | |
| 15:25:31 | brinzhang | mriedem: I know, I will use your paste linke | |
| 15:25:38 | brinzhang | s/linke/link | |
| 15:26:27 | brinzhang | dansmith: I copied from windows, so it's backslashes! | |
| 15:26:38 | dansmith | brinzhang: hence the gasp! :) | |
| 15:27:03 | brinzhang | dansmith: The github sometime I cannot open it fastly, sometime 400 for me, I donot know why .. | |