Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-29
19:59:57 mriedem *Table
20:00:08 mriedem i see you're trying to match the nova table to glance https://github.com/openstack/glance/blob/master/glance/db/sqlalchemy/models.py#L159
20:00:12 mriedem correct ^ ?
20:02:35 mriedem an alternative solution would be to add an image_props column to the instance_extra table and just store the serialized image properties in there, rather than the instance_system_metadata table
20:02:55 mriedem but then we could have n * TEXT entries in that json blob
20:03:39 mriedem jaypipes: TEXT is not preallocated right?
20:04:40 ameeda mriedem: I am new on openstack , I just follow the bug to fix it, I need to merge this bug. I worked on it a lot as you see :(
20:04:54 jaypipes mriedem: correct.
20:05:24 mriedem ameeda: do you have a customer hitting htis?
20:05:25 mriedem *this
20:06:54 jaypipes ameeda: reviewed.
20:07:37 ameeda I am working at local company here, and the customer ask us to fix bugs in openstack. so I have to get points
20:07:48 ameeda jaypipes: Thank you
20:08:42 mriedem ameeda: hmm, well, there are probably less controversial bugs to fix :)
20:08:48 mriedem if you're just looking for something to get into stackalytics
20:10:19 mriedem reading the history on this bug, from lbragstad - probably due to some issue with storing powervc configuration strategy xml in image metadata back in 2013 - it sounds like the 255 length restriction was from when the image service was passing properties as http headers
20:10:35 ameeda mriedem: I still learn about nova and try to find medium bugs to fix . if my current opened bugs not merged , they will fire me :(
20:10:48 mriedem ameeda: sounds like a terrible employer
20:11:53 ameeda mriedem: yes it was sending by header and now sending on body, so the limitation is gone
20:11:59 ameeda mriedem: no comments ...
20:12:46 openstackgerrit melanie witt proposed openstack/nova stable/ocata: Stop globally caching host states in scheduler HostManager https://review.openstack.org/539013
20:21:21 ameeda mriedem: regarding to your last comment, is this something should I do ?
20:22:16 mriedem ameeda: up to you, it could be done in devstack if you use the nova fake virt driver ,and then just create 1000 instances
20:22:35 mriedem then apply your change and run the migration and see how long it takes
20:23:56 ameeda sure that will take long time for large data, but what I should I do after what I have done until now :(
20:27:31 openstackgerrit Ameed Ashour proposed openstack/nova master: change instance_system_metadata column type https://review.openstack.org/526900
20:28:45 ameeda jaypipes: I uploaded the patch set
20:34:10 ameeda jaypipes mriedem, kindly, can you please review those also https://review.openstack.org/#/c/528069/ and https://review.openstack.org/#/c/528385/
20:35:34 ameeda and please let me know what do you think about this bug "https://bugs.launchpad.net/nova/+bug/1737708"
20:35:37 openstack Launchpad bug 1737708 in OpenStack Compute (nova) "create instance failed when the userdata size is larger than 64k" [Undecided,New] - Assigned to Deepak Mourya (mourya007)
20:36:00 ameeda sorry for inconvenience, Thank you for your time
20:39:18 mriedem ameeda: melwitt has been working on a duplicate of one of those for a long time https://review.openstack.org/#/c/340614/
20:41:09 melwitt yeah, took many iterations to get it right back when the earlier versions caused gate failures in the postgres job. looks like it needs another rebase
20:43:09 melwitt we still have customers that need a fix for it
20:43:13 ameeda mriedem: yes, but my bug scenario doesn't hit _local_delete, so we have to cover all the cases, right ?
20:44:04 mriedem melwitt's patch is fixing a local delete case
20:44:08 mriedem where the instance isn't on a compute host,
20:44:19 mriedem so it is either shelved offloaded or failed during scheduling
20:44:24 melwitt it is on a compute host, it's just in ERROR state
20:44:29 mriedem 'local delete' == delete from the API, not compute
20:45:07 melwitt there's code in compute manager that sets host = None while setting to ERROR state if something fails. IIRC
20:46:53 melwitt like this https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1941-L1943
20:48:54 ameeda I am not sure about the case , but what about this fix https://review.openstack.org/#/c/528385/
20:49:18 ameeda this detach volume when instance creation fails
20:51:22 mriedem ameeda: -1 on that one too
20:51:32 mriedem ameeda: we already attempt to detach volumes when deleting a server
20:51:41 mriedem so if that's failing, i don't think your patch is going to fix it
20:53:41 ameeda what is the case !, I just follow the bugs to fix them , after that I found that the bug is invalid ?
20:54:00 mriedem ameeda: well, you could be trying to fix a very old bug that's already been fixed
20:54:10 mriedem or the recreate scenario from the bug is not clear
20:55:44 mriedem so the thing to do here, is probably go through the recreate steps in the bug (which are pretty clear) and see if you can reproduce it
20:55:47 mriedem and if so, see where things are failing
20:56:24 mriedem the bug report was written over a year ago and doesn't say which version of nova they were using when they hit this,
20:56:26 mriedem so it might be fixed
20:56:37 mriedem i'm sorry that you invested time in a patch, but we need to make sure it's still a valid bug first
20:56:43 ameeda mriedem: but I reproduce the bug and fix it , then test it again to make sure that I fix the issue, and there are reviewers ask me to make changes on it :(
20:57:21 ameeda I am sure that the bug is still valid , I reproduce it
20:58:16 mriedem ok, so then why is the original detach failing in _shutdown_instance?
20:58:22 mriedem because that's what should be detaching the volume
21:02:15 melwitt mriedem: I think it's the same bug I'm fixing. I think he means volume quota was exceeded (it's checked in compute). compute catches the exceptions, nulls out the instance.host, then sets it to ERROR state
21:03:00 melwitt then when you go to delete the instance, because compute nulled out the host, the compute api thinks there's no resources to free up because it assumes instance.host = None means failed to schedule
21:03:44 melwitt so I think my patch will take care of it. I'm rebasing it now and rewriting the commit message to be clearer
21:03:46 ameeda my case when vm creations fails when there are no enough quota for volumes. as this https://review.openstack.org/#/c/528069/ scenario.
21:03:53 melwitt I know, that's what I just said
21:05:12 ameeda melwitt: you fix solve the issue when there is no hosts, my fix solve the issue in another case !
21:05:29 ameeda mriedem: what do you think ?
21:05:56 melwitt it's not another case. compute will set instance.host = None because of the volume quota exceeded
21:07:28 ameeda ok great !!
21:08:59 ameeda duplicated bugs , and my work and time have gone in vain
21:29:39 efried mriedem: Any tips for how I can check in nova.tests.functional.test_servers that a particular exception got raised up from _update() ?
21:30:34 efried So far my mechanism for getting _update() to run is self._run_periodics()
21:37:09 melwitt efried: I think that's the usual way to run it
21:38:00 efried melwitt: I think I'm going to have to deconstruct it a bit and run rt.update_available_resource directly.
21:38:52 melwitt efried: any reason you're trying to verify that in a func test instead of a unit test?
21:39:17 efried melwitt: A reasonable question.
21:39:26 melwitt if there's no way to detect what you expect from the instance or other things you can inspect in the func test
21:39:29 efried melwitt: I was hoping to avoid unit test altogether.
21:39:56 melwitt so there's an existing func test and you want to add an assertion to it?
21:40:36 efried melwitt: I'm writing a new func suite. Stand by, I'll put up what I've got.
21:43:04 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Use update_provider_tree from resource tracker https://review.openstack.org/520246
21:43:04 openstackgerrit Eric Fried proposed openstack/nova master: SchedulerReportClient.update_from_provider_tree https://review.openstack.org/533821
21:43:04 openstackgerrit Eric Fried proposed openstack/nova master: New-style _set_inventory_for_provider https://review.openstack.org/537648
21:43:05 openstackgerrit Eric Fried proposed openstack/nova master: Fix nits in update_provider_tree series https://review.openstack.org/531260
21:43:06 openstackgerrit Eric Fried proposed openstack/nova master: Move refresh time from report client to prov tree https://review.openstack.org/535517
21:44:06 efried melwitt: Right here is where I want to make sure that exception happened: https://review.openstack.org/#/c/520246/41/nova/tests/functional/test_servers.py@1615
21:44:20 efried melwitt: And similar for other kinds of error conditions.
21:44:45 efried melwitt: I mean, I'm at least proving the whole thing doesn't blow up when I inject error conditions, which is good.
21:49:08 melwitt I see. I suppose you could monkey patch _update (or whatever) to call the real _update inside self.assertRaises. I wonder if that would work
21:51:33 efried melwitt: Well, I don't see _run_periodicals really doing anything that adds value to my narrow little test case.
21:52:31 melwitt so you don't want to assert a particular exception is raised? I'm confused :)
21:53:15 efried I do, and the only reason I can't do assertRaises(self._run_periodicals) is because _run_periodicals invokes x, which invokes y, which invokes z, which swallows exceptions.
21:53:24 efried So I'm going to dig in and invoke z directly, so I can assertRaises(z)
21:53:44 efried ...which I just did, and it seems to work a treat! melwitt thanks for sounding boarding :)
21:54:06 efried Now everyone will want to do what I just did
21:54:09 melwitt yeah, that's what I was trying to say, monkey patch the method that will raise it and just call the real method inside self.assertRaises
21:56:31 efried The monkey patching isn't needed
21:56:52 melwitt oh, so run that instead of periodics
21:56:55 melwitt I see
21:56:58 efried just so.
21:57:17 efried Just needed a few extra LOC to dig into the call stack a couple more levels.

Earlier   Later