| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-16 | |||
| 18:07:34 | esberglu | What am I missing | |
| 18:07:48 | mriedem | @abc.abstractproperty | |
| 18:07:55 | mriedem | https://docs.python.org/2/library/abc.html#abc.abstractproperty | |
| 18:07:56 | mriedem | i think anyway | |
| 18:08:51 | esberglu | mriedem: I guess I misunderstood add_metaclass. New patch incoming | |
| 18:09:08 | mriedem | yeah unit tests and pep8 failed in https://review.openstack.org/#/c/549053/ | |
| 18:14:05 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: DiskAdapter parent class https://review.openstack.org/549053 | |
| 18:14:06 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: Localdisk https://review.openstack.org/549300 | |
| 18:23:37 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Update nova-status and docs for required placement 1.24 https://review.openstack.org/568923 | |
| 18:25:04 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Update nova-status and docs for required placement 1.24 https://review.openstack.org/568923 | |
| 18:31:06 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Update placement upgrade docs for nova-api dependency on placement https://review.openstack.org/568925 | |
| 18:33:05 | mriedem | esberglu: hmm http://logs.openstack.org/53/549053/19/check/openstack-tox-py27/c882f7f/testr_results.html.gz | |
| 18:33:47 | esberglu | mriedem: Guess it's time to figure out why I can't run tox locally | |
| 18:34:41 | mriedem | self.st_adpt = disk_dvr.DiskAdapter(None, None) | |
| 18:34:46 | mriedem | your test is trying to instantiate the abc | |
| 18:36:28 | mriedem | so either just leave that in the test for SSPDiskAdapter, | |
| 18:36:45 | mriedem | or make a little fake DiskAdapter subclass for your generic base class tests? | |
| 18:54:54 | openstackgerrit | Merged openstack/nova master: Debug logs for allocation_candidates filters https://review.openstack.org/568712 | |
| 19:00:24 | dansmith | mriedem: melwitt: cells meeting? I've got nothing and tssurya isn't around | |
| 19:00:35 | melwitt | I don't have anything either | |
| 19:01:22 | mriedem | nope | |
| 19:01:25 | dansmith | woot | |
| 19:14:43 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Add additional information for z/VM spec. https://review.openstack.org/562154 | |
| 19:15:28 | mriedem1 | dansmith: +2 on ^ - barring some english / grammar stuff in there, it sounds like that reflects the ML thread which i think you know more about than i do | |
| 19:39:34 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: DiskAdapter parent class https://review.openstack.org/549053 | |
| 19:39:35 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: Localdisk https://review.openstack.org/549300 | |
| 19:44:29 | mriedem | can we remove the 'build_instance' hook yet? https://github.com/openstack/nova/blob/7bdb7dbbddf9fcb4284d490bf315d6756f4015e7/nova/compute/manager.py#L2209 | |
| 19:44:41 | mriedem | hooks have been officially deprecated for a couple of years now | |
| 19:45:31 | mriedem | "will be removed in a future release")) | |
| 19:45:31 | mriedem | LOG.warning(_LW("Hooks are deprecated as of Nova 13.0 and " | |
| 19:46:23 | mriedem | the issue being, because _do_build_and_run_instance has the hook, we return the build_results enum, which means when we are checking to see if the compute should be auto-disabled due to consecutive failures, we don't have any context on why the build failed https://github.com/openstack/nova/blob/7bdb7dbbddf9fcb4284d490bf315d6756f4015e7/nova/compute/manager.py#L1679 | |
| 19:46:26 | mriedem | dansmith: ^ | |
| 19:47:12 | mriedem | if we get a BuildAbortException from the bowels https://github.com/openstack/nova/blob/7bdb7dbbddf9fcb4284d490bf315d6756f4015e7/nova/compute/manager.py#L1903 we just return the 'failed' enum and consider everything fatal / misconfig even if it's not | |
| 19:48:01 | mriedem | 13.0 is mitaka | |
| 19:48:16 | dansmith | I'm missing what the linkage with the hook is | |
| 19:48:21 | dansmith | (but of course I'm fine with removing) | |
| 19:49:59 | dansmith | mriedem: ^ | |
| 19:51:08 | mriedem | https://bugs.launchpad.net/nova/+bug/1742102 | |
| 19:51:09 | openstack | Launchpad bug 1742102 in OpenStack Compute (nova) "Simple user can disable compute" [High,Confirmed] - Assigned to jichenjc (jichenjc) | |
| 19:51:15 | mriedem | see the 2nd to last comment there | |
| 19:52:39 | mriedem | trying to determine context on what happens when we call _build_failed() | |
| 19:52:43 | dansmith | mriedem: sorry I | |
| 19:52:50 | dansmith | am totally missing what hooks has to do with anything | |
| 19:52:55 | mriedem | if it's volume overquota, we don't need to count that against consecutive build failures | |
| 19:53:16 | dansmith | I get that | |
| 19:53:20 | mriedem | this is where we call _build_failed() to see if we disable the service https://github.com/openstack/nova/blob/7bdb7dbbddf9fcb4284d490bf315d6756f4015e7/nova/compute/manager.py#L2209 | |
| 19:53:32 | mriedem | at that point, we have 0 context on the actual exception that occurred | |
| 19:53:51 | mriedem | so if i wanted to inspect the BuildAbortException let's say to see if it was a volume overquota issue, i can't do that | |
| 19:54:12 | dansmith | what does that have to do with hooks? | |
| 19:54:17 | mriedem | i can't do that because, except for one base64 decode error exception, _do_build_and_run_instance doesn't raise exceptions | |
| 19:54:31 | mriedem | _do_build_and_run_instance only returns build_results.* enums which are there for hooks | |
| 19:54:36 | mriedem | https://github.com/openstack/nova/blob/7bdb7dbbddf9fcb4284d490bf315d6756f4015e7/nova/compute/manager.py#L2209 | |
| 19:55:10 | mriedem | at least that's my understanding, | |
| 19:55:22 | mriedem | so what i want is for _do_build_and_run_instance to raise it's actual failure, | |
| 19:55:32 | mriedem | so we can inspect it up the stack in _build_failed() | |
| 19:55:40 | dansmith | you know you're not actually linking me to _do_build_and_run_instance right? | |
| 19:55:41 | mriedem | which means breaking the "return build_results.FAILED" contract in _do_build_and_run_instance which is there for hooks | |
| 19:55:59 | mriedem | sorry, https://github.com/openstack/nova/blob/7bdb7dbbddf9fcb4284d490bf315d6756f4015e7/nova/compute/manager.py#L1788 | |
| 19:56:23 | mriedem | this is where we call _build_failed() https://github.com/openstack/nova/blob/7bdb7dbbddf9fcb4284d490bf315d6756f4015e7/nova/compute/manager.py#L1752 | |
| 19:57:29 | dansmith | see, I never connected hooks with those build result values | |
| 19:58:04 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: DiskAdapter parent class https://review.openstack.org/549053 | |
| 19:58:05 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: Localdisk https://review.openstack.org/549300 | |
| 19:58:32 | dansmith | mriedem: what makes you think that those are there for the hooks? git history? | |
| 19:58:42 | dansmith | that said, I don't care at all about breaking that contract as you know, nor removing it | |
| 19:58:43 | dansmith | however, | |
| 19:58:50 | mriedem | i think that's what alaski told me they were for years ago when i asked | |
| 19:58:59 | dansmith | I'm not sure that unwinding all of that is a huge benefit, | |
| 19:59:14 | dansmith | and I'd say we just need another build result that is, like you said "failed, but not my fault" | |
| 19:59:35 | dansmith | that's kindof a large change to alter all that exception handling | |
| 19:59:45 | dansmith | not that it's great, but I just wouldn't want to go refactoring it myself | |
| 20:00:19 | dansmith | https://review.openstack.org/#/c/106482/ | |
| 20:00:29 | dansmith | so that's why I guess, but I had never made that connection | |
| 20:01:31 | dansmith | that is a lot more recent than I would have thought | |
| 20:01:54 | mriedem | yeah, kilo | |
| 20:01:57 | mriedem | all the andrews were in on that one | |
| 20:02:27 | mriedem | ok so i'll hack something nasty up quick and see what it looks like using the existing build_results gorp with a new enum | |
| 20:02:32 | dansmith | that was basically 2015, mitaka was early 2016 | |
| 20:05:32 | mriedem | gerrit says it's in kilo-eol | |
| 20:06:12 | dansmith | I know, it was deprecated in mitaka right? | |
| 20:13:51 | mriedem | hooks in general were deprecated in mitaka | |
| 20:14:19 | mriedem | https://github.com/openstack/nova/commit/7be56442703d071c9256267abb8acfabae642a1a | |
| 20:14:25 | mriedem | via mr dague | |
| 20:14:35 | mriedem | when he was on the "burn all extension points with fire" crusade | |
| 20:16:07 | dansmith | yeah | |
| 20:21:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Don't count volume over quota when disabling compute https://review.openstack.org/568953 | |
| 20:21:10 | mriedem | dansmith: ok here is the hack a mole WIP ^ | |
| 20:24:50 | dansmith | commented | |
| 20:25:10 | dansmith | there are other things, like failing to verify an image signature | |
| 20:25:18 | dansmith | which probably shouldn't count | |
| 20:25:28 | dansmith | since you could probably make that happen externally | |
| 20:26:15 | mriedem | oh there are a buttload of things | |
| 20:27:07 | mriedem | there was one i saw earlier about hitting a compute with device tags but the virt driver doesn't support devices tags (so anything besides libvirt, hyperv and xen) | |
| 20:27:20 | dansmith | hrm yeah | |
| 20:27:26 | mriedem | multiattach is in the same boat | |
| 20:27:41 | mriedem | that's why we added a policy rule on the cinder side to disable being able to make those types of volumes | |
| 20:27:53 | mriedem | if you're purely non-libvirt | |
| 20:28:22 | mriedem | or hell, if you're not using any of the supported volume types for multiattach, which in queens was only 3 out of like the 400 available cinder backends | |
| 20:34:07 | mriedem | mnaser: how much do you hate this? https://review.openstack.org/#/c/568953/ - realizing we should still be doing a volume quota check in conductor, but that could still pass during a race | |
| 20:34:36 | mriedem | dansmith: you didn't pick up on the name of my villian in the commit message | |
| 20:35:23 | dansmith | mriedem: heh, didn't even read the commit message | |