On Dec 15, 2014, at 9:13 AM, Assaf Muller <amul...@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 <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. +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@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 _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
