Comment 1 for bug 919265

Revision history for this message
Madhav Puri (madhav-puri) wrote :

Hi,

I tried to debug the behavior seen here. And based on my understanding, I see two issues and hence two potential places of fixing this:

1. We are missing validation of allowed config-port-states passed to the Quantum API from a quantum-client, as seen in code snippet below.

++++++
class Controller(common.QuantumController):
    """ Port API controller for Quantum API """

...
...
   @common.APIFaultWrapper([exception.NetworkNotFound,
                             exception.StateInvalid])
    def create(self, request, tenant_id, network_id, body=None):
        """ Creates a new port for a given network
            The request body is optional for a port object.

        """
        body = self._prepare_request_body(body, self._port_ops_param_list)
        port = self._plugin.create_port(tenant_id,
                                        network_id, body['port']['state'], <<< no state validation don at API level
                                        **body)
        builder = ports_view.get_view_builder(request, self.version)
        result = builder.build(port)['port']
        return dict(port=result)

...
...
++++++

I believe we need a fix at this level if we anticipate plugins to only expect certain API prescribed states. This become a bit restrictive wrt plugin and needs more intelligence in API layer. But at same time it presents a uniform API to user irrespective of underlying plugin.

2. Looking at plugin-code (cisco and ovs) looks like the behavior might differ at plugin level as well. Like for Cisco nexus-plugin create-port API seems a no-op (this is just based on going through some code and I might be wrong). For the ovs-plugin (class OVSQuantumPlugin), if a state is passed it is used as-is otherwise 'DOWN' state is used, as seen in snippet below.

+++++
class OVSQuantumPlugin(QuantumPluginBase):
...
...
def port_create(net_id, state=None, op_status=OperationalStatus.UNKNOWN):
    # confirm network exists
    network_get(net_id)

    session = get_session()
    with session.begin():
        port = models.Port(net_id, op_status)
        port['state'] = state or 'DOWN' <<< no state validation done at plugin level here.
        session.add(port)
        session.flush()
        return port
...
...

+++++

We might need a fix at plugin level if we are to allow plugins to accept potentially different states or handle create-port request differently. In this case API just acts as a layer that passes this info to plugin and all intelligence shall reside in plugin.

Let me know in case I am missing something and took off a totally wrong path :). Or else if there are/were any relevant discussions that might have taken place on desired behavior.

Thanks.