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
|
+--------------------------------------+---------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| 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
~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
Mailing list: https://launchpad.net/~quantum-core
Post to : [email protected]
Unsubscribe : https://launchpad.net/~quantum-core
More help : https://help.launchpad.net/ListHelp
