Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-20
17:18:43 mriedem yup
17:19:38 EmilienM or even change something in the commit message to actually kick off jobs (jobs are still running now)
17:20:09 mriedem when i push the dep, zuul will kill those running jobs
17:21:27 EmilienM ah indeed
17:22:08 edmondsw johnthetubaguy I replied to your comments in https://review.openstack.org/#/c/389314/
17:24:07 johnthetubaguy edmondsw: so I think we need a deprecated default on oslo.policy for all this kind of stuff
17:24:17 johnthetubaguy s/on/in/
17:24:42 edmondsw johnthetubaguy for most things I agree, but I don't think you can in this case
17:25:38 openstackgerrit Sujitha proposed openstack/nova master: Remove unused policy rule in admin_actions.py https://review.openstack.org/447611
17:25:45 openstackgerrit Matt Riedemann proposed openstack/nova master: DNM: Log placement result in scheduler https://review.openstack.org/447520
17:26:39 edmondsw johnthetubaguy I gave 4 reasons that doesn't really make sense here in my reply to one of your comments
17:26:47 edmondsw patch set 5
17:27:08 johnthetubaguy edmondsw: the problem is how do we keep our upgrade promise, that the old policy file works perfectly with the new release
17:28:03 openstackgerrit Matt Riedemann proposed openstack/nova master: DNM: Log placement result in scheduler https://review.openstack.org/447520
17:28:09 edmondsw johnthetubaguy you shouldn't be promising that... 99.999% of the time, sure, but there will always be a 0.001% case where...
17:28:41 edmondsw johnthetubaguy with this change, there is a 0.000001% chance that someone would break on upgrade... or something extremely slim like that
17:29:08 johnthetubaguy edmondsw: there are always exceptions, but that is our promise in here: https://governance.openstack.org/tc/reference/tags/assert_supports-upgrade.html#requirements
17:29:37 asettle mriedem cdent - we finally *might* have an answer on the doc patch: https://review.openstack.org/#/c/438328/
17:29:51 asettle Petr has been looking into it.
17:29:55 johnthetubaguy edmondsw: well the additional bit of info I guess is I could policy as configuration
17:29:58 asettle We probably need someone from the nova team to take a look at this too.
17:30:02 edmondsw johnthetubaguy this patch fits fine with that tag
17:30:14 cdent asettle: the Directory thing? hasn't that been mentioned a few times before?
17:30:18 edmondsw it has an upgrade note for operators to consider in the very unlikely event that they need to consider this
17:30:26 cdent asettle: or maybe it was only in related email
17:30:45 asettle cdent: I didn't see it in the comments. Perhaps I am forgetting - if so, sorry! Maybe on email.
17:30:56 johnthetubaguy edmondsw: it violates bullet point 2, now that might be best thing to do, I just want to make sure thats the case
17:31:00 asettle cdent: did we document it?
17:31:48 cdent asettle: sorry, not fully contextualized at the moment, but I remember discussing it in a few different places, including asserting that one person's solution to grant all on '/' was too broad
17:32:07 edmondsw johnthetubaguy that depends on how you read point 2
17:32:45 asettle cdent: interesting. I've mentioned it to Lana and Brian, since it's nearing the end of my day rapidly. I'll ensure I check in with them again in the morning. If by chance you see it again, please let us know :)
17:33:03 cdent asettle: yeah, will look around some more once I'm done with the current thing
17:33:21 asettle Thank you cdent :)
17:33:23 edmondsw you have to consider policy as config (which I do, but not everyone does) and you have to assume that it means even really unlikely edge cases that nobody would do must still work... even if you've covered that under bullet 5
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 mriedem sdague: you want to fix it? https://bugs.launchpad.net/nova/+bug/1674449
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: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

Earlier   Later