| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-22 | |||
| 16:20:44 | eandersson | as you are passing on endpoint = None | |
| 16:20:59 | eandersson | https://review.opendev.org/#/c/653839/ | |
| 16:21:02 | eandersson | This was the actual test | |
| 16:22:02 | efried | eandersson: But that test only reports that our endpoint lookup failed | |
| 16:22:12 | eandersson | Exactly | |
| 16:22:13 | efried | eandersson: ...which is still a valid code path | |
| 16:22:16 | eandersson | Sure | |
| 16:22:21 | eandersson | But regions don't actually work | |
| 16:22:21 | efried | like, if you specify a bogus region name or whatever. | |
| 16:22:25 | efried | right | |
| 16:22:29 | eandersson | Not sure what your point is | |
| 16:22:30 | efried | see latest comment in the devstack patch. | |
| 16:22:55 | eandersson | Well that patch was not to prove that multi region was broken | |
| 16:22:59 | efried | eandersson: My point is: I'm happy to have the behavior fixed, but it would be nice to have a regression test in place. | |
| 16:23:07 | eandersson | That patch was to prove that get_endpoint was broken if region is supplied | |
| 16:23:21 | eandersson | Of course if you introduced multi-region it would have been way worse | |
| 16:23:29 | efried | okay, and that proof is the presence of that log message... | |
| 16:23:30 | eandersson | Since that would never have worked | |
| 16:23:59 | efried | so now if we reinstate that patch and rebase it onto the fix, the log message should be absent. | |
| 16:24:12 | eandersson | Exactly | |
| 16:24:20 | efried | In fact, we could just raise an exception and blow up the world if the endpoint is None there. | |
| 16:24:29 | eandersson | I think we should. | |
| 16:24:36 | efried | cool, you or me? :) | |
| 16:24:55 | eandersson | I can follow up after your patch so people can git blame me :p | |
| 16:25:05 | efried | So hold on a sec | |
| 16:25:15 | efried | I'm not completely convinced we should be doing this for real | |
| 16:25:20 | efried | just to prove the fix | |
| 16:25:25 | eandersson | We can also take path nr 2 | |
| 16:25:32 | eandersson | just pass on the region_name to the ironic client | |
| 16:25:51 | efried | If ironicclient has a way to recover, and we disable/disallow it, that could piss off some operators :) | |
| 16:26:02 | efried | So yeah, passing the region_name to the ironicclient's recovery dealy would be better I guess. | |
| 16:26:04 | eandersson | The problem is really that we are letting the ironic client handle a failure scenario, but not actually giving it the context it needs | |
| 16:26:09 | efried | Though once again we're throwing good code after bad. | |
| 16:26:47 | efried | okay, to pass the region_name through, we only need changes on the nova side, right? ironicclient is already set up to handle it if it's in kwargs? | |
| 16:27:14 | eandersson | https://github.com/openstack/python-ironicclient/blob/master/ironicclient/client.py#L121 | |
| 16:27:18 | eandersson | Looks like it yea | |
| 16:28:20 | efried | Cool. So two things: | |
| 16:28:21 | efried | 2) New patch to pass region_name through to irocicclient setup | |
| 16:28:21 | efried | 1) Test patch that raises an exception if the ksa endpoint lookup fails, based on top of the fix https://review.opendev.org/654457 <== expect it *not* to blow up | |
| 16:28:33 | eandersson | Perfect! | |
| 16:28:36 | efried | I wonder if 2) should actually take two completely separate code paths. | |
| 16:28:49 | efried | One if ksa endpoint lookup succeeds, that passes in no additional kwargs | |
| 16:28:51 | eandersson | Yes - that would be fine | |
| 16:28:57 | eandersson | And safer | |
| 16:29:01 | efried | and a separate one if ksa lookup fails, that passes in basically all the ksa opts :( | |
| 16:29:37 | efried | If you have the time to propose those, I'll happily review. I'm in the midst of summit chart prep hell. | |
| 16:30:45 | eandersson | Sure - I'll try. I am also slammed this week though. | |
| 16:31:19 | efried | looks like interface is already passed through. service_type *should* be unnecessary, but <shrug> guess it wouldn't hurt to pass it through. | |
| 16:31:29 | efried | and those are the only three used at the moment. | |
| 16:31:32 | eandersson | Yea - was thinking the same. | |
| 16:32:49 | eandersson | Unrelated to this discussion. Is it just for me that I can't open links on review.opendev.org in new tabs anymore? | |
| 16:34:32 | eandersson | (right click, Open link in new tab) | |
| 16:36:01 | efried | uh | |
| 16:36:17 | efried | eandersson: wfm | |
| 16:36:31 | efried | what happens when you try it? | |
| 16:36:35 | eandersson | Nothing | |
| 16:36:59 | eandersson | Hmm works on Firefox | |
| 16:37:03 | eandersson | I guess my browser is just acting up. | |
| 16:37:41 | efried | I'm Chrome 73.0.3683.75 on Bionic fwiw | |
| 16:38:21 | efried | kashyap: I'm reading https://review.opendev.org/#/c/645814/4/specs/train/approved/cpu-selection-with-hypervisor-consideration.rst and not understanding it, at I think a more fundamental level than other reviewers. | |
| 16:39:20 | efried | I suspect I would be able to infer more if you addressed mriedem's comments | |
| 16:40:08 | efried | Do you have a sec to give me a brief rundown on what actual operations or behaviors are affected by these *HypervisorCPU() calls? | |
| 16:41:52 | openstackgerrit | Eric Fried proposed openstack/nova master: Fix {min|max}_version in ironic Adapter setup https://review.opendev.org/654457 | |
| 17:47:29 | openstackgerrit | Nikolay Fedotov proposed openstack/nova master: Fix disappearing ironic hypervisors after rebuilding hashring https://review.opendev.org/654584 | |
| 17:54:25 | sean-k-mooney | efried: kashyap spec is mainly techdebt removal through the use of newer libvirt apis that would allow nova to validate that the configuration specified in the nova.conf is valid | |
| 17:55:09 | efried | sean-k-mooney: Resulting in... earlier failures in spawn()? Refusal to start nova-compute? | |
| 17:55:54 | sean-k-mooney | i was suggesting failing to start the agent if the cpu flags were not compatible with the host/model specifed | |
| 17:56:07 | sean-k-mooney | but the spec does not really make it clear | |
| 17:57:32 | sean-k-mooney | it will in theroy also make the traits we report more acurate as it would take into account if kvm or qemu/which version of qemu is used too. | |
| 18:00:40 | sean-k-mooney | with all that said i agree with mriedem commets. the spec does not really spell out how useing the new apis deliver a new feature or capablity to operators/users form a nova perspecitve | |
| 18:03:10 | efried | thanks sean-k-mooney. I'm putting together charts for the project update at the summit and need to understand to *some* extent what each blueprint is about. | |
| 18:04:43 | sean-k-mooney | efried: from kasyaps top comment on the rewview 'Nova will now be able to answer: "Is this combination of IvyBridge + 'pcid' & 'pdpe1gb' supported by KVM, QEMU and libvirt on the host?"' | |
| 18:05:24 | efried | Right, "be able to answer" doesn't help me understand what will actually *happen* | |
| 18:05:58 | efried | so, detect whether A+B+C is copacetic and... what? | |
| 18:06:17 | sean-k-mooney | so really kashyap is trying to turn the a possible runtime failure when you boot the first vm into hopefully a startup failure of the agent with a detailed error message | |
| 18:06:28 | efried | when you say "agent", you mean n-cpu? | |
| 18:06:43 | efried | ...by blowing up the virt driver init, I guess | |
| 18:06:43 | sean-k-mooney | ya at least that what i hope is the plan | |
| 18:06:50 | sean-k-mooney | yep | |
| 18:06:54 | efried | Cool. Definitely needs to be spelled out in the spec. | |
| 18:07:02 | efried | That is *not* an implementation detail :) | |
| 18:07:31 | sean-k-mooney | ya kasap has a second spec that he inherited that is related | |
| 18:07:54 | sean-k-mooney | i think the agent stoping might be in that one | |
| 18:08:16 | sean-k-mooney | https://review.opendev.org/#/c/642030/ | |
| 18:09:17 | sean-k-mooney | ya line 71 https://review.opendev.org/#/c/642030/6/specs/train/approved/cpu-model-selection.rst@71 | |
| 18:14:13 | openstackgerrit | sean mooney proposed openstack/nova master: [WIP] Libvirt: add nfv job https://review.opendev.org/652197 | |
| 18:15:08 | efried | stephenfin: mriedem: I don't see a bp for nova-console removal. Who was going to be spearheading that? | |
| 18:16:03 | sean-k-mooney | efried: fyi its a bankholiday here in ireland so stephenfin proably wont see that until tomorrow | |
| 18:16:34 | efried | sean-k-mooney: ack, thx | |
| 18:16:37 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add placement request filter for disabled computes https://review.opendev.org/654596 | |
| 18:16:39 | mriedem | efried: sean-k-mooney: dansmith: belmoreira: ^ here is a PoC for the disabled compute pre-filtering stuff we talked about in berlin | |
| 18:16:43 | mriedem | melwitt: ^ | |
| 18:16:44 | efried | mdbooth_: nudge, dud you see http://lists.openstack.org/pipermail/openstack-discuss/2019-April/005366.html ? | |
| 18:16:59 | mriedem | efried: i don't think there is a blueprint or a driver | |
| 18:17:19 | efried | ack | |
| 18:17:20 | sean-k-mooney | oh cool | |
| 18:17:32 | mriedem | efried: i wouldn't probably move forward with nova-console removal without a definitive ack from the citrix team | |
| 18:17:37 | dansmith | isn't jaypipes going to lose his mind about that trait? | |
| 18:17:54 | mriedem | why? | |
| 18:18:04 | sean-k-mooney | metadata in placement | |