nova.db.api.project_get_network has unused parameter.

[Ryu Ishimoto]

project_get_network method of nova.db.api module takes in 'associate' parameter but does not pass it into
IMPL.project_get_network. IMPL.project_get_network, in turn, accepts it, but since it can never get called with this parameter set to anything other than its default value, it is a useless parameter.

[Ryu Ishimoto (ryu-midokura)]

A consequence of adding 'associate' parameter to the call of IMPL.project_get_network is that, AuthManager().get_project_vpn_data() would now work the way this method was originally intended to work.

[Vish Ishaya (vishvananda)]

Vlans are a scarce resource, and the idea is to "use up" a network only when it is really needed. The associate parameter allows us to check if a project has a network without actually associating one. The only current place the associate paramater is used in AuthManager().get_project_vpn_data(), as you mentioned. This allows us to list vpns for projects without associating all of the projects to a network.

[Ryu Ishimoto (ryu-midokura)]

Yes, I agree that this parameter allows us to check if a project has a network without associating one, but the issue I found was that it IS associating a new network even when you don't want to, using up the scarce resource. AuthManager().get_project_vpn_data() should not be associating a new network to the project but it is doing so anyway because the DB API forces the associate parameter to be set to True all the time.

The call chain is:

AuthManager:
network_ref = db.project_get_network(context.get_admin_context(), Project.safe_id(project), False)
 => here, authmanager does not want to associate project and network

db.api:
def project_get_network(context, project_id, associate=True):
    return IMPL.project_get_network(context, project_id)
 => here, associate is accepted but never used in the method. This is the problematic part.

db.sqlalchemy.api:
def project_get_network(context, project_id, associate=True)
 => here, associate is accepted, and defaults to True, which means this method tries to associate project and network even if the original caller specifically told it not to.

My question was, shouldn't db.api.project_get_network pass along the associate parameter to IMPL.project_get_network?

It seems strange to me that the behavior of db.api and db.sqlalchemy.api would be different when 'associate' is set to False, and that AuthManager would pass in associate=False parameter only to be overridden to True later.

Let me know what you think! Thnx.

[Vish Ishaya (vishvananda)]

Ah I misunderstood.

This is definitely a bug. Associate should be passed in in IMPL. I'll make a fix.