| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-29 | |||
| 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. | |
| 22:09:25 | openstackgerrit | Jay Pipes proposed openstack/nova master: tests for alloc candidates with nested and traits https://review.openstack.org/531899 | |
| 22:09:26 | openstackgerrit | Jay Pipes proposed openstack/nova master: _get_trees_matching_all_resources() sum children https://review.openstack.org/534866 | |
| 22:09:26 | openstackgerrit | Jay Pipes proposed openstack/nova master: add test for scenario with sum of child resources https://review.openstack.org/534339 | |
| 22:09:27 | openstackgerrit | Jay Pipes proposed openstack/nova master: validate provider summary structure https://review.openstack.org/536516 | |
| 22:09:27 | openstackgerrit | Jay Pipes proposed openstack/nova master: add _get_usages_from_trees() function https://review.openstack.org/534967 | |
| 22:09:28 | openstackgerrit | Jay Pipes proposed openstack/nova master: WIP - populate alloc cands for trees https://review.openstack.org/534968 | |
| 22:26:42 | openstackgerrit | Matt Riedemann proposed openstack/nova master: reno for notification-transformation-queens https://review.openstack.org/518018 | |
| 22:30:11 | mriedem | dansmith: want to hit this pike backport? https://review.openstack.org/#/c/536981/ | |
| 22:30:26 | mgagne | what's the use case for aggregates in Placement API? | |
| 22:32:12 | mriedem | mgagne: the primary one is modeling things where one provider shares a resource with another, like a shared storage pool | |
| 22:33:12 | mgagne | mriedem: thanks for the clarification, I tried to read/understand https://developer.openstack.org/api-ref/placement/#resource-provider-aggregates | |
| 22:34:31 | mgagne | and I'm not sure the doc fully explain its purpose, I would have never guess what you said from the doc, it mainly explains how it's not nova aggregates | |
| 22:34:37 | mriedem | mgagne: is the spec that talks about aggregates the most i think https://specs.openstack.org/openstack/nova-specs/specs/newton/implemented/generic-resource-pools.html | |
| 22:34:46 | mriedem | mgagne: heh yeah that's what i was thinking | |