| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-13 | |||
| 16:20:03 | mriedem | if it's thursday and you're using vcenter you get a free vm | |
| 16:20:06 | mriedem | correct | |
| 16:31:42 | cdent | mriedem: nothing ever comes for free from vcenter | |
| 16:33:40 | mriedem | heh | |
| 17:40:28 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/stein: Delete unused get_all_host_states method https://review.opendev.org/676255 | |
| 17:40:28 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/stein: Convert HostMapping.cells to a dict https://review.opendev.org/676256 | |
| 17:40:29 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/stein: Cache host to cell mapping in HostManager https://review.opendev.org/676257 | |
| 17:48:04 | mriedem | the nova-lxc job finds it's first bug https://bugs.launchpad.net/nova/+bug/1840068 | |
| 17:48:04 | openstack | Launchpad bug 1840068 in OpenStack Compute (nova) "(lxc) Instance failed to spawn: TypeError: object of type 'filter' has no len()" [High,Confirmed] | |
| 17:50:36 | sean-k-mooney | oh you/someone got an lxc job running? cool | |
| 17:51:41 | sean-k-mooney | looks like its more or less in teh same nbd mount region of the code that the other issue was in | |
| 17:51:52 | mriedem | this is a py3 specific failure | |
| 17:53:16 | sean-k-mooney | oh that is becasue filter is nolonger a keyword | |
| 17:53:26 | sean-k-mooney | and not you have to import it form itertools | |
| 17:53:40 | sean-k-mooney | so we need to use six | |
| 17:53:56 | sean-k-mooney | s/keywork/built in fucntion/ | |
| 18:01:24 | mriedem | that implies the problem would actually be in py27 rather than 3 | |
| 18:01:25 | mriedem | https://github.com/openstack/nova/blob/e3a55493ba0cb405f14b9090282d93eb230c5715/nova/virt/disk/mount/nbd.py#L44 | |
| 18:01:28 | mriedem | since it's using filter() | |
| 18:01:44 | mriedem | i think the problem is passing an iterable from filter() to random.shuffle | |
| 18:02:00 | sean-k-mooney | well filter is a builtin in python 2 | |
| 18:02:08 | sean-k-mooney | in python 3 its not and you have to import it | |
| 18:02:39 | sean-k-mooney | so i was about to push what i think is a fix | |
| 18:02:40 | mriedem | ok i'm not sure what was used then, because we | |
| 18:02:40 | mriedem | TypeError: object of type 'filter' has no len() | |
| 18:02:59 | sean-k-mooney | it might be something else | |
| 18:03:15 | sean-k-mooney | does this show up in unit tests | |
| 18:03:22 | mriedem | nope, b/c there is no test coverage | |
| 18:03:25 | mriedem | the method is mocked out | |
| 18:03:42 | mriedem | lots of this old virt/disk/mount code isn't tested very deep | |
| 18:03:59 | sean-k-mooney | ok i guess i should add a test. i was hoping ot just quickly fix this and move on | |
| 18:04:08 | mriedem | it uses a stub, | |
| 18:04:09 | mriedem | _fake_detect_nbd_devices_none | |
| 18:04:18 | mriedem | which returns a [] rather than the results of a filter | |
| 18:04:55 | sean-k-mooney | right it shoudl be mocking the os.listdir call really | |
| 18:05:16 | sean-k-mooney | well the stub is fine for oter tests | |
| 18:05:34 | sean-k-mooney | but we should have a test for that _detect_nbd_devices function directly. | |
| 18:09:29 | mriedem | yeah i see there is a _fake_listdir_nbd_devices stub but it's not used by anything, i.e. removing that doesn't make tests fail | |
| 18:21:36 | mriedem | yeah so filter() in py27 returns a list, or some kind of iterable, and in py3 it's a built-in function | |
| 18:21:42 | mriedem | which is why random.shuffle doesn't handle it | |
| 18:27:40 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Introduces SDK to IronicDriver and uses for node.get https://review.opendev.org/642899 | |
| 18:27:41 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for node.list https://review.opendev.org/656027 | |
| 18:27:42 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for validating instance and node https://review.opendev.org/656028 | |
| 18:27:42 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for setting instance id https://review.opendev.org/659690 | |
| 18:27:42 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for add/remove instance info from node https://review.opendev.org/659691 | |
| 18:27:43 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for getting network metadata from node https://review.opendev.org/670213 | |
| 18:40:27 | openstackgerrit | sean mooney proposed openstack/nova master: lxc: make use of filter python3 compatiable https://review.opendev.org/676263 | |
| 18:43:21 | sean-k-mooney | mriedem: i think that should fix it ^ | |
| 18:43:42 | mriedem | yup just commented | |
| 18:43:48 | mriedem | i was working on the same thing | |
| 18:45:10 | sean-k-mooney | :) i can make the chagne you suggest but if you have tehm locally i can also abandon min | |
| 18:45:23 | sean-k-mooney | it will only take 30 secons either way | |
| 18:45:34 | mriedem | i dropped mine | |
| 18:45:55 | sean-k-mooney | ok ill make it a static method so it does not need to be an instance method at all | |
| 18:46:54 | sean-k-mooney | we dont traselate logs anymore right | |
| 18:47:38 | sean-k-mooney | you know what ill jsut fix this and i wont rewrite teh fule file | |
| 18:48:22 | sean-k-mooney | its tempting but i have no idea why we save sutff to self.error in the oter functions and i dont want to break that | |
| 18:48:25 | openstackgerrit | Merged openstack/nova master: Add TargetDBSetupTask https://review.opendev.org/627892 | |
| 18:48:34 | openstackgerrit | Merged openstack/nova master: Prevent init_host test to interfere with other tests https://review.opendev.org/675553 | |
| 18:49:48 | openstackgerrit | Eric Fried proposed openstack/nova master: Get ironicclient retry conf from ksa https://review.opendev.org/676264 | |
| 18:52:08 | efried | dustinc: Having done all that work... now I'm not sure it's the right thing at all. | |
| 18:52:15 | efried | dansmith: Why are those other methods remotable? | |
| 18:53:29 | dansmith | efried: there are some legacy cases where aggregate methods got called from the compute, like when the xen driver was doing some sync | |
| 18:53:44 | dansmith | and potentially also just people adding them because they see the decorator elsewhere and don't understand | |
| 18:53:53 | efried | yes, that's certainly what I would have done | |
| 18:58:43 | openstackgerrit | sean mooney proposed openstack/nova master: lxc: make use of filter python3 compatiable https://review.opendev.org/676263 | |
| 18:59:57 | sean-k-mooney | mriedem: done im going to grab something to eat so ill be afk for a while | |
| 19:06:23 | efried | o/ cfriesen! how are things? | |
| 19:06:55 | cfriesen | efried: just got back from long road trip with family. now recovering from my vacation | |
| 19:06:59 | cfriesen | :) | |
| 19:07:07 | efried | ah yes, I know the feeling | |
| 19:18:53 | mriedem | easy fix https://review.opendev.org/#/c/676263/ | |
| 19:21:31 | dansmith | mriedem: sean-k-mooney won't list() fix it without the six thing? | |
| 19:23:20 | mnaser | mriedem: about https://review.opendev.org/#/c/675496/ would it make sense to make a patch updating the index? | |
| 19:23:28 | mnaser | (in master, so that one alone is backportable) | |
| 19:24:25 | mriedem | dansmith: yeah i think so | |
| 19:25:00 | dansmith | mriedem: so am I missing why we don't just do that? I mean, this works too but I don't think it's what we've done elsewhere | |
| 19:25:29 | mriedem | that's what i was doing in my local patch, but sean-k-mooney beat me to it and used six here so... | |
| 19:25:35 | mriedem | i'm fine with removing the six.moves | |
| 19:26:03 | mriedem | mnaser: you mean change the unique index from just uuid to uuid + deleted? | |
| 19:26:09 | mnaser | yeah | |
| 19:26:12 | dansmith | I know he'll be back.. should we wait to see if he had some reason or just go ahead and fix it? | |
| 19:26:15 | dansmith | not sure if it's a rush | |
| 19:26:38 | mriedem | mnaser: the schema change is a bit wonkier b/c the unique index is defined on the uuid column rather than an actual index | |
| 19:26:49 | mriedem | it's not a rush, can wait for sean | |
| 19:27:17 | mnaser | mriedem: i guess the only issue with that is if you put your entire fleet into maintenance mode and then bring it back | |
| 19:27:20 | mnaser | you just doubled your db | |
| 19:27:32 | mriedem | mnaser: nvm, i guess it's not defined on the column https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/models.py#L115 | |
| 19:27:35 | mnaser | so i think i rather not end up with that type of thing | |
| 19:27:37 | mriedem | it's just a unique index | |
| 19:28:38 | mnaser | my suggestion was to drop the update-deleted workaround | |
| 19:28:52 | mnaser | and make it so that it creates a new entry anyways | |
| 19:28:57 | mnaser | actually i havent had enough coffee | |
| 19:29:00 | mnaser | ignore me :) | |
| 19:31:31 | mriedem | what tz are you in? | |
| 19:32:03 | dansmith | he's a millenial | |
| 19:32:15 | mriedem | the order of the columns in the index also matters and i'm not sure what order we'd use for that unique index | |
| 19:32:16 | dansmith | so, start work at 11am, lunch at 12, etc | |
| 19:33:58 | efried | dustinc: You handy? | |
| 19:35:18 | efried | dustinc: I just put up https://review.opendev.org/676268 which, until it's merged, you should Depends-On from https://review.opendev.org/#/c/642899/ which should also bump the ksa minimum to 3.16.0. | |