| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-03 | |||
| 13:06:33 | shilpasd | same shared location | |
| 13:06:55 | shilpasd | can you please help me where exatly this logic is | |
| 13:07:23 | shilpasd | i have checked def _create_image() of libvirt | |
| 13:08:26 | openstackgerrit | Jared Winborne proposed openstack/nova master: Leave brackets on Ceph IP addresses for libguestfs https://review.openstack.org/649405 | |
| 13:10:08 | sean-k-mooney | shilpasd: https://github.com/openstack/nova/blob/1554d35834a474514f827449bd7d4f1d2f0af1d6/nova/virt/libvirt/driver.py#L6611-L6641 | |
| 13:11:19 | sean-k-mooney | shilpasd: the issue here however is not related to this check. | |
| 13:11:36 | KH-Jared | I'm just going to blame my long lines on trusting my IDE too much. Its pep8 warning was at 120 characters instead of 79, whoops | |
| 13:12:07 | sean-k-mooney | if you use force host down, you are required to ensure the vm is not running before you call evacuate. that is the cause of the error you hit | |
| 13:13:20 | shilpasd | sean-k-mooney: yes, thanks for this input, will check with VM down, and will debug more the given refrence code, and get back to you on the same later | |
| 13:13:25 | sean-k-mooney | KH-Jared: ya the 79 column limmit is particlaly annoying because no ide defualt to 79. some defult to 80 but even pycharm does not defualt to 79 and its a python focused ide | |
| 13:15:15 | KH-Jared | I can get behind it though, the code has always been extremely nice to read, I assume it adds to that. I also have it fixed in my ide now, so hopefully won't see a pep8 failure again in the future | |
| 13:16:08 | sean-k-mooney | i have been working on openstack for the better part of 6 years and i still dont write pep8 complient code by default | |
| 13:16:26 | sean-k-mooney | but i run fast8 on it most of the time before i push | |
| 13:16:44 | sean-k-mooney | KH-Jared: you are aware of the fast8 env | |
| 13:16:50 | sean-k-mooney | tox -e fast8 | |
| 13:16:53 | KH-Jared | i am now | |
| 13:17:08 | sean-k-mooney | it runs pep8 but just on your patched files | |
| 13:17:13 | sean-k-mooney | not on all of nova | |
| 13:17:15 | sean-k-mooney | is way fater | |
| 13:17:17 | artom | sean-k-mooney, whoa, that exists? | |
| 13:17:21 | sean-k-mooney | *faster | |
| 13:17:30 | sean-k-mooney | artom: yes... | |
| 13:17:43 | artom | I've been hacking something like it with $(tox -e pep8 `git show --name-only | grep ^nova`) | |
| 13:18:09 | sean-k-mooney | artom: stephenfin added it in pike | |
| 13:18:16 | artom | ... | |
| 13:18:20 | KH-Jared | this was my first change on a large project, not even just openstack, all testing I've done has been much smaller by comparison. I'm going to be using flake8 more often atleast | |
| 13:18:21 | artom | \o/ | |
| 13:18:22 | sean-k-mooney | its also in a few other repos at this point | |
| 13:18:44 | efried | jaypipes: Do we still have one RT per ironic node? | |
| 13:19:23 | mriedem | gibi_off: do you know why this was made into a warning? https://github.com/openstack/nova/blob/b33fa1c054ba4b7d4e789aa51250ad5c8325da2d/nova/scheduler/client/report.py#L1880 we hit that a lot in normal resizes: https://bugs.launchpad.net/nova/+bug/1822917 | |
| 13:19:24 | openstack | Launchpad bug 1822917 in OpenStack Compute (nova) ""Overwriting current allocation" warnings in logs during move operations although there are no failures" [Undecided,In progress] - Assigned to Takashi NATSUME (natsume-takashi) | |
| 13:20:42 | mriedem | efried: there is one RT per nova-compute service, | |
| 13:20:47 | mriedem | and the RT has a dict of compute nodes | |
| 13:21:13 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L544 | |
| 13:21:32 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L139 | |
| 13:21:52 | efried | mriedem: I'm looking at stephenfin's change https://review.openstack.org/#/c/649559/ which seems sane in itself, but https://review.openstack.org/#/c/649559/1/nova/compute/resource_tracker.py@605 is only being run through once, for the "first" node. | |
| 13:22:26 | efried | it's probably moot by luck because we don't track PCI devices on ironic nodes (right?) | |
| 13:23:20 | mriedem | probably | |
| 13:25:29 | jaypipes | efried: no, not for years. | |
| 13:25:44 | efried | swhat I thought, see above | |
| 13:26:22 | jaypipes | k, will look. | |
| 13:29:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Remove dead resource tracker code https://review.openstack.org/649569 | |
| 13:30:58 | sean-k-mooney | stephenfin: regarding ^ this conflits with some other chages that might be starting to used some of that dead code | |
| 13:31:18 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Remove dead nova.db functions https://review.openstack.org/649570 | |
| 13:31:30 | sean-k-mooney | im thinking about https://review.openstack.org/#/q/topic:bug/1809095+(status:open+OR+status:merged) | |
| 13:32:19 | stephenfin | sean-k-mooney: Possibly. If so, let me know. I was just using vulture (https://pypi.org/project/vulture/) to figure that stuff out so there's a lot of missing context | |
| 13:32:25 | sean-k-mooney | stephenfin: actully no never mind | |
| 13:32:27 | sean-k-mooney | they are not | |
| 13:32:35 | sean-k-mooney | its just a conflit in the tests | |
| 13:33:16 | sean-k-mooney | stephenfin: we might have some function that were added for the sriov migration code too | |
| 13:33:31 | sean-k-mooney | we merged all the resouce tracker code but have not merged the 2 patches that use it | |
| 13:33:49 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/stein: Error out migration when confirm_resize fails https://review.openstack.org/649421 | |
| 13:36:00 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Remove dead nova.db functions https://review.openstack.org/649570 | |
| 13:36:21 | mriedem | sean-k-mooney: are you going to cherry pick https://review.openstack.org/#/c/649409/ to stable/stein now? | |
| 13:36:23 | sean-k-mooney | speaking of https://review.openstack.org/#/q/topic:bp/libvirt-neutron-sriov-livemigration+(status:open) if people have time to review that again it would be nice to see that merged now that train is open | |
| 13:36:27 | mriedem | efried: we're doing an rc2 for https://review.openstack.org/#/c/649409/ right? | |
| 13:36:50 | gibi_off | mriedem: I didn't find the specific reason for the warning in move_allocation so I guess I added it becase it thought that is should not really happen that we overwrite existing allocation during move | |
| 13:36:53 | sean-k-mooney | im waiting for zuul to merge it | |
| 13:37:08 | sean-k-mooney | but yes ill cherry pick it once that is done | |
| 13:37:28 | mriedem | gibi_off: it's there because of https://specs.openstack.org/openstack/nova-specs/specs/queens/implemented/migration-allocations.html | |
| 13:37:30 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Remove unused constants, functions https://review.openstack.org/649567 | |
| 13:37:35 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Remove dead resource tracker code https://review.openstack.org/649569 | |
| 13:37:41 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Remove dead nova.db functions https://review.openstack.org/649570 | |
| 13:38:32 | sean-k-mooney | ... it going to fail again because fo failure in lower constarits | |
| 13:38:39 | stephenfin | mriedem, gibi_off: Have reshuffled those around to remove the placement changes and fix an issue with the last patch, FYI | |
| 13:38:52 | sean-k-mooney | http://logs.openstack.org/09/649409/3/check/openstack-tox-lower-constraints/1a8274d/testr_results.html.gz | |
| 13:40:13 | mriedem | i assume that's related to http://status.openstack.org/elastic-recheck/#1793364 but the signature on that query is old | |
| 13:40:22 | gibi_off | mriedem: so during revert resize/migrate the target allocation is non empty as we keep the allocation exists both on the source and the target host of the migration | |
| 13:41:38 | mriedem | during revert we should drop the target node allocations, held by the instance consumer, and move the source node allocations, held by the migratoin consumer, to the instance consumer | |
| 13:41:39 | sean-k-mooney | mriedem: well the patch to fix our lower constratins job just laned so maybe this is a bug that was fixed in later versions of sqlacamy/pymysql | |
| 13:41:54 | mriedem | so after revert there are no allocations for the instance on the target node and the instance has the source node allocations again | |
| 13:42:19 | gibi_off | mriedem: yeah I mixed up it is not moving allocation between hosts it moves between cosnumers | |
| 13:42:35 | mriedem | dansmith: sounds like we might be doing an RC2 for stein, and bug 1715374 is latent, but do you think it would be worth putting out a known issue release note for stein anyway? | |
| 13:42:36 | openstack | bug 1715374 in OpenStack Compute (nova) "Reloading compute with SIGHUP prevents instances from booting" [High,In progress] https://launchpad.net/bugs/1715374 - Assigned to Ralf Haferkamp (rhafer) | |
| 13:42:59 | dansmith | mriedem: I dunno, it's been this way for a long time apparently | |
| 13:43:06 | dansmith | so doesn't really seem like it | |
| 13:43:38 | gibi_off | mriedem: anyhow I think the warning can be deleted | |
| 13:43:53 | mriedem | gibi_off: or at least dropped to debug | |
| 13:44:03 | gibi_off | yeah | |
| 13:45:31 | mriedem | dansmith: just thinking about our upgrade docs and such that mention to use sighup during an upgrade https://docs.openstack.org/nova/stein/user/upgrade.html?highlight=sighup#concepts | |
| 13:45:51 | dansmith | well, I know | |
| 13:45:52 | mriedem | maybe that should have a note instead | |
| 13:46:01 | dansmith | yeah, that would make more sense I think | |
| 13:46:31 | mriedem | and https://docs.openstack.org/nova/stein/configuration/config.html?highlight=sighup#compute.resource_provider_association_refresh for efried | |
| 13:46:48 | mriedem | ^ might work for that option, but then kills the event listening stuff so you can't boot a server right? | |
| 13:46:56 | mriedem | neutron events i mean | |
| 13:48:21 | mriedem | blech we have stuff in here too https://docs.openstack.org/nova/stein/admin/configuration/schedulers.html?highlight=sighup#compute-capabilities-as-traits | |
| 13:49:59 | mriedem | in other fun news, i learned last night that with cross-cell resize, we have to do a hard delete of the instance in a cell db rather than a soft delete | |
| 13:49:59 | sean-k-mooney | so appraently rechecking a runing job does not work so i can either rebase to head of mater if peopel feel like +w again or i can recheck in 50mins when zuul finishies it current run | |
| 13:50:16 | mriedem | sean-k-mooney: just rebase | |
| 13:50:24 | openstackgerrit | sean mooney proposed openstack/nova master: Libvirt: gracefully handle non-nic VFs https://review.openstack.org/649409 | |
| 13:51:20 | sean-k-mooney | done that will retriger the jobs and it just need +w. im going to grab lunch before a meeting so brb | |
| 13:54:40 | mriedem | so i wonder if we could recreate this sighup issue in one of our post-test hook scripts, we'd just sighup the local compute service and then create a server which should timeout waiting for the network-vif-plugged callback right? | |
| 13:54:53 | dansmith | no, | |
| 13:55:02 | dansmith | you have to have a server in the middle of creating when you sighup I think | |
| 13:55:07 | dansmith | before the event comes in | |
| 13:55:38 | dansmith | the other option is probably just to pick another signal as a stop-gap, register for it ourselves, and wire it up to our existing handlers | |
| 13:55:45 | dansmith | but that's even more icky for rc2 | |
| 13:56:04 | mriedem | on a new server create after the signup, won't the _events dict be None and when registering the callback we'd hit this? https://review.openstack.org/#/c/420026/7/nova/compute/manager.py@316 | |
| 13:56:53 | dansmith | I don't think so because we'll be re-created at the point before we get there | |