Open Stack

On Dec 15, 2014, at 9:13 AM, Assaf Muller <amuller at redhat.com> wrote:

> 
> 
> ----- Original Message -----
>> -----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 usual you explain yourself better than I can... I think my main
> original objection to the patch is that it feels like an accidental
> API change to fix a bug. If you want unique naming:
> 1) We need to be consistent across different resources
> 2) It needs to be in a dedicate change, and perhaps blueprint
> 
> Since there's conceivable alternative solutions to the bug that aren't
> substantially more costly or complicated, I don't see why we would pursue
> the proposed approach.


+1

Regardless of the merits of security groups having unique names, I don’t think it is a change that should be slipped in as part of a bugfix.  If we want to see this kind of API-modifying change introduced in Neutron (or any other OpenStack project), there is a process that needs to be followed.



Maru


> 
>> 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-----
>> 
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>> 
> 
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev