| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-20 | |||
| 17:33:57 | johnthetubaguy | edmondsw: very true. We had deployers complain they had to tweak config and policy files in sync with upgrades, so we promised to avoid that, hence that was added in there and the grenade tests kinda built around that assumption. | |
| 17:34:13 | edmondsw | sure, I'm fully on board with that | |
| 17:35:13 | edmondsw | the main point here is that this is not going to break anyone. We're fixing a bug here. The chances of someone relying on this bug for things to work are miniscule | |
| 17:35:39 | edmondsw | furthermore, it's not possible to fix this bug without any possible chance of an upgrade impact | |
| 17:36:19 | edmondsw | johnthetubaguy so do we not fix the bug? That would be much worse for operators | |
| 17:38:08 | edmondsw | johnthetubaguy we can't leave a fully-supported API not only doing its own policy check but also relying on the policy check of another API that is deprecated... non-deprecated cannot rely on deprecated | |
| 17:38:36 | johnthetubaguy | edmondsw: as I said above, I am just trying to make sure we are doing the best thing here, you probably are | |
| 17:38:45 | edmondsw | as an operator, I want to restrict policy for the deprecated API more than for the non-deprecated API, and I can't do that unless we fix this | |
| 17:38:51 | edmondsw | cool | |
| 17:41:07 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/nova master: Read NIC features in libvirt https://review.openstack.org/444415 | |
| 17:43:14 | johnthetubaguy | edmondsw: I should nudge sneti and aunnam about this change, as it will sure help the policy descriptions to have this one merged | |
| 17:43:46 | edmondsw | sneti aunnam talking about https://review.openstack.org/#/c/389314/ | |
| 17:44:56 | johnthetubaguy | edmondsw: got a nit on the reno, what do you think is best there? | |
| 17:45:48 | sneti | edmondsw, checking that | |
| 17:46:17 | edmondsw | johnthetubaguy let me see what I can do there | |
| 17:47:04 | edmondsw | johnthetubaguy fyi, I removed _items to satisfy your earlier comment :) | |
| 17:47:26 | johnthetubaguy | edmondsw: sorry, I was unclear, it was the policy check moving that worried me | |
| 17:47:36 | edmondsw | I could go back to patch set 5 if you prefer | |
| 17:48:10 | johnthetubaguy | edmondsw: but that moves the polcy check into the _details method, which is the bit that I really don't like | |
| 17:48:34 | johnthetubaguy | edmondsw: just wanting to see policy checks at the top of all the API methods, where possible | |
| 17:49:00 | edmondsw | johnthetubaguy what about _details? there is no _details here | |
| 17:49:14 | johnthetubaguy | edmondsw: oops, I mean _items | |
| 17:49:36 | edmondsw | I don't think it makes sense to create the context var in the API method so we can use it for the policy check, then either recreate it in _items or update _items to let it be passed in, when _items isn't really needed anyway | |
| 17:49:45 | edmondsw | easier to just remove it | |
| 17:49:59 | edmondsw | but that's just my opinion... I can do it differently if need be | |
| 17:51:20 | johnthetubaguy | edmondsw: agreed with you, its just there are two changes in a single patch there, which isn't ideal | |
| 17:51:34 | edmondsw | sure | |
| 17:52:15 | johnthetubaguy | not blocking on that though, because its simple enough, and I like that change, its just a nit | |
| 17:56:03 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Allow PUT and POST without bodies https://review.openstack.org/447625 | |
| 17:56:23 | cdent | alex_xu: assume you are gone now, but for when you return ^ | |
| 17:56:56 | mriedem | EmilienM: looks like the revert patch failed in the ovb-nonha job, timeout in infra https://review.openstack.org/#/c/447514/ | |
| 17:58:13 | edmondsw | johnthetubaguy how does this sound for the reno? The os-volume_attachments APIs no longer check ``os_compute_api:os-volumes`` policy. They do still check ``os_compute_api:os-volumes-attachments`` policy rules. Deployers who have customized policy should confirm that their settings for os-volume_attachments policy checks are sufficient. | |
| 17:58:41 | johnthetubaguy | edmondsw: thats cool, that matches what we do for config then. | |
| 17:58:50 | edmondsw | cool, I'll throw it up | |
| 18:04:14 | openstackgerrit | Matthew Edmonds proposed openstack/nova master: fix os-volume_attachments policy checks https://review.openstack.org/389314 | |
| 18:04:48 | sneti | edmondsw, that's a simple change. so two changes in single patch looks good. may be you can add a comment on why it is removed | |
| 18:06:32 | edmondsw | sneti, sure, I'll edit the commit msg | |
| 18:06:37 | openstackgerrit | Claudiu Belu proposed openstack/nova master: POC: tests: autospecs all the mock.patch usages https://review.openstack.org/447505 | |
| 18:07:13 | sneti | edmondsw, thanks! | |
| 18:07:30 | EmilienM | mriedem: right, though it passed our functional tests (called pingtest) | |
| 18:08:22 | mriedem | claudiub: if you're still around, can you also hit these in newton and mitaka? https://review.openstack.org/#/q/I1d217620e52d45595a3e0e49ed57b4ab33cd1688,n,z | |
| 18:08:48 | openstackgerrit | Matthew Edmonds proposed openstack/nova master: fix os-volume_attachments policy checks https://review.openstack.org/389314 | |
| 18:09:11 | edmondsw | johnthetubaguy sneti ^ | |
| 18:09:28 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Allow PUT and POST without bodies https://review.openstack.org/447625 | |
| 18:11:49 | sdague | hmmm... bad log message somewhere - http://logs.openstack.org/90/417590/9/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/c831c5d/logs/screen-n-cpu.txt.gz?level=TRACE#_2017-03-20_17_10_19_178 ? | |
| 18:17:50 | mriedem | i think i know where | |
| 18:19:54 | mriedem | ha it's actually super obvious | |
| 18:19:56 | mriedem | if thing: | |
| 18:20:09 | mriedem | else: LOG.(something %s, thing.attribute) | |
| 18:21:09 | mriedem | https://review.openstack.org/#/c/381888/1/nova/compute/manager.py@6641 | |
| 18:23:42 | openstack | Launchpad bug 1674449 in OpenStack Compute (nova) "NoneType logging warning with "Received unexpected event network-vif-unplugged" message" [Medium,Triaged] | |
| 18:23:42 | mriedem | sdague: you want to fix it? https://bugs.launchpad.net/nova/+bug/1674449 | |
| 18:23:51 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Log a warning if there is only one cell when listing instances https://review.openstack.org/440652 | |
| 18:23:58 | sdague | mriedem: sure | |
| 18:24:12 | sdague | also... http://logs.openstack.org/90/417590/9/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/c831c5d/logs/screen-n-sch.txt.gz#_2017-03-20_17_07_00_573 | |
| 18:24:26 | sdague | why would the compute node be auto disabled in the middle of a tempest run? | |
| 18:24:38 | mriedem | b/c libvirt crashes | |
| 18:24:43 | mriedem | that's an old gate bug for xenial | |
| 18:24:55 | mriedem | the libvirt driver auto-disables the service when the host connection drops | |
| 18:25:07 | mriedem | http://status.openstack.org/elastic-recheck/#1643911 | |
| 18:25:12 | mriedem | http://status.openstack.org/elastic-recheck/#1646779 | |
| 18:25:19 | mriedem | http://status.openstack.org/elastic-recheck/#1638982 | |
| 18:27:37 | sdague | mriedem: ok, well interesting, the libvirt fail is on a different test where the guests are irrelevant | |
| 18:28:05 | mriedem | but if libvirt fails and disables the only compute service in a single node run... | |
| 18:28:11 | mriedem | it's going to break any test that needs to create a vm right? | |
| 18:28:17 | sdague | it recovers | |
| 18:28:23 | mriedem | yeah eventually | |
| 18:28:29 | sdague | well, actually pretty quick | |
| 18:29:29 | sdague | I think we're building new guests within 12 seconds | |
| 18:29:42 | sdague | only a single tempest test failed in this case | |
| 18:32:26 | mriedem | johnthetubaguy: should we really not expect hostnames on compute services to be unique across cells? | |
| 18:34:52 | johnthetubaguy | mriedem: seems like a bad assumption, given its not the FQDN, its just the hostname, but I might just be paranoid | |
| 18:36:40 | johnthetubaguy | mriedem: given we have a rabbit bus per cell, there is no restriction on that, so far, I might be overthinking that | |
| 18:36:53 | johnthetubaguy | mriedem: I just figured if we started over, all those APIs would just use uuids | |
| 18:36:55 | mriedem | i figured the hostname was fqdn | |
| 18:37:20 | johnthetubaguy | I thought it was conf.host, which isn't usually fqdn | |
| 18:37:21 | mriedem | johnthetubaguy: yeah i don't disagree with starting over, see my latest reply https://review.openstack.org/#/c/447149/1/specs/pike/approved/service-hyper-uuid-in-api.rst@148 | |
| 18:37:51 | mriedem | it is CONF.host | |
| 18:38:28 | mriedem | Hostname, FQDN or IP address of this host. Must be valid within AMQP key. | |
| 18:38:43 | mriedem | so i guess it doesn't have to be fqdn | |
| 18:39:00 | mriedem | sdague: you might have input on ^ | |
| 18:39:14 | mriedem | i don't know why the services actions apis are different from the server actions apis in this regard | |
| 18:39:39 | mriedem | and why they are PUTs rather than POSTs | |
| 18:39:56 | johnthetubaguy | mriedem: yeah, it does all seem a bit crufty in there | |
| 18:40:37 | johnthetubaguy | mriedem: I quite like the idea of adding the uuid in the URL in the new micro-version, to make it more normal | |
| 18:44:05 | mriedem | i can throw it in the weekly api subteam meeting for discussions | |
| 18:46:59 | sdague | mriedem: I'm confused, you pointed at https://github.com/openstack/nova/blob/6f66119153a36ab2ee7d63c98418af3a56fd1f1e/nova/compute/manager.py#L6812 for that logging issue | |
| 18:47:06 | sdague | except, that line is printed fine in the logs | |
| 18:47:31 | mriedem | sdague: it's blowing up trying to process the sub vars for the message | |
| 18:47:53 | mriedem | i'm assuming because event is None | |
| 18:48:02 | sdague | well, are you sure? | |
| 18:48:11 | mriedem | although it shouldn't be | |
| 18:48:17 | sdague | Received unexpected event network-vif-unplugged-8ed92137-a81a-4afd-a391-854848cd2916 for instance | |
| 18:48:18 | mriedem | because if it was https://github.com/openstack/nova/blob/6f66119153a36ab2ee7d63c98418af3a56fd1f1e/nova/compute/manager.py#L6867 | |
| 18:48:35 | mriedem | ah yeah | |
| 18:48:37 | mriedem | my mistake | |
| 18:48:45 | mriedem | when i originally looked at it, i thought it was using _event | |
| 18:50:01 | mriedem | https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/processutils.py#L355 ? | |
| 18:50:03 | sdague | and there are tons of these other places | |
| 18:50:12 | mriedem | https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/processutils.py#L385 | |