Earlier  
Posted Nick Remark
#openstack-nova - 2018-01-29
19:07:18 openstackgerrit Matt Riedemann proposed openstack/nova master: Rollback instance.image_ref on failed rebuild https://review.openstack.org/538961
19:07:19 openstackgerrit Matt Riedemann proposed openstack/nova master: Collapse duplicate error handling in rebuild_instance https://review.openstack.org/539001
19:11:53 jaypipes mriedem: +Wallaby'd
19:12:04 mriedem thanks
19:14:50 efried jaypipes: Can we continue the discussion as to whether or not we should in fact be requiring update_provider_tree to return True/False?
19:18:26 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Rollback instance.image_ref on failed rebuild https://review.openstack.org/539003
19:18:29 cfriesen lbragstad: just back from lunch. yes, there's a scenario where we would like to answer the question "did this request come from another openstack service, not a 'normal' user". was hoping to use service tokens for this.
19:20:40 jaypipes efried: sure
19:21:58 efried jaypipes: So first off, it's trivial and inexpensive for report client to figure it out, so there's no real *need* for virt to tell us.
19:22:35 efried jaypipes: And after talking through a couple of potential impls from VMWare, it became clear that there's certainly the possibility that it would be awkward for the virt driver to figure it out.
19:23:11 efried jaypipes: For example, one viable implementation is to say, "I don't care what you gave me, I'm going to delete everything and build the ProviderTree as I know it from scratch"
19:23:34 lbragstad cfriesen: that kinda sounds like federation?
19:23:36 openstackgerrit melanie witt proposed openstack/nova stable/pike: Stop globally caching host states in scheduler HostManager https://review.openstack.org/539005
19:24:39 efried jaypipes: Softer than that, it's IMO a source of extra unnecessary bugs to ask the virt driver to make sure they get that bool return correct.
19:25:10 cfriesen lbragstad: not sure federation applies since it's all within the same cloud. We just want glance to be able to special-case a request to change an image location if it comes from nova, but not if it comes from a regular user.
19:25:31 ameeda gibi: now its okay, can you please merge the bug ? or it need something else ?
19:25:52 efried jaypipes: Put another way: why have two chunks of code doing the same thing when one will do?
19:26:31 jaypipes efried: ok
19:27:04 jaypipes efried: I just thought it would make the RT's life easier if it could say "ok, no changes from virt driver... just move on"
19:27:12 lbragstad cfriesen: oh - sorry, for some reason i was thinking of different services
19:27:31 efried jaypipes: Definitely could have worked out that way.
19:29:26 lbragstad cfriesen: that sounds like new territory for service tokens, the first thing we started working on with them was the whole long running operation issue.. it'd be good to sync with jamielennox though
19:30:10 lbragstad cfriesen: he was one of the original people driving the effort, so i wouldn't be surprised if he's ventured down a couple different paths similar to what you're describing
19:31:53 cfriesen lbragstad: move it over to the keystone channel?
19:31:58 lbragstad cfriesen: sure
19:36:38 openstackgerrit Matt Riedemann proposed openstack/nova stable/ocata: Rollback instance.image_ref on failed rebuild https://review.openstack.org/539008
19:51:41 ameeda is this error caused by me ? "http://logs.openstack.org/00/526900/26/check/openstack-tox-functional/c777a93/testr_results.html.gz"
19:51:47 ameeda from this gerrit "https://review.openstack.org/#/c/526900/"
19:51:57 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove redundant call to add_instance_fault_from_exc in rebuild_instance https://review.openstack.org/539011
19:52:19 ameeda jaypipes: please check this for me when you available https://review.openstack.org/#/c/526900/
19:56:55 mriedem1 ameeda: instance_system_metadata is a table where one row is a key/value pair for a single instance, and we can have a lot of sysmeta per instance, and a ton of instances,
19:57:09 mriedem1 in what world do we have a system metadata value that needs to be length TEXT?
19:58:56 ameeda mriedem: so I did something wrong ?
19:59:56 mriedem no, it's just, this is potentially a very large change in storage for that talbe
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 ?

Earlier   Later