osapi_compute_unique_server_name_scope is a strange little option

[Mark McLoughlin]

See https://review.openstack.org/19174

osapi_compute_unique_server_name_scope sounds like its a detail of the openstack API layer, but the option and its usage lives in the DB layer

It would be nice if we could figure out some way of moving it into the API layer

[David Ripton (dripton)]

Is this fixed? The above review was merged.

[Mark McLoughlin (markmc)]

The review was only to move the option definition into the DB layer

The point is that it belongs in the API layer

So, no - it's not fixed

[Jiajun Liu (ljjjustin)]

how about move this option into api/openstack/common.py where defined some global options for osapi.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/33711

[Anthony Dodd (thedodd)]

Patch #2 is ready for review.

[Anthony Dodd (thedodd)]

@MarkMcLoughlin I'm concerned that I may not have addressed your 4th inline comment in api.py from patch #1.

I traced through the code, and it looks like '_metadata_refs' is used in 'instance_create' which is decorated by 'require_context' which essentially validates user/admin context. Perhaps you could explain to me how it is that '_metadata_refs' is dependent upon the presence of 'osapi_compute_unique_server_name_scope'. You mentioned something about having an argument passed into said function?

If anything, it looks like the function ('_validate_unique_server_name') immediately below your inline comment #4, is the function which is dependent upon the 'osapi_compute_unique_server_name_scope' config.

'_metadata_refs' is obviously used in 'instance_create' which is reliant upon '_validate_unique_server_name' which directly utilizes the 'osapi_compute_unique_server_name_scope' option, but I do not see any need for a new argument to be passed into the '_metadata_refs' function.

I do not mean to be confrontational, of course. I ask out of genuine intention to learn and contribute. Thanks in advance.

[Anthony Dodd (thedodd)]

I remain to be at somewhat of a loss in regards to everything that needs to be done here. I will summarize what has been said on the Gerrit review pages, as well as this page, and hopefully we will be able to achive some clarity as to what all needs to take place here.

1. The osapi_compute_unique_server_name_scope option should be moved out of the db layer where it currently resides, and into the api layer. Preferably into the nova.api.openstack.common module (as mentioned above by Jiajun Liu).
• Because oslo.config options should exist in the modules whithin which they are used, the _validate_unique_server_name function – which is the only function that directly uses said option – needs to be moved to the api layer as well, and more specifically to the same module as said option.

# This is where things become unclear for me.
2. Because the _validate_unique_server_name function is used in the db layer (in the instance_create() function, located in nova.db.sqlalchemy.api), "the db layer should expose some way (e.g. function arguments) for the api layer to request this behaviour" ... that is the osapi_compute_unique_server_name_scope behavior.
• It has been sugested that "we could add a unique_server_name kwarg to instance_create() and then in nova.api.openstack.compute.servers we'd pass the value of osapi_compute_unique_server_name_scope to compute_api.create()".

Point #1 and its subpoint are clear enough, but I could use some input as far as how to implement point #2. In particular, I do not see the connection between adding a "unique_server_name" kwarg to instance_create() – which resides in nova.db.sqlalchemy.api – and "passing the value of osapi_compute_unique_server_name_scope to compute_api.create()" – which resides in nova.api.openstack.compute.servers.

Take a look at the code here: https://review.openstack.org/#/c/33711/. We are on patch #2.

[Sean Dague (sdague)]

The constraint checking kind of needs to be in the db layer because that's the only place we have access to it. Possibly a future clean up. But I think Opinion marks the current real state.