Open Stack

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

I was (rightfully) asked to share my comments on the matter that I
left in gerrit here. See below.

On 12/12/14 22:40, Sean Dague wrote:
> On 12/12/2014 01:05 PM, Maru Newby wrote:
>> 
>> On Dec 11, 2014, at 2:27 PM, Sean Dague <sean at dague.net> wrote:
>> 
>>> On 12/11/2014 04:16 PM, Jay Pipes wrote:
>>>> On 12/11/2014 04:07 PM, Vishvananda Ishaya wrote:
>>>>> On Dec 11, 2014, at 1:04 PM, Jay Pipes <jaypipes at gmail.com>
>>>>> wrote:
>>>>>> On 12/11/2014 04:01 PM, Vishvananda Ishaya wrote:
>>>>>>> 
>>>>>>> On Dec 11, 2014, at 8:00 AM, Henry Gessau
>>>>>>> <gessau at cisco.com> wrote:
>>>>>>> 
>>>>>>>> On Thu, Dec 11, 2014, Mark McClain <mark at mcclain.xyz>
>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>>> On Dec 11, 2014, at 8:43 AM, Jay Pipes
>>>>>>>>>> <jaypipes at gmail.com <mailto:jaypipes at gmail.com>>
>>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>> I'm generally in favor of making name attributes
>>>>>>>>>> opaque, utf-8 strings that are entirely
>>>>>>>>>> user-defined and have no constraints on them. I 
>>>>>>>>>> consider the name to be just a tag that the user
>>>>>>>>>> places on some resource. It is the resource's ID
>>>>>>>>>> that is unique.
>>>>>>>>>> 
>>>>>>>>>> I do realize that Nova takes a different approach
>>>>>>>>>> to *some* resources, including the security group
>>>>>>>>>> name.
>>>>>>>>>> 
>>>>>>>>>> End of the day, it's probably just a personal
>>>>>>>>>> preference whether names should be unique to a
>>>>>>>>>> tenant/user or not.
>>>>>>>>>> 
>>>>>>>>>> Maru had asked me my opinion on whether names
>>>>>>>>>> should be unique and I answered my personal
>>>>>>>>>> opinion that no, they should not be, and if 
>>>>>>>>>> Neutron needed to ensure that there was one and
>>>>>>>>>> only one default security group for a tenant,
>>>>>>>>>> that a way to accomplish such a thing in a
>>>>>>>>>> race-free way, without use of SELECT FOR UPDATE,
>>>>>>>>>> was to use the approach I put into the pastebin
>>>>>>>>>> on the review above.
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> I agree with Jay.  We should not care about how a
>>>>>>>>> user names the resource. There other ways to
>>>>>>>>> prevent this race and Jay’s suggestion is a good
>>>>>>>>> one.
>>>>>>>> 
>>>>>>>> However we should open a bug against Horizon because
>>>>>>>> the user experience there is terrible with duplicate
>>>>>>>> security group names.
>>>>>>> 
>>>>>>> The reason security group names are unique is that the
>>>>>>> ec2 api supports source rule specifications by
>>>>>>> tenant_id (user_id in amazon) and name, so not
>>>>>>> enforcing uniqueness means that invocation in the ec2
>>>>>>> api will either fail or be non-deterministic in some
>>>>>>> way.
>>>>>> 
>>>>>> So we should couple our API evolution to EC2 API then?
>>>>>> 
>>>>>> -jay
>>>>> 
>>>>> No I was just pointing out the historical reason for
>>>>> uniqueness, and hopefully encouraging someone to find the
>>>>> best behavior for the ec2 api if we are going to keep the
>>>>> incompatibility there. Also I personally feel the ux is 
>>>>> better with unique names, but it is only a slight
>>>>> preference.
>>>> 
>>>> Sorry for snapping, you made a fair point.
>>> 
>>> Yeh, honestly, I agree with Vish. I do feel that the UX of
>>> that constraint is useful. Otherwise you get into having to
>>> show people UUIDs in a lot more places. While those are good
>>> for consistency, they are kind of terrible to show to people.
>> 
>> While there is a good case for the UX of unique names - it also
>> makes orchestration via tools like puppet a heck of a lot simpler
>> - the fact is that most OpenStack resources do not require unique
>> names.  That being the case, why would we want security groups to
>> deviate from this convention?
> 
> Maybe the other ones are the broken ones?
> 
> Honestly, any sanely usable system makes names unique inside a 
> container. Like files in a directory.

Correct. Or take git: it does not use hashes to identify objects, right?

> In this case the tenant is the container, which makes sense.
> 
> It is one of many places that OpenStack is not consistent. But I'd 
> rather make things consistent and more usable than consistent and
> less.

Are we only proposing to make security group name unique? I assume
that, since that's what we currently have in review. The change would
make API *more* inconsistent, not less, since other objects still use
uuid for identification.

You may say that we should move *all* neutron objects to the new
identification system by name. But what's the real benefit?

If there are problems in UX (client, horizon, ...), we should fix the
view and not data models used. If we decide we want users to avoid
using objects with the same names, fine, let's add warnings in UI
(probably with an option to disable it so that we don't push the
validation into their throats).

Finally, I have concern about us changing user visible object
attributes like names during db migrations, as it's proposed in the
patch discussed here. I think such behaviour can be quite unexpected
for some users, if not breaking their workflow and/or scripts.

My belief is that responsible upstream does not apply ad-hoc changes
to API to fix a race condition that is easily solvable in other ways
(see Assaf's proposal to introduce a new DefaultSecurityGroups table
in patchset 12 comments).

As for the whole object identification scheme change, for this to
work, it probably needs a spec and a long discussion on any possible
complications (and benefits) when applying a change like that.

For reference and convenience of readers, leaving the link to the
patch below: https://review.openstack.org/#/c/135006/



> 
> -Sean
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.22 (Darwin)

iQEcBAEBCgAGBQJUjxI1AAoJEC5aWaUY1u579M8H/RC+M7/9YYDClWRjhQLBNqEq
0pMxURZi8lTyDmi+cXA7wq1QzgOwUqWnJnOMYzq8nt9wLh8cgasjU5YXZokrqDLw
zEu/a1Cd9Alp4iGYQ6upw94BptGrMvk+XwTedMX9zMLf0od1k8Gzp5xYH/GXInN3
E+wje40Huia0MmLu4i2GMr/13gD2aYhMeGxZtDMcxQsF0DBh0gy8OM9pfKgIiXVM
T65nFbXUY1/PuAdzYwMto5leuWZH03YIddXlzkQbcZoH4PGgNEE3eKl1ctQSMGoo
bz3l522VimQvVnP/XiM6xBjFqsnPM5Tc7Ylu942l+NfpfcAM5QB6Ihvw5kQI0uw=
=gIsu
-----END PGP SIGNATURE-----