Horizon support for Neutron Trunks

Registered by Bence Romsics

Summary
=======
Support the trunk ports (a.k.a. vlan-aware-vms) feature of OpenStack
Neutron in Horizon.

Motivation
==========
Since release Newton the trunk port feature is available in Neutron
but only via the API and the CLI. It would be nice if it was exposed in
Horizon too.

Description
===========
OpenStack Neutron supports a new feature called trunk ports since the
Newton release. This feature allows the cloud user to connect lots of
networks to a single instance without having lots of vNICs. Instead of
many vNICs the networks are differentiated via tagged traffic. For more
information please see [1][2][3].

As discussed on openstack-dev [4] this Horizon feature should live in
the openstack/horizon repo.

Overview
--------
Create a new panel: Project / Network / Trunks.
Change the table of the available ports in the Boot Instance
workflow to mark trunk parent ports as such. The marking can be done
based on the presence of the 'trunk_details' port attribute.

The primary impact on Horizon is the need for a new workflow to create Trunks.

A secondary Horizon impact is the need to create certain ports through Horizon
when booting by trunk parent port (from here on "boot by trunk"). In the
traditional boot by network case Horizon does not need to create a port
because Nova does that. However relying on Nova to create the port would
be too late in the boot by trunk case, because (1) the trunk must be
created with a reference to the parent port and (2) some Neutron drivers
(notably the Open vSwitch driver) do not allow the creation of a trunk on
a parent port already bound.

Because of bug #1399252 [7] port creation is an admin-only operation in
Horizon as of 2017-02 (while it is not an admin-only operation in Neutron
with the usual policy configuration). This blueprint depends on the fix for
bug #1399252. See also [5] and [8].

(Note by amotoki: the reason it is not supported in the initial implementaiton is just because it is not a good idea to
touch port directly as nova, neutorn internal services or others maintain ports and deleting them directly
will break the behavior. Exposing the port operation potentially leads to unexpected behaviors for light users.)

In the API, trunks refer to ports (one parent and zero or many child ports)
and ports refer to networks. These relations must be exposed to the user.

For input validation Horizon's existing practices should be followed. That
is duplication of basic validation logic is acceptable if it does not
involve API calls. In the first implementation to keep the scope small all
input validation should be left to Neutron.

UX
==
A new panel: Project / Network / Trunks
  Has a table of trunks:
    With columns:
      name
      parent port
      admin state
      status
      subport count (Nice to have, not a must.)
  With row actions:
    edit trunk (This encompasses 'edit subport details' because 'subports' is a trunk attribute.)
    delete trunk (The parent and child ports referred by the trunk should not be automatically deleted.)
  With table actions:
    create trunk
Mark trunk parents in the table of available ports

Ideas for later improvements (not addressed here):

* While setting one or more ports as child ports to a trunk, there should be a choice:
** to leave the MAC addresses of the child ports as they are, or
** to update the MAC addresses of the child ports to the parent's MAC address.
That is to say to "clone" the parent port's MAC address. This helps the user
to bring up the subport VLAN interfaces inside the guest, because VLAN
subinterfaces by default get their MAC addresses from the interface they are
being added to. For example eth0.100 will have the same MAC address as eth0,
unless specified otherwise. Note, that "cloning" does usually not lead to
a conflict because all of these these ports are on different networks.

* Modify the table of available ports in the 'Network Ports' step
  in the Launch Instance workflow, so to filter out the trunk child ports.
  Because booting on those would be an error without an error message.
  See [6].

* Catch invalid subport configurations (eg. violations of uniqueness
  constraints) early from client side JavaScript.

* Option to cascade trunk delete to the ports too.

Wireframes, Mocks, Videos and UI Markup
---------------------------------------
None.

Testing
=======
Unit tests.

Outside Dependencies
====================
Neutron extensions needed:

trunk
trunk_details

Neutron provides these extensions (at least for some drivers) since Newton.

Requirements Update Required
============================
None.

Doc Impact
==========
Release note.
Any new settings must be documented.

References
==========
[1] https://blueprints.launchpad.net/neutron/+spec/vlan-aware-vms
[2] https://github.com/openstack/openstack-manuals/blob/master/doc/networking-guide/source/config-trunking.rst
[3] https://wiki.openstack.org/wiki/Neutron/TrunkPort
[4] http://lists.openstack.org/pipermail/openstack-dev/2017-February/111608.html
[5] http://docs.openstack.org/user-guide/dashboard-create-networks.html#create-a-port
[6] https://bugs.launchpad.net/neutron/+bug/1636485
[7] https://launchpad.net/bugs/1399252
[8] https://review.openstack.org/320203

Blueprint information

Status:
Complete
Approver:
Akihiro Motoki
Priority:
Low
Drafter:
Bence Romsics
Direction:
Approved
Assignee:
Bence Romsics
Definition:
Approved
Series goal:
Accepted for 13.0.0-queens
Implementation:
Implemented
Milestone target:
milestone icon queens-rc1
Started by
Rob Cresswell
Completed by
Akihiro Motoki

Related branches

Sprints

Whiteboard

[Feb 6, 2018 - amotoki] Mark it as Implemented.
As we discussed in the team meeting last week, the final patch of admin trunk support has been merged. Further improvements will be made as separate bugs (or blueprint if necessary).

---

As discussed on the weekly meeting here are three topics in which we are asking for core guidance:

Question #1 - Delayed opening of create/edit modals.

When the user clicks 'Create' or 'Edit' in the trunks panel the modal
window is opened only after some neutron API calls were made. There
are only 2 to 3 calls made. Depending on the response time of the API
the delay may or may not be instantly noticeable. Anyway this is clearly
suboptimal user experience. Ideally we should give instant response -
at least an hourglass/spinner.

We originally tried opening the modal (including the transfer tables
in the steps) and concurrently querying neutron. Then after neutron
responded loading the fresh response into the already initialized transfer
tables. However we've hit problems while trying to re-initialize the
transfer tables. Now I think transfer table does not support changing
its contents from behind.

So we are looking for a compromise where the user is given feedback
about the API requests started in the background, but we avoid hitting
the problems of re-initializing transfer tables.

See also:

https://review.openstack.org/#/c/468396/32/openstack_dashboard/static/app/core/trunks/actions/create.action.service.js@72
https://review.openstack.org/#/c/485473/19/openstack_dashboard/static/app/core/trunks/actions/edit.action.service.js@69

Question #2 - Project and admin versions of angular-only panels.

I know this was a topic on the PTG, but unfortunately I missed the final
conclusion. Given the angularjs-only trunks panel which one shall we have?

a) Two panels: A project and an admin panel in the usual manner. That
is only an admin can see the admin panel. The project panel is always
restricted for a single project - even for an admin.

b) One panel: A project panel which behaves differently for admins
and non-admins.

Question #3 - A more generic stepModels would help.

This is complicated. Please bear with me. First please take a quick look
into the background: [1][2].

In short: The (create/edit) action services are meant to be stateless
(to keep them reusable), so all state (mostly user input) should be
kept in the step controllers. But the model proposed in a previous
refactoring (see [2]) looks a bit too simplistic for our use case. I
think the refactor in [2] solves one problem:

#3.1 Keep the state (ie. user input) in the step controllers and feed
(communicate) it into the API calls without ever storing the data in
the action service.

But our use case has further problems to solve:

#3.2 Communicate between the steps. Use case: One port selected to
become a trunk parent should not be available as a subport candidate
(ie. must be hidden) and vice versa.

#3.3 In the edit action keep the old state around, so we can process it
together with the new state. Here we basically had some Python code [3] that
we hoped to reuse in the API layer. It took the old and new states of a
trunk and turned the difference into neutron API updates. This problem is
of secondary importance for question #2, because there's a clearcut alternative:
Move the diffing logic (between old/new) from the Python API layer
to the JavaScript controller layer. Then the old state is only needed
in the JavaScript controller. The controller diffs the old and new,
throws away the old, sends only the diff back to the API layer. But
anyway keeping the old state around until it's available together with
the new may make sense for others too.

As a result we are stuffing much more into stepModels then it was
originally meant.

Original stepModels:

* model1
* model2
* etc

Our stepModels:

* trunkSlices
** closure1 returning model1
** closure2 returning model2
** etc
* the bits on which other steps can put watchers
* the oldTrunk state

Note: We replaced the models with closures returning the models, so we
can spare recomputing the models on every user keystroke.

I think there's no problem with the extended stepModels in itself. But
the change of what's inside stepModels is clearly in conflict with the
original goal of making the steps reusable from multiple workflows.
Is that a problem for us? Or is the extended stepModels okay as it is?

See also:

[1] http://lists.openstack.org/pipermail/openstack-dev/2016-July/099368.html
[2] https://review.openstack.org/345145
[3] https://review.openstack.org/#/c/485473/19/openstack_dashboard/api/neutron.py
[4] https://review.openstack.org/#/c/468396/32/openstack_dashboard/static/app/core/trunks/steps/trunk-details.controller.js@54

[yingzuo 2017-10-25] Thanks for writing down the questions. Here are my thoughts:
#1 Show the loading icon(fa-spinner) in the place of the transfer table until it has all data needed.
#2 We decided to keep two panels which is approach a.
#3.1 It's not a problem right?
#3.2 Can we use watchers to update the selection?
#3.3 Why do we need to do the diff between the new and old states?
Can we just send the new state instead of the diff to the API? I don't see why you have to use the closure to return the model 1 and 2. Do you have a patch for this?

--------

Gerrit topic: https://review.openstack.org/#q,topic:bug/1667778,n,z

Addressed by: https://review.openstack.org/444296
    WIP Option to specify MAC address of port

Gerrit topic: https://review.openstack.org/#q,topic:bp/neutron-trunk-ui,n,z

Addressed by: https://review.openstack.org/447028
    New input field with validation: MACAddressField

Addressed by: https://review.openstack.org/449217
    WIP Readonly trunkpanel

Addressed by: https://review.openstack.org/452725
    TrunkPort, Horizon workflow: launch instance, django version

Addressed by: https://review.openstack.org/454143
    WIP Trunks panel: item and batch delete

Addressed by: https://review.openstack.org/464727
    WIP: Details for trunks

Addressed by: https://review.openstack.org/468050
    WIP Idea to avoid duplication of port subtype filtering logic

Addressed by: https://review.openstack.org/468396
    WIP Trunks panel: create button

Addressed by: https://review.openstack.org/481468
    Hide forewer spinner in case of load problem

Addressed by: https://review.openstack.org/485473
    Trunks panel: edit button

Addressed by: https://review.openstack.org/487743
    Trunks panel: default to disabled

Addressed by: https://review.openstack.org/489184
    Trunks panel: delete unused part of drawer

Addressed by: https://review.openstack.org/489185
    Trunks panel: improve details' test coverage

Addressed by: https://review.openstack.org/489186
    WIP Trunks panel: default to enabled

Addressed by: https://review.openstack.org/493070
    WIP transfer-table: allow independent updates of avail

Addressed by: https://review.openstack.org/493538
    Remove initScope from trunk delete.action.service

Gerrit topic: https://review.openstack.org/#q,topic:bp/is,n,z

Addressed by: https://review.openstack.org/494540
    Update neutron policy file

Addressed by: https://review.openstack.org/516657
    Trunks admin panel

Addressed by: https://review.openstack.org/524231
    WIP Trunks panel: eliminate spinner at create/edit

Gerrit topic: https://review.openstack.org/#q,topic:bug/1681627,n,z

Addressed by: https://review.openstack.org/524619
    Trunks Panel: Add links to parent and subports

Addressed by: https://review.openstack.org/525670
    Trunks panel: display the MAC of ports

Gerrit topic: https://review.openstack.org/#q,topic:bug/1704118,n,z

Addressed by: https://review.openstack.org/513173
    Remove redirection from trunk details

Addressed by: https://review.openstack.org/532817
    Trunks panel: simplify code for easier testing

Addressed by: https://review.openstack.org/532818
    DO NOT MERGE Trunks panel: delay loading data

(?)

Work Items

This blueprint contains Public information 
Everyone can see this information.