----- 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 <s...@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 <jaypi...@gmail.com> > >>>>> wrote: > >>>>>> On 12/11/2014 04:01 PM, Vishvananda Ishaya wrote: > >>>>>>> > >>>>>>> On Dec 11, 2014, at 8:00 AM, Henry Gessau > >>>>>>> <ges...@cisco.com> wrote: > >>>>>>> > >>>>>>>> On Thu, Dec 11, 2014, Mark McClain <m...@mcclain.xyz> > >>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>>> On Dec 11, 2014, at 8:43 AM, Jay Pipes > >>>>>>>>>> <jaypi...@gmail.com <mailto:jaypi...@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. > 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@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
