| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-03 | |||
| 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 | |
| 13:57:32 | dansmith | I think it only happens if we end up with a server in the middle of that while the event comes in, but once the sighup has finished the full restart, the new manager is hooked to new rpc connections, etc | |
| 13:57:38 | dansmith | it doesn't completely bork the server forever, AFAIK | |
| 13:57:48 | dansmith | else it would be really obvious that it's totally broken | |
| 13:58:10 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove unreachable codepaths https://review.openstack.org/649559 | |
| 13:59:49 | mriedem | ok i thought the service was fubar after the sighup | |
| 13:59:57 | mriedem | so this is less severe than i thought | |
| 14:01:13 | bauzas | any urgent reviews I should do before RC2 tagging ? | |
| 14:01:33 | dansmith | not that I know of | |
| 14:01:36 | trident | mriedem: Regarding https://review.openstack.org/#/c/648653/ | |
| 14:01:38 | bauzas | https://etherpad.openstack.org/p/nova-stein-rc-potential is pretty done | |
| 14:02:38 | trident | mriedem: You are correct, forbidden traits never worked unless there were also a required trait in the same flavor. If there were, both would be used, if not, the forbidden trait was lost. | |
| 14:03:14 | bauzas | mriedem: https://review.openstack.org/#/c/649454/ wanting it for RC2 ? I can vote on my own change given melwitt proposed it | |