Hi, In the current router-list external_gateway_info, gw_port does not provide useful information at all (except mac_address). fixed_ips of gw_port could be useful but it is intentionally hidden in the l3_db code. IMO, it is better that router-list external_gateway_info should have 'network_id only' or 'network_id + fixed_ips'. (The same thing applies to router-show.9
Regarding router-show I prefer yong's proposal that router interfaces are also displayed. However, displaying all information may be too verbose. The fields I often look at are fixed_ips and subnet_id (+ sometimes port_id). If we want to know more detail, port-show (or router-port-list) can be used. This is just my preference. I also understand selecting some field to be displayed needs more logic and requires additional changes if router resources is changed in future. -- Akihiro MOTOKI <[email protected]> Cloud System Research Laboratories, NEC Corporation >>>>> Date: Mon, 5 Nov 2012 21:43:25 -0800 >>>>> From: Dan Wendlandt <[email protected]> >>>>> Subject: Re: [Quantum-core] [Quantum] Fwd: [Bug 1069782] Re: >>>>> quantumrouter-show should return interfaces too > > On Mon, Nov 5, 2012 at 9:24 PM, gong yong sheng <[email protected]> > wrote: > > Yes, you have to use new python client: > https://review.openstack.org/#/c/15100/ > > But since you do not agree with it, I am going to remove the gw_port from > server side and > abandon this client change. > > That was just my personal preference... I was hoping more people would chime > in on this thread > with their opinions. Especially from the CLI sub-team :) > > > On 11/06/2012 01:01 PM, Dan Wendlandt wrote: > > to pick up this thread, after this change, I also noted that the > quantum router-list > command is very difficult to read, due to the inclusion of all > gw_port info. Here's a > router-list with a single router from devstack: > > nicira@com-dev:~/devstack$ quantum router-list > > +--------------------------------------+---------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ > | id | name | > > external_gateway_info > &n! bsp; > > >  ! ; &nb sp; | > > > +--------------------------------------+---------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ > | 166728cb-fc7f-477f-bfab-449a11827b02 | router1 | {"network_id": > "1406a82e-7137-4c03-9d6a-7919fa000a82", "gw_port": {"status": > "ACTIVE", "name": "", > "admin_state_up": true, "network_id": > "1406a82e-7137-4c03-9d6a-7919fa000a82", "tenant_id": > "", "device_owner": "network:router_gateway", "mac_address": > "fa:16:3e:33:bc:52", "id": > "8d11222f-cb0d-4556-ad3b-51de5f48f658", "device_id": > "166728cb-fc7f-477f-bfab-449a11827b02"}} | > > +--------------------------------------+---------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ > > On Fri, Nov 2, 2012 at 9:09 AM, Dan Wendlandt <[email protected]> wrote: > > On Thu, Nov 1, 2012 at 5:57 PM, gong yong sheng > <[email protected]> wrote: > > Dear quantum statckers, > > When I was at the vmware quantum workshop (it is a perfect > one), I observed it > is very unfriendly for a user to use many commands to know > about one router > information. > These un-friendlinesses are: > > To be clear, I definitely noticed the same issue during the > workshop and completely > agree with giving folks a more intuitive way to see this info. > > > 1. quantum router-list > it just lists the network-id of external gateway information. > For admin to know > the gw_port, he has to type in: > quantum port-list -- --device_id=<router-id> and device_owner= > network:router_gateway > of course we can add another convenient command: > quantum router-interface-show router_id > > with this change, > quantum router-list can show more about gw port besides the > network-id, for > example gw_port_id > after that, to show the gwport, admin can use normal command: > quantum port-show gw-port-id > 2. quantum router-show router-id > As a show command of a router, we should show every thing of > the router here. (of > course according to the user's role) > > My feeling here is slightly different. I see the relationship of > router + ports > similar to how we already do networks + ports. If you do a show > network, it doesn't > show each port on the network. You need to list ports for that. > The motivation for > this was to make it easy to see the key details of a device even > when it has many > ports. I would advocate for adding convenience methods that are > network-port-list > <network-id) and router-port-list <router-id> that simplify the > task of listing the > ports associated with a particular device (with output similar to > port-list). > > I also think it would be nice if the CLI had a good way of doing > a "bulk show", > similar to what you do below with the showing all of the > interface details in more of > a list format (rather than requiring the user to do a 'show' on a > set of ids). > Perhaps this could be a --details flag that is past to any list > command. That way it > would be easy to see all details for ports on a router just be > doing "quantum > list-router-ports --details <router-id>". > > > > This include the interfaces, external gateway information > among the current simple > ones. > Admin is also a person, he should hate to remember many other > commands, even if > they are so-called convenient ones. > With the fully-fledged, true show command, admin's life will > be easier ( we cannot > assume admin is just admining quantum) > gongysh@gongysh-laptop:~$ quantum router-show > 3d7229c6-2594-425e-8551-665817ecbb99 > > +-----------------------+----------------------------------------------------------------------+ > | Field | > > Value | > > +-----------------------+----------------------------------------------------------------------+ > | admin_state_up | > > True | > | external_gateway_info | > > { | > | | "network_id": > "900a24a6-ae02-4000-a010-81a5c2045a20", | > | | "gw_port": > { | > | | "status": > "ACTIVE", | > | | "name": > "", | > | | "admin_state_up": > true, | > | | "network_id": > "900a24a6-ae02-4000-a010-81a5c2045a20", | > | | "tenant_id": > "", | > | | "device_owner": > "network:router_gateway", | > | | "mac_address": > "fa:16:3e:87:4c:ef", | > | | "id": > "ec340103-2810-41f1-afb5-0fdcefd56681", | > | | "device_id": > "3d7229c6-2594-425e-8551-665817ecbb99" | > | | } > > | > | | } > > | > | id | > > 3d7229c6-2594-425e-8551-665817ecbb99 | > | interface_ports | > > { | > | | "status": > "ACTIVE", | > | | "name": > "", | > | | "admin_state_up": > true, | > | | "network_id": > "723c5d8e-6e66-4f7c-ad89-8ed58468fe30", | > | | "tenant_id": > "b7445f221cda4f4a8ac7db6b218b1339", | > | | "device_owner": > "network:router_interface", | > | | "mac_address": > "fa:16:3e:2b:63:f0", | > | | "fixed_ips": > [ | > | | > { | > | | "subnet_id": > "5d1a4f3d-2264-4011-98cd-772513b2a1f9", | > | | "ip_address": > "20.0.1.1" | > | | } > | > | | > > ], | > | | "id": > "968e6944-a2b3-4d05-9e7c-020392f40eaf", | > | | "device_id": > "3d7229c6-2594-425e-8551-665817ecbb99" | > | | } > > | > | | > > { | > | | "status": > "ACTIVE", | > | | "name": > "", | > | | "admin_state_up": > true, | > | | "network_id": > "a9843ef2-3446-4d71-91f9-588f8418a180", | > | | "tenant_id": > "b7445f221cda4f4a8ac7db6b218b1339", | > | | "device_owner": > "network:router_interface", | > | | "mac_address": > "fa:16:3e:46:99:89", | > | | "fixed_ips": > [ | > | | > { | > | | "subnet_id": > "4c459b17-6185-4451-9f1e-996ee0232bc2", | > | | "ip_address": > "10.0.1.1" | > | | } > | > | | > > ], | > | | "id": > "a61f9820-276b-460c-8372-62c5a1b3d488", | > | | "device_id": > "3d7229c6-2594-425e-8551-665817ecbb99" | > | | } > > | > | name | > > router1 | > | status | > > ACTIVE | > | tenant_id | > > b7445f221cda4f4a8ac7db6b218b1339 | > > +-----------------------+----------------------------------------------------------------------+ > > solution to your concerns: > 1. API back compatibility: > I only add more information, so just as Salvatore said, it is > not super bad. It is > back compatible. > But I should have put this change under more flash lights. > 2. for breaking policy or security, I suggest to add admin > check in the code: > so if context.is_admin(), I will put the gw_port into router > result. > 3. for only gw_port_id or entire gw_port > I prefer to entire gw_port. with gw_port, client side (cli or > l3agent) does not > need to fetch the gw_port again if it needs it. current > l3agent will fetch it > again. > > Including the entire port can complicate CLI parsing, as there > now would be two > different 'id' fields in the output, one for the router, one for > the gw_port. I'd > vote for showing these ports details using the list-router-ports > command above. > > > 4. of course, if you are not satisfied with my solution to > add more into API, we > can do it totally on client command side. > That means client command quantum router-show will run one > more time to get the > gw_port information just as current l3agent does so that it > can do true show. > > > I'm actually OK changing the API if everyone feels like its the > right thing to do. I > just want to make sure we have a good process for doing so. > > Thanks, > > dan > > any ideas? > > Yong Sheng Gong > > On 11/02/2012 06:40 AM, Salvatore Orlando wrote: > > HI, > > I am becoming aware of this patch just now, and I would > like to thank Dan for > raising the issue. > Bug 1069782 was initially filed for adding convenience > client-side commands > for retrieving router interfaces, and I believe it was > not supposed to have > any server side change. > > In general we should avoid being 'trigger-happy' with > changes on the > tenant-facing API, even when the change appears totally > reasonable and > improves the API. Keeping that in mind, this particular > change won't break > backward compatibility, so it's not super-bad. However, > it will break the > default policy engine rules - or, to be more precise, it > will circumvent the > policy engine. Indeed it won't matter what is the policy > for ports on external > network, you'll end up showing some details about it. You > might argue that > this is something that needs to be addressed in the > policy engine, and I would > agree with you. > > Dan proposes two solution which I believe are dictated by > a "safety first" > approach. They both seem reasonable to me. The > client-side option looks > cleaner but less efficient, where the admin-only option > would be more > efficient, and still safe (although I might have some > reservation if the > admin-only constraint becomes hard coded). > > Salvatore > > On 1 November 2012 22:11, Dan Wendlandt <[email protected]> > wrote: > > Hi team, > > I have two concerns with the commit below, one > general, one specific. > > First off, the general concern: this change changes > our tenant-facing API, > but it seems that there was no real public discussion > or notification of > this change. Our tenant-facing APIs, including > extensions, must be > stable, consistent, and documented. I would argue > that changes to the API > should be part of blueprints, which will trigger a > discussion about API > consistency, documentation, etc. > > Second, the specific one: I want to explain the > reasoning why this field > was not included in the original version of the L3 > API, and why my current > thinking is that it probably should not be added. > The model we were going > with for L3 routers is that the details of the > gateway are abstracted from > the tenant. In particular, the port used to "hold" > the IP allocation for > gateway and floating-ip ports are not visible to a > common tenant and > should not be manipulated directly. They are only > visible to an admin > user. As a result, I would argue that include the gw > port info in the > JSON that is visible to all tenants doesn't really > make sense. We could > consider adding it as an admin only attribute > (similar to how the provider > network attributes are included in a network, but > only for admins), though > this info is already available to admin users if they > search for ports > that have the device_owner=<router-id> and > device_owner= > network:router_gateway . We could even has the CLI > do this as part of the > router-show command, if the user was an admin. > > I think we can have a technical discussions about the > second issue, but my > most major concern is actually the first issue in > that we need have a more > deliberate process when it comes to making changes to > tenant-facing APIs. > > Dan > > p.s. if we end up deciding to make the change to the > API, I would also > argue that the field be named something like > gw_port_id, rather than > gw_port, as we try to maintain consistency that UUID > fields end with _id. > This is where Salvatore and the API police come into > play :P > > ---------- Forwarded message ---------- > From: OpenStack Hudson <[email protected]> > Date: Thu, Nov 1, 2012 at 2:40 AM > Subject: [Bug 1069782] Re: quantum router-show should > return interfaces > too > To: [email protected] > > Reviewed: https://review.openstack.org/15101 > Committed: http://github.com/openstack/quantum/commit/ > c1c19b11792e8e220b11466051739ea5b4279a7a > Submitter: Jenkins > Branch: master > > commit c1c19b11792e8e220b11466051739ea5b4279a7a > Author: gongysh <[email protected]> > Date: Wed Oct 31 22:01:35 2012 +0800 > > Put gw_port into router dict result. > > Bug #1069782 > > We put gw_port into router dict result so that > client can get more > information for the result router. > > Change-Id: > I54cec8a71441a9370c7ba95767a92190bf1c9c21 > > ** Changed in: quantum > Status: In Progress => Fix Committed > > -- > You received this bug notification because you are a > member of Netstack > Core Developers, which is subscribed to quantum. > https://bugs.launchpad.net/bugs/1069782 > > Title: > quantum router-show should return interfaces too > > Status in Python client library for Quantum: > In Progress > Status in OpenStack Quantum (virtual network service): > Fix Committed > > Bug description: > interfaces and external_gateway_info are two arms > of a router, so we > should be able to show interface information too. > > quantum router-show myrouter1 > > +-----------------------+--------------------------------------+ > | Field | Value > | > > +-----------------------+--------------------------------------+ > | admin_state_up | True > | > | external_gateway_info | > | > | id | > c508d501-8019-4e12-b332-fd19ac3be79e | > | name | myrouter1 > | > | status | ACTIVE > | > | tenant_id | > b7445f221cda4f4a8ac7db6b218b1339 | > > +-----------------------+--------------------------------------+ > > To manage notifications about this bug go to: > > https://bugs.launchpad.net/python-quantumclient/+bug/1069782/ > +subscriptions > > -- > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > Dan Wendlandt > Nicira, Inc: www.nicira.com > twitter: danwendlandt > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > -- > Mailing list: https://launchpad.net/~quantum-core > Post to : [email protected] > Unsubscribe : https://launchpad.net/~quantum-core > More help : https://help.launchpad.net/ListHelp > > -- > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > Dan Wendlandt > Nicira, Inc: www.nicira.com > twitter: danwendlandt > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > -- > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > Dan Wendlandt > Nicira, Inc: www.nicira.com > twitter: danwendlandt > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > -- > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > Dan Wendlandt > Nicira, Inc: www.nicira.com > twitter: danwendlandt > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > -- Mailing list: https://launchpad.net/~quantum-core Post to : [email protected] Unsubscribe : https://launchpad.net/~quantum-core More help : https://help.launchpad.net/ListHelp
