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