[RFE] l3_agent should separate router_info creation logic

Bug #1804634 reported by Yang Youseok
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Fix Released
Wishlist
Yang Youseok

Bug Description

Currently, l3-agent has tightly coupled with router_info creation logic, so there is no way to make a big change of default router's behaviors. Even there are already many diverse routers (dvr, dvrha, dvr_snat...), every routers depend detailed implementation rather than structured interfaces. So it makes hard to add new feature.

I found a majority of networking-* with l3 implementation just override core L3NatAgent and add a little tweaking functions to change default l3-agent behavior. IMHO, if there is clear interface in L3 agent for specific router, they just add their router business logic rather than did not override core agent class. I noticed they did not change RPC mechanism a lot in agent side, and want to just make few tweak like us.

What I suggest is make abstract class for router with minimal functions (initialize(), process()..), and add create router logic with dynamically configured router_info class. It decouples the creation and RPC controller so that plugin developer can easily change the default router behavior.

To provide more specific use case, what we have done was to add linux VRF feature instead of namespace and East-West traffic is enabled by default so that there is no need to add GW / internal port at all. We do not need majority of functions except floating IP, but every kind of API / RPC is useful at the same time from server side.

Tags: rfe-approved
Yang Youseok (ileixe)
tags: added: rfe
Yang Youseok (ileixe)
Changed in neutron:
assignee: nobody → Yang Youseok (ileixe)
Revision history for this message
YAMAMOTO Takashi (yamamoto) wrote :

in case you have a concrete proposal, i guess it's nice to have a spec.

tags: added: rfe-confirmed
removed: rfe
tags: added: rfe-triaged
removed: rfe-confirmed
Changed in neutron:
importance: Undecided → Wishlist
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron-lib (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/620348

Changed in neutron:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (master)

Fix proposed to branch: master
Review: https://review.openstack.org/620349

Revision history for this message
Yang Youseok (ileixe) wrote :

@YAMAMOTO

Hello YAMAMOTO. Thanks for reply.

I did not notice your reply, so I just initial commits for the patch, Sorry for that.
Since I am newbie for this project, may I ask you what to do for spec? I think spec you said
seems to 'https://specs.openstack.org/openstack/neutron-specs/' though, but I'm not sure.

Revision history for this message
Miguel Lavalle (minsel) wrote :

This RFE was discussed and approved today. Since the proposed enhancement has the potential to impact not only the Neutron reference implementation but also other networking related projects, a spec is required as a next step. Please create the spec here: https://github.com/openstack/neutron-specs/tree/master/specs/stein

tags: added: rfe-approved
removed: rfe-triaged
Revision history for this message
Yang Youseok (ileixe) wrote :

@Miguel Thanks for the approve. I will be glad to make the spec to follow your request.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron-specs (master)

Reviewed: https://review.openstack.org/625647
Committed: https://git.openstack.org/cgit/openstack/neutron-specs/commit/?id=bd23545778016827e7f520488c224a7943b59310
Submitter: Zuul
Branch: master

commit bd23545778016827e7f520488c224a7943b59310
Author: Yang Youseok <email address hidden>
Date: Tue Dec 18 01:21:05 2018 +0900

    Registering RouterInfo by L3 extention API

    Currently, most plugin implementations releated L3 override the
    L3NatAgent class itself for their own logic since there is no proper
    interface to extend RouterInfo class. This adds unnecessary complexity
    for developers who just want to extend agent mechanism instead of
    whole RPC related to L3 functinalities.

    This spec introduces RouterFactory class which acts on the factory for
    creating RouterInfo class, and add new l3 extension API which enable to
    dynamically add RouterInfo to the factory. Now plugin developers can
    use new extension API for their own specific router.

    Change-Id: Ic3486830e449f6ee8dbe19db614179b2077bcf7b
    Related-Bug: #1804634
    Implements: blueprint router-factory-with-l3-extension

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron-lib (master)

Reviewed: https://review.openstack.org/620348
Committed: https://git.openstack.org/cgit/openstack/neutron-lib/commit/?id=176c09fa192df4b42570d1478039607108fad6d1
Submitter: Zuul
Branch: master

commit 176c09fa192df4b42570d1478039607108fad6d1
Author: Yang Youseok <email address hidden>
Date: Fri Nov 23 15:41:20 2018 +0900

    Add RouterNotFoundInRouterFactory exception

    Add new exception for l3-agent. It occurs when there is no valid
    router_info class which registered in router_factory.

    Related-Bug: #1804634
    Change-Id: I55a1cb60898ab87ffe137f8c4b0b2f2803bfb219

Changed in neutron:
assignee: Yang Youseok (ileixe) → Igor D.C. (igordcard)
Changed in neutron:
assignee: Igor D.C. (igordcard) → Yang Youseok (ileixe)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.opendev.org/620349
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=ec875b42b6e92300093c895668532966de9e1327
Submitter: Zuul
Branch: master

commit ec875b42b6e92300093c895668532966de9e1327
Author: Yang Youseok <email address hidden>
Date: Fri Nov 23 15:55:02 2018 +0900

    Add router_factory to l3-agent and L3 extension API

    Currently, most implementations override the L3NatAgent class itself
    for their own logic since there is no proper interface to extend
    RouterInfo class. This adds unnecessary complexity for developers
    who just want to extend router mechanism instead of whole RPC.

    Add a RouterFactory class that developer can registers RouterInfo class
    and delegate it for RouterInfo creation. Seperate functions and variables
    which currently used externally to abstract class from RouterInfo, so that
    extension can use the basic interface.

    Provide the router registration function to the l3 extension API so that
    extension can extend RouterInfo itself which correspond to each features
    (ha, distribtued, ha + distributed)

    Depends-On: https://review.openstack.org/#/c/620348/
    Closes-Bug: #1804634
    Partially-Implements: blueprint openflow-based-dvr
    Change-Id: I1eff726900a8e67596814ca9a5f392938f154d7b

Changed in neutron:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/neutron 15.0.0.0b1

This issue was fixed in the openstack/neutron 15.0.0.0b1 development milestone.

Revision history for this message
Nate Johnston (nate-johnston) wrote :

FYI this caused a problem for neutron-fwaas since it was not adjusted to include the new required arguments. See: https://bugs.launchpad.net/neutron/+bug/1845227 - fix in progress.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.