| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-06-08 | |||
| 13:32:07 | mriedem | it just gets very complicated | |
| 13:32:29 | mriedem | we could build a 3rd list of networks :) | |
| 13:41:33 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: handle missing rbd_secret_uuid from old connection info https://review.openstack.org/472266 | |
| 13:42:32 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: handle missing rbd_secret_uuid from old connection info https://review.openstack.org/472266 | |
| 13:42:33 | mriedem | sdague: here is that fix for the rbd secret uuid ^ | |
| 13:42:47 | mriedem | sdague: which reminds me of something i've thought about for awhile, | |
| 13:43:02 | mriedem | which is adding an admin api to force a bdm.connection_info refresh | |
| 13:43:26 | openstackgerrit | Alex Szarka proposed openstack/nova master: Add method for verify multiple versioned notifications https://review.openstack.org/465526 | |
| 13:43:36 | mriedem | since if you changed things somewhere on the cinder side, or in your underlying storage, i think you have to detach the volume and re-attach it, or reboot the instance to refresh the connection info | |
| 13:43:48 | mriedem | this has come up when people recycle secrets in ceph | |
| 13:44:00 | mriedem | and the only way to update the existing bdm connection info is via straight db hackery | |
| 13:45:43 | mriedem | so we could do something as simple as an rpc cast to the compute, call os-initialize_connection in cinder to get the connection_info and then update the bdm record | |
| 13:45:59 | mriedem | melwitt: haven't you and i talked about something like this before? ^ or at least a periodic task | |
| 13:46:29 | mriedem | the problem is i bet initialize_connection in cinder isn't the same (idempotent) for all volume backends | |
| 13:49:15 | sdague | mriedem: could that come from cinder in the admin event interface? | |
| 13:49:32 | sdague | it would be best if cinder was the one that said to do that when needed | |
| 13:52:18 | mriedem | it could | |
| 13:52:29 | mriedem | but, | |
| 13:52:48 | mriedem | send what? just telling us to do it? because that api doesn't take something like a big json blob | |
| 13:53:15 | mriedem | we could get a "refresh-connection-info" event from cinder though yeah, like we do for network-changed in neutron | |
| 13:57:02 | sdague | yeh, that's kind of what I was thinking | |
| 13:57:24 | sdague | I guess it should be instance oriented? | |
| 13:58:22 | mriedem | yes it would be | |
| 13:58:26 | mriedem | just sent to the ML | |
| 13:58:33 | mriedem | might be a duplicate idea i've already brought up before | |
| 13:58:35 | mriedem | anyway, bbiab | |
| 14:13:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: remove related pinning from __init__ of InstanceNUMATopology https://review.openstack.org/408242 | |
| 14:13:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: remove cpuset_reserved from __init__ of InstanceNUMATopology https://review.openstack.org/466030 | |
| 14:13:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: remove pagesize from __init__ of InstanceNUMATopology https://review.openstack.org/408241 | |
| 14:18:56 | sfinucan | dansmith: I think I finally figured out the remaining kinks with the above ^^^ patches, if you want to take a look at some point | |
| 14:20:06 | openstackgerrit | Hironori Shiina proposed openstack/nova master: Ironic: Check a last error of power action https://review.openstack.org/470754 | |
| 14:20:20 | bauzas | sfinucan: I think the consensus is not to have __eq__ | |
| 14:20:42 | dansmith | sfinucan: nak on the __eq__ yeah :) | |
| 14:20:49 | sfinucan | bauzas: Oh? Any particular reason? | |
| 14:20:59 | bauzas | sfinucan: when you want to verify o.vo objects, you need to verify the fields | |
| 14:21:39 | sfinucan | bauzas: Yeah - we do that in nova/objects/pci_device.py and nova/objects/numa.py | |
| 14:21:51 | sfinucan | ...and we could also do with it in nova/objects/instance_numa_topology.py | |
| 14:22:09 | bauzas | sfinucan: there are multiple reasons, one being that objects can be semantically identical without being the same, like if they're passed over RPC | |
| 14:24:05 | sfinucan | bauzas: You mean there'd be differences other than timestamps and the likes? | |
| 14:25:30 | bauzas | well, take an object, pass it as an argument to a remote method, you won't get the same object at the end | |
| 14:29:52 | sfinucan | hmm, well I'm going to try something to see if it makes sense | |
| 14:30:01 | sfinucan | it might not, but then I'll know :) | |
| 14:31:19 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP/POC Implement new attach Cinder flow https://review.openstack.org/330285 | |
| 14:31:20 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Skip microversion discovery check for update/delete volume attachments https://review.openstack.org/469668 | |
| 14:31:21 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use microversions for new style volume attachments https://review.openstack.org/469579 | |
| 14:31:21 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use 3.27 microversion when creating new style volume attachments https://review.openstack.org/469609 | |
| 14:31:44 | mriedem | sdague: could you add the bottom 3 changes here to your review queue? ^ | |
| 14:31:50 | mriedem | the new cinder stuff depends on that | |
| 14:33:03 | sdague | yeh, can do | |
| 14:33:21 | sdague | it will be a minute, in the middle of unrelated stuff | |
| 14:34:24 | mriedem | sure, thanks | |
| 14:34:44 | mriedem | dansmith: looks like your devstack fleet week patch needs to depend on the check min service version in all cells patch | |
| 14:34:51 | mriedem | http://logs.openstack.org/94/436094/32/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/0d3a7e2/console.html#_2017-06-08_13_33_37_461957 | |
| 14:34:54 | mriedem | i think... | |
| 14:38:03 | bauzas | sfinucan: the consensus is basically about providing a general __eq__ rule | |
| 14:38:18 | bauzas | sfinucan: semantically, the equivalence really depends on the object type | |
| 14:38:39 | bauzas | sfinucan: like for example, you could assume some fields are irrelevant for that | |
| 14:38:43 | sfinucan | Yeah, that sounds fair. It does seem pretty contextual | |
| 14:39:21 | bauzas | the same goes with __hash__ | |
| 14:39:45 | bauzas | since objects can be very different, we try hard to not define __hash__ on a global basis | |
| 14:40:50 | bauzas | sfinucan: that said, you know we have an obj_equal_prims() right? | |
| 14:41:22 | sfinucan | bauzas: I've just spotted that, alright | |
| 14:41:41 | bauzas | cool | |
| 14:41:58 | bauzas | that's the only thing we can really do on a general basis | |
| 14:42:05 | bauzas | if primitives are equal, fine | |
| 14:42:29 | bauzas | but standing that obj_1 == obj_2 is a total different deal | |
| 14:42:47 | bauzas | same goes for obj1 in an objectlist | |
| 14:46:34 | dansmith | mriedem: aye, not sure why I wasn't hitting that before | |
| 14:47:14 | mriedem | dansmith: i think the test was recently-ish merged | |
| 14:47:22 | mriedem | well, | |
| 14:47:23 | dansmith | ah | |
| 14:47:36 | mriedem | and it required the ssh validation stuff being turned on, which happened globally about 2 weeks ago | |
| 14:47:50 | dansmith | mriedem: thanks for fixing the bottom patch, I just realized I needed to get back to it | |
| 14:48:10 | dansmith | is the merge conflict because of all the infra drama yesterday? | |
| 14:48:17 | mriedem | yes, i've rechecked | |
| 14:48:25 | mriedem | well, i assume it is, i haven't tried rebasing | |
| 14:48:26 | dansmith | cool | |
| 14:48:39 | dansmith | it's running it so I think it must've been | |
| 14:48:42 | dansmith | otherwise it'd kick right out | |
| 14:49:29 | cfriesen | kashyap: I haven't, one of our people might have and I'm pinging for confirmation. We're just going GA with Newton though, so we're not exactly bleeding edge. | |
| 14:52:40 | johnthetubaguy | mriedem: was just taking a peak at the cinder patches (https://review.openstack.org/#/c/469668/3) | |
| 14:53:02 | johnthetubaguy | mriedem: any reason not to cache the availablity of a microversion, once we find its available? | |
| 14:54:53 | cfriesen | johnthetubaguy: couldn't they reboot the cinder service at any time? | |
| 14:55:11 | cfriesen | johnthetubaguy: or are you expecting monotonic increases | |
| 14:55:21 | johnthetubaguy | cfriesen: yeah, but we don't really support downgrades | |
| 14:55:40 | johnthetubaguy | cfriesen: your right in general though, just wondering if we can be simpler | |
| 14:56:14 | johnthetubaguy | actually, yeah, downgrading past this point just isn't going to work, as far as I understand it | |
| 14:58:35 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Add NovaComparableObject mixin https://review.openstack.org/472285 | |
| 14:59:04 | sfinucan | bauzas, dansmith: What about that? ^ | |
| 14:59:06 | mriedem | johnthetubaguy: replied inline | |
| 14:59:18 | mriedem | johnthetubaguy: in general because caching of versions gets complicated | |
| 14:59:18 | sfinucan | johnthetubaguy: Oh hai | |
| 14:59:32 | mriedem | especially when nova as the client might need to request multiple different versions based on which operation we're doing | |
| 14:59:42 | mriedem | like if we need 3.27 today for something but 3.33 tomorrow for something else | |
| 14:59:55 | dansmith | sfinucan: why not use equal_prims for that? | |
| 14:59:56 | mriedem | johnthetubaguy: for now i'm just keeping it simple | |
| 15:00:10 | dansmith | sfinucan: more "is this equal" helpers seems bad to me | |
| 15:00:16 | johnthetubaguy | mriedem: I was thinking caching was simpler, but you are right | |
| 15:00:26 | mriedem | it's simple in the simple case :) | |
| 15:00:29 | johnthetubaguy | mriedem: what you have is clearly correct | |