Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-04-27
19:46:09 melwitt abhi89: okay, I'm curious why the bug calls out getting the message of "Failure prepping block device" then. because the change doesn't seem related to that
19:46:15 abhi89 there is more to it..
19:46:57 abhi89 "VolumeLimitExceeded: Maximum number of volumes allowed (5) exceeded for quota 'volumes'." this message will be generated in case of "volumes" quota exceed
19:47:10 abhi89 number there is just an example
19:48:15 abhi89 "VolumeSizeExceedsLimit: Requested volume size 5G is larger than maximum allowed limit 4G." message will be there when 'per_volume_gigabyte' quota is exceeded
19:48:53 abhi89 "VolumeSizeExceedsAvailableQuota: Requested volume or snapshot exceeds allowed gigabytes quota. Requested 5G, quota is 25G and 21G has been consumed." message will be generated when 'gigabyte' quota exceeds
19:49:48 melwitt abhi89: okay, so what you are doing is letting the original overquota exception bubble up which will contain the correct message
19:50:40 abhi89 so these are the 3 specific messages which come as part of OverLimit exception. This OverLimit exception get translated to OverQuota exception. After translation, I embed the appropriate message from OverLImit to OverQuota, so that we have the exact reason why the VM deploy failed
19:50:52 abhi89 original OverLimit message
19:52:06 melwitt abhi89: okay. so I think the main thing your patch needs is a bit of test coverage that shows those scenarios and verifies we get the message we expect in each case. similar to the regression test that I think should be kept
19:52:39 abhi89 I removed the regression test because it was very specific to just 'VolumeLimitExceeded' for all 3 cases
19:55:06 abhi89 we already have a test case to check whehter we have OverQuota exception or not.. for all 3 specific cases, as we have replaced the OverQuota message to OverLimit message, we wont be knowing which of the 3 quota exceeded
20:00:34 mriedem dansmith: jaypipes: comments in https://review.openstack.org/#/c/457782 - overall i don't have anything to block it, but i don't like the tight coupling and assumptions being made in the virt driver about what the RT is going to fil lin for it
20:00:37 mriedem *fill
20:03:35 abhi89 melwitt: was i able to put I point clearly! can you please suggest what can I improve upon if you are not satisfied with this change..
20:04:53 melwitt abhi89: yes, was looking at it some more. basically, I want to see test coverage that verifies that we preserve and pass through the error message from cinder. because that's what we're relying on for the correct message. I don't think I see coverage of that yet unless I've missed something
20:06:00 melwitt abhi89: I had been thinking you could adapt the regression test (instead of removing it) to put in an example message from cinder in the OverLimit exception, then assert the message is as expected in our response text
20:06:15 cdent macsz: :(
20:08:32 abhi89 melwitt: we can put have one of the example messages to assert with.. but we should know which of the 3 occured i.e., which of the 3 have we replaced OverQuota's message with OverLimit's. That info is not with us.
20:10:48 melwitt abhi89: what do you mean? if we use an example message of "VolumeSizeExceedsAvailableQuota: Requested volume or snapshot exceeds allowed gigabytes quota. Requested 5G, quota is 25G and 21G has been consumed." and we assertIn('VolumeSizeExceedsAvailableQuota', e.response.text) that will simulate it, no?
20:11:03 macsz cdent: and i was just getting comfortable with nova
20:11:51 edleafe macsz: that was quick - I'm still not comfortable :)
20:12:04 cdent yeah, me neither. very impressive
20:12:06 mriedem edleafe: try a larger pant size
20:12:07 cdent :)
20:12:11 mriedem edleafe: you and your skinny jeans
20:12:15 mriedem always the fashionista
20:12:21 cdent mordred has some writing pants that he sometimes shares
20:12:39 edleafe mriedem: Not fair. I was into hating on hipsters before it was cool
20:12:50 abhi89 melwitt: we can assertIn('VolumeSizeExceedsAvailableQuota', e.response.text).. but how will we know which exception message is coming in?
20:12:55 mriedem i for one wear my fat pants as often as possible now that i'm working from home
20:13:00 mordred efried: yah - I believe that's spot-on. I also poked morgan to look at it as well
20:13:10 edleafe mriedem: you wfh and you still wear pants?
20:13:13 mordred edleafe: sorry- I wrote that a few hours ago - I just forgot to hit enter
20:13:18 abhi89 in the previous case we could, as it was always the VolumeLimitExceeded coming in
20:13:19 mriedem edleafe: i'm still learning
20:13:28 mordred cdent: it's important to note that it's a single shared pair of pants
20:13:32 smcginnis edleafe: The question is whether he's showered or not.
20:13:37 mriedem oh i shower
20:13:44 cdent mordred: that may explain my current discomfort
20:13:47 abhi89 melwitt: in the previous case we could, as it was always the VolumeLimitExceeded coming in
20:13:54 cdent mriedem: have you stopped shaving yet?
20:13:54 edleafe mriedem showers every month whether he needs it or not
20:13:55 macsz edleafe: i was just getting there, which would then require getting there a little further and further :D
20:14:01 mriedem cdent: i actually shave more surprisingly
20:14:06 mriedem b/c i have more time in the morning
20:14:57 cdent ah the halcyon days of early work-at-home: "time in the morning"
20:15:04 melwitt abhi89: in the test if you provide an example message when constructing the OverLimit exception, to simulate the cinder client raising that, then you can assert it in our response text
20:19:23 abhi89 melwitt: when constructing the OverQuota exception(copying OverLimit exception's message to OverQuota exception), i should do what! I didnot understand
20:19:45 cdent 'night all
20:21:55 melwitt abhi89: in the test "test_over_limit_volumes" in nova/tests/functional/regressions/test_bug_1554631.py you could provide an example exception message when constructing the cinder_exceptions.OverLimit() object. and then you assert part of the message in the response text. that would verify that we're passing through the error message from cinder. does that make sense?
20:27:06 bauzas mriedem: dansmith: others, have you seen https://review.openstack.org/#/c/460455/1 ?
20:27:53 dansmith bauzas: hadn't yet but cool
20:28:04 efried mordred Thanks for looking. Will proceed with UT.
20:28:22 efried mordred Having pored over os-client-config code a bit, I think I may be ready to start asking leading questions, whenever you have a mo.
20:28:26 bauzas dansmith: mriedem: the main problem with the scheduler is that we don't have the instance UUIDs there
20:29:04 mordred efried: awesome - I'm on (very fun) phone call at the moment- hopefully off soon
20:29:15 abhi89 melwitt: so the example exception message when constructing cinder_exceptions.OverLimit() should have common text from all 3 types of exception messages, so that we can check with assertIn whether this example message is part of all 3 exception messages.. this is what you meant?
20:29:36 erhudy hdecejhbhdednddujvregvebfhcjhjngbrjhrekfikgf
20:29:39 erhudy oops!
20:29:54 erhudy treacherously betrayed by a hairtrigger yubikey
20:31:17 mriedem bauzas: we don't? i thought the host manager pulled the instance uuids per host on startup?
20:31:18 melwitt abhi89: no, I was suggesting we can just pick one specific one and use it as the example in the test. because what we really want to know is if we are preserving the message from cinder and propagating that to the end user
20:31:22 mriedem via update_instance_info?
20:31:26 mriedem which is configurable though
20:32:00 bauzas mriedem: because we only provide the first instance into the RequestSpec object to the scheduler
20:32:56 openstackgerrit OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/460515
20:34:29 abhi89 melwitt: but how can we pick one specific message & test it against the 3 exception messages.. the UT will fail in the other 2 cases.. i agree with you that we need to know whether we are preservinfg the message from cinder or not..
20:36:01 melwitt abhi89: we pick one specific message and test against the one specific message. I think that would be good enough. earlier I mentioned all scenarios but since we've been talking about it, the main idea is verifying we preserve the message from cinder, and we can use just one specific example to do that
20:37:07 mriedem dansmith: comments in https://review.openstack.org/#/c/460616/
20:42:29 openstackgerrit Chris Friesen proposed openstack/nova master: show flavor info in server details https://review.openstack.org/434514
20:42:40 cfriesen ^ hopefully this passes all the tests now
20:46:41 mriedem sdague: if you can get https://review.openstack.org/#/c/457008/ at some point we can close out the 2.45 blueprint
20:46:52 sdague mriedem: looking
20:47:20 sdague mriedem: bam!
20:48:06 mriedem thanks
20:48:31 mriedem did you see the easter egg i put in there for you?
20:55:05 mriedem nova meeting in #openstack-meeting in 5 minutes
20:56:54 dansmith mriedem: I'm going to skip the nova meeting today.. feeling crappy and doing a few other things
20:57:05 dansmith mriedem: I think you can fill everyone in on the extensive discussions at the cells meeting, right?
20:57:12 mriedem god i hope so
20:58:14 abhi89 melwitt: I will trying doing what you have told. Thanks for your input..
21:00:35 openstackgerrit OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/460515
21:03:57 openstackgerrit Steve Noyes proposed openstack/nova master: Add Cinder v3 detach to shutdown_instance https://review.openstack.org/456877
21:56:34 mriedem hferenc: gibi: blasted the ML :)
22:43:40 mriedem mtreinish: some easy stuff to fix up in https://review.openstack.org/#/c/459460
23:15:32 Kevin_Zheng mriedem: hi, for https://review.openstack.org/#/c/459569 , the assertion fails now, I think it is not what we expected?
23:16:48 mtreinish mriedem: so on the pokicy bits, I tried to delete them before but a bunch of testing started complaining there was no policy for the endpoint
23:17:07 mtreinish s/pokicy/policy/
23:23:06 openstackgerrit Matthew Treinish proposed openstack/nova master: Remove nova-cert https://review.openstack.org/459460
23:23:46 mtreinish mriedem: ^^^
23:31:55 mriedem mtreinish: probably some policy test to see that all apis have policy registered,
23:32:09 mriedem but i'd think we could remove that for os-certificates as we don't actually use the policy
23:33:34 mriedem Kevin_Zheng: you mean this right? http://logs.openstack.org/69/459569/6/check/gate-nova-python27-ubuntu-xenial/3f13311/console.html#_2017-04-27_07_11_57_717635
23:35:19 mriedem Kevin_Zheng: maybe rather than assert equality, which might not be implemented for ObjectListBase.__eq__, assert the length is the same, is 1, and the single tag between both is the same
23:35:22 Kevin_Zheng yeah
23:35:23 mtreinish mriedem: ok, I'll look at that tomorrow, or later tonight
23:35:50 Kevin_Zheng mriedem: ok, got it
23:36:15 mriedem Kevin_Zheng: yeah it doesn't implement __eq__ https://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/base.py#L778
23:36:34 mriedem mtreinish: you must be getting soft in your old age

Earlier   Later