| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-04 | |||
| 10:02:28 | kashyap | Thanks! | |
| 10:03:02 | mdbooth | kashyap: I eventually resorted to grep :/ | |
| 10:03:18 | mdbooth | Python was hiding that mock extremely well. | |
| 10:03:44 | kashyap | :-) Still reading | |
| 10:03:47 | mdbooth | I couldn't even work it out from the debugger, even though I was almost 100% sure that guest.migrate was being mocked. | |
| 10:04:54 | kashyap | mdbooth: If you couldn't work it out...then I had little hope :P | |
| 10:10:47 | kashyap | mdbooth: So, this part was dodgy: | |
| 10:10:48 | kashyap | 135 self.stub_out('nova.virt.libvirt.guest.Guest.migrate', | |
| 10:10:48 | kashyap | 136 lambda self, destination, migrate_uri, params, flags, | |
| 10:10:51 | kashyap | 137 domain_xml, bandwidth: None) | |
| 10:11:11 | mdbooth | kashyap: Yep. You need to pop the domain_xml argument from that lambda. | |
| 10:11:30 | kashyap | Yep, now that I've removed it | |
| 10:11:47 | kashyap | Huh, "obtuse" was the apt word I was looking for. Thanks | |
| 10:11:59 | kashyap | I'll also see about the 'params' dict comments | |
| 10:12:45 | mdbooth | It was buried in an unrelated file in a function called by the setUp of a Grandparent class. | |
| 10:15:46 | mdbooth | Might do better to convert those few to Mock with an autospec | |
| 10:16:24 | mdbooth | I believe that will behave the same, but will automatically copy the argument signature of the function being mocked | |
| 10:16:55 | kashyap | mdbooth: Probably in separate change? | |
| 10:17:09 | mdbooth | Although tbh it smells that we're looking so deep inside the libvirt driver in that test anyway | |
| 10:17:15 | kashyap | (Prefer to avoid fixing everything in this one change) | |
| 10:17:30 | mdbooth | kashyap: Indeed. It would obviously have to be a preceding change, though. | |
| 10:18:25 | kashyap | mdbooth: Yeah, noted. Thanks, you saved my Monday morning | |
| 10:23:50 | kashyap | mdbooth: You went looking for 'domain_xml' via `grep`? | |
| 10:24:07 | mdbooth | kashyap: Yep :( | |
| 10:24:23 | mdbooth | kashyap: Was looking for mocks including domain_xml | |
| 10:24:40 | kashyap | I see | |
| 10:24:50 | mdbooth | I was pretty sure that migrate() was being mocked and I just couldn't find it. | |
| 10:25:41 | mdbooth | kashyap: Disappointingly brute force. | |
| 10:26:02 | kashyap | Yeah, I was stupefied. Embarassed that I didn't do the `grep` :-) | |
| 10:26:03 | sahid | jangutter: i think you can just add an assertion on the vif_name value | |
| 10:26:10 | sahid | is that not enough? | |
| 10:28:37 | kashyap | sahid: Hey, thanks for the review | |
| 10:30:00 | kashyap | sahid: Are you really -1 on the 'params' thing? | |
| 10:30:50 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Remove usage of migrateToURI{2} APIs https://review.openstack.org/567258 | |
| 10:31:06 | kashyap | sahid: Haven't addressed your comment yet, still thinking and playing locally. | |
| 10:31:12 | sahid | seems reasonable yes, it's convention we have. unfortunatly for that method we had two handle several prototypes but now that you are remove the old method that looks reasonable | |
| 10:31:26 | sahid | s/two/to | |
| 10:32:41 | sahid | kashyap: that should not be so difficult to change, i'm ok if you do that in a next patch | |
| 10:33:02 | kashyap | sahid: Yeah, I am not opposed to it. But you make a good point. So still considering. | |
| 10:33:19 | kashyap | sahid: Would prefer to change it in a next patch, if that's alright | |
| 10:34:26 | sahid | kashyap: ok so please make it to happen and i will review both patch. since that does not really make sence to generate the params dict for only bandwidth and migrate_uri | |
| 10:35:49 | sahid | ok... if you don't want do it now, adding a todo note seems the thing to do | |
| 10:36:01 | sahid | s/ok/or | |
| 10:36:09 | kashyap | sahid: No worries, I'll make a second patch | |
| 10:36:23 | kashyap | sahid: And post it as a two-patch series. | |
| 10:37:00 | kashyap | sahid: Thanks for looking | |
| 10:43:31 | kashyap | Maybe it makes more sense to do it all in one change. | |
| 11:21:07 | pvc | Filter PciPassthroughFilter returned 0 hosts | |
| 11:32:16 | openstackgerrit | Jan Gutter proposed openstack/os-vif master: Add support for generic offload representors https://review.openstack.org/572081 | |
| 11:32:25 | openstackgerrit | Jan Gutter proposed openstack/nova-specs master: Spec to implement vRouter HW offloads https://review.openstack.org/567148 | |
| 11:32:30 | openstackgerrit | Jan Gutter proposed openstack/nova master: Convert vrouter legacy plugging to os-vif https://review.openstack.org/571325 | |
| 11:32:31 | openstackgerrit | Jan Gutter proposed openstack/nova master: [WIP] Add support for vrouter HW offloads https://review.openstack.org/572082 | |
| 11:49:39 | openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add policy to InstanceGroup object and api models. https://review.openstack.org/563375 | |
| 11:49:40 | openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401 | |
| 11:49:41 | openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Change the anti-affinity Filter to adapt to new policy https://review.openstack.org/571166 | |
| 11:49:42 | openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465 | |
| 11:49:43 | openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Microversion 2.63 - Use new format policy in server group https://review.openstack.org/567534 | |
| 12:07:48 | openstackgerrit | Damon Li proposed openstack/nova master: Remove minimum version check when save service https://review.openstack.org/572089 | |
| 12:13:37 | openstackgerrit | Damon Li proposed openstack/nova master: Remove minimum version check when save service https://review.openstack.org/572089 | |
| 12:26:55 | openstackgerrit | Jan Gutter proposed openstack/os-vif master: Add support for generic offload representors https://review.openstack.org/572081 | |
| 12:32:24 | openstackgerrit | Jan Gutter proposed openstack/nova-specs master: Spec to implement vRouter HW offloads https://review.openstack.org/567148 | |
| 13:28:53 | mike99201 | does nova-conductor call transport_url from anywhere other than nova.conf or is it possibly cached somewhere? | |
| 13:31:48 | naichuans | jaypipes: Hi, jay. I have a disscuss with Eric on a vgpu n-rp patch:https://review.openstack.org/#/c/520313/40/nova/virt/xenapi/driver.py. Could you review it and give some suggestion when you have the time? | |
| 13:32:15 | jaypipes | naichuans: yep! | |
| 13:32:36 | naichuans | jaypipes: Thanks! | |
| 13:33:03 | jaypipes | mike99201: it's cached in the API database, unfortunately: https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api_models.py#L118 | |
| 13:33:44 | mike99201 | could that be a reason that conductor service starts find and connects to rabbitmq, but when i launch an instance i'm getting invalid credentials when connecting to rabbitmq? | |
| 13:33:57 | mike99201 | i've changed the password between the time the database was setup and now | |
| 13:36:15 | jaypipes | mike99201: no, I don't think that's an issue. Instances cannot connect to the infrastructure message bus. | |
| 13:36:20 | jaypipes | (by design) | |
| 13:36:24 | efried | naichuans: Hi there. I'm still having trouble seeing the reserved=total case. Since you're online, perhaps you can explain it? | |
| 13:36:25 | dansmith | mike99201: yes | |
| 13:36:39 | dansmith | mike99201: if you changed it in the config and in rabbit after that DB was setup, you need to update the db | |
| 13:36:54 | mike99201 | should I update the database, or can I just delete/recreate it | |
| 13:37:19 | dansmith | update | |
| 13:37:30 | mike99201 | thanks, I'll give it a try | |
| 13:44:43 | openstackgerrit | Chen proposed openstack/nova master: Fix some inconsistencies in doc https://review.openstack.org/570407 | |
| 13:45:12 | efried | jaypipes: In case you were in the middle of looking at the xen thing, I just posted some responses. | |
| 13:50:18 | bauzas | jaypipes: dansmith: I tried to follow your convo for the upgrade thread, but could you please give me the consensus ? | |
| 13:52:53 | efried | bauzas: We're trying to figure out a way to do the upgrade smoothly, in-band, without a separate script. | |
| 13:53:32 | efried | bauzas: It starts with the algorithm I suggested, which is having the virt driver, via update_provider_tree, indicate which resource classes have "moved" from one provider to another. | |
| 13:53:32 | bauzas | efried: by having a new API ? | |
| 13:54:04 | efried | bauzas: Then jaypipes pointed out some race conditions in that algorithm that we can resolve via a new placement API, yes. | |
| 13:54:15 | efried | bauzas: The latter is being worked at https://etherpad.openstack.org/p/placement-migrate-operations | |
| 13:55:01 | efried | brb | |
| 14:00:26 | mriedem | alex_xu_: i have a couple of questions regarding server query parameter validation in this trusted certs patch https://review.openstack.org/#/c/486204/101/nova/api/openstack/compute/schemas/servers.py | |
| 14:00:48 | alex_xu_ | mriedem: yea | |
| 14:00:55 | mriedem | tomorrow if you get a chance | |
| 14:02:36 | mriedem | dansmith: we should probably talk about this in the trusted certs series https://review.openstack.org/#/c/561262/17/nova/virt/driver.py@134 | |
| 14:03:49 | dansmith | mriedem: you know that is just under my name because I split their patch up right? | |
| 14:04:10 | mriedem | yes, but that's not why i'm bringing it up with you specifically | |
| 14:04:26 | mriedem | it's because you and i talk about this same type of thing quite a bit | |
| 14:04:58 | dansmith | okay just checking | |
| 14:05:11 | dansmith | I'm trying to figure something out but I'll go read that comment in a bit | |
| 14:06:41 | melwitt | sahid: hi, just wanted to double check with you that https://blueprints.launchpad.net/nova/+spec/overhead-pin-set has had all related changes merged and okay to mark as complete? | |
| 14:09:47 | mriedem | the other thing that bothers me with the trusted image certs stuff is that by default the policy allows anyone to specify trusted cert image IDs when creating a server, but the default config says that image validation isn't enabled | |
| 14:10:44 | mriedem | the config option just says, | |
| 14:10:45 | mriedem | "If left disabled, image signature verification can still | |
| 14:10:45 | mriedem | occur but the end user will not have any assurance that the signing | |
| 14:10:45 | mriedem | certificate used to generate the image signature is still trustworthy." | |