Rich Megginson <rmegg...@redhat.com> writes:
> On 09/30/2015 11:43 AM, Sofer Athlan-Guyot wrote:
>> Gilles Dubreuil <gil...@redhat.com> writes:
>>
>>> On 30/09/15 03:43, Rich Megginson wrote:
>>>> On 09/28/2015 10:18 PM, Gilles Dubreuil wrote:
>>>>> On 15/09/15 19:55, Sofer Athlan-Guyot wrote:
>>>>>> Gilles Dubreuil <gil...@redhat.com> writes:
>>>>>>
>>>>>>> On 15/09/15 06:53, Rich Megginson wrote:
>>>>>>>> On 09/14/2015 02:30 PM, Sofer Athlan-Guyot wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Gilles Dubreuil <gil...@redhat.com> writes:
>>>>>>>>>
>>>>>>>>>> A. The 'composite namevar' approach:
>>>>>>>>>>
>>>>>>>>>> keystone_tenant {'projectX::domainY': ... }
>>>>>>>>>> B. The 'meaningless name' approach:
>>>>>>>>>>
>>>>>>>>>> keystone_tenant {'myproject': name='projectX',
>>>>>>>>>> domain=>'domainY',
>>>>>>>>>> ...}
>>>>>>>>>>
>>>>>>>>>> Notes:
>>>>>>>>>> - Actually using both combined should work too with the domain
>>>>>>>>>> supposedly overriding the name part of the domain.
>>>>>>>>>> - Please look at [1] this for some background between the two
>>>>>>>>>> approaches:
>>>>>>>>>>
>>>>>>>>>> The question
>>>>>>>>>> -------------
>>>>>>>>>> Decide between the two approaches, the one we would like to
>>>>>>>>>> retain for
>>>>>>>>>> puppet-keystone.
>>>>>>>>>>
>>>>>>>>>> Why it matters?
>>>>>>>>>> ---------------
>>>>>>>>>> 1. Domain names are mandatory in every user, group or project.
>>>>>>>>>> Besides
>>>>>>>>>> the backward compatibility period mentioned earlier, where no domain
>>>>>>>>>> means using the default one.
>>>>>>>>>> 2. Long term impact
>>>>>>>>>> 3. Both approaches are not completely equivalent which different
>>>>>>>>>> consequences on the future usage.
>>>>>>>>> I can't see why they couldn't be equivalent, but I may be missing
>>>>>>>>> something here.
>>>>>>>> I think we could support both. I don't see it as an either/or
>>>>>>>> situation.
>>>>>>>>
>>>>>>>>>> 4. Being consistent
>>>>>>>>>> 5. Therefore the community to decide
>>>>>>>>>>
>>>>>>>>>> Pros/Cons
>>>>>>>>>> ----------
>>>>>>>>>> A.
>>>>>>>>> I think it's the B: meaningless approach here.
>>>>>>>>>
>>>>>>>>>> Pros
>>>>>>>>>> - Easier names
>>>>>>>>> That's subjective, creating unique and meaningful name don't look
>>>>>>>>> easy
>>>>>>>>> to me.
>>>>>>>> The point is that this allows choice - maybe the user already has some
>>>>>>>> naming scheme, or wants to use a more "natural" meaningful name -
>>>>>>>> rather
>>>>>>>> than being forced into a possibly "awkward" naming scheme with "::"
>>>>>>>>
>>>>>>>> keystone_user { 'heat domain admin user':
>>>>>>>> name => 'admin',
>>>>>>>> domain => 'HeatDomain',
>>>>>>>> ...
>>>>>>>> }
>>>>>>>>
>>>>>>>> keystone_user_role {'heat domain admin user@::HeatDomain':
>>>>>>>> roles => ['admin']
>>>>>>>> ...
>>>>>>>> }
>>>>>>>>
>>>>>>>>>> Cons
>>>>>>>>>> - Titles have no meaning!
>>>>>>>> They have meaning to the user, not necessarily to Puppet.
>>>>>>>>
>>>>>>>>>> - Cases where 2 or more resources could exists
>>>>>>>> This seems to be the hardest part - I still cannot figure out how
>>>>>>>> to use
>>>>>>>> "compound" names with Puppet.
>>>>>>>>
>>>>>>>>>> - More difficult to debug
>>>>>>>> More difficult than it is already? :P
>>>>>>>>
>>>>>>>>>> - Titles mismatch when listing the resources (self.instances)
>>>>>>>>>>
>>>>>>>>>> B.
>>>>>>>>>> Pros
>>>>>>>>>> - Unique titles guaranteed
>>>>>>>>>> - No ambiguity between resource found and their title
>>>>>>>>>> Cons
>>>>>>>>>> - More complicated titles
>>>>>>>>>> My vote
>>>>>>>>>> --------
>>>>>>>>>> I would love to have the approach A for easier name.
>>>>>>>>>> But I've seen the challenge of maintaining the providers behind the
>>>>>>>>>> curtains and the confusion it creates with name/titles and when
>>>>>>>>>> not sure
>>>>>>>>>> about the domain we're dealing with.
>>>>>>>>>> Also I believe that supporting self.instances consistently with
>>>>>>>>>> meaningful name is saner.
>>>>>>>>>> Therefore I vote B
>>>>>>>>> +1 for B.
>>>>>>>>>
>>>>>>>>> My view is that this should be the advertised way, but the other
>>>>>>>>> method
>>>>>>>>> (meaningless) should be there if the user need it.
>>>>>>>>>
>>>>>>>>> So as far as I'm concerned the two idioms should co-exist. This
>>>>>>>>> would
>>>>>>>>> mimic what is possible with all puppet resources. For instance
>>>>>>>>> you can:
>>>>>>>>>
>>>>>>>>> file { '/tmp/foo.bar': ensure => present }
>>>>>>>>>
>>>>>>>>> and you can
>>>>>>>>>
>>>>>>>>> file { 'meaningless_id': name => '/tmp/foo.bar', ensure =>
>>>>>>>>> present }
>>>>>>>>>
>>>>>>>>> The two refer to the same resource.
>>>>>>>> Right.
>>>>>>>>
>>>>>>> I disagree, using the name for the title is not creating a composite
>>>>>>> name. The latter requires adding at least another parameter to be part
>>>>>>> of the title.
>>>>>>>
>>>>>>> Also in the case of the file resource, a path/filename is a unique
>>>>>>> name,
>>>>>>> which is not the case of an Openstack user which might exist in several
>>>>>>> domains.
>>>>>>>
>>>>>>> I actually added the meaningful name case in:
>>>>>>> http://lists.openstack.org/pipermail/openstack-dev/2015-September/074325.html
>>>>>>>
>>>>>>>
>>>>>>> But that doesn't work very well because without adding the domain to
>>>>>>> the
>>>>>>> name, the following fails:
>>>>>>>
>>>>>>> keystone_tenant {'project_1': domain => 'domain_A', ...}
>>>>>>> keystone_tenant {'project_1': domain => 'domain_B', ...}
>>>>>>>
>>>>>>> And adding the domain makes it a de-facto 'composite name'.
>>>>>> I agree that my example is not similar to what the keystone provider has
>>>>>> to do. What I wanted to point out is that user in puppet should be used
>>>>>> to have this kind of *interface*, one where your put something
>>>>>> meaningful in the title and one where you put something meaningless.
>>>>>> The fact that the meaningful one is a compound one shouldn't matter to
>>>>>> the user.
>>>>>>
>>>>> There is a big blocker of making use of domain name as parameter.
>>>>> The issue is the limitation of autorequire.
>>>>>
>>>>> Because autorequire doesn't support any parameter other than the
>>>>> resource type and expects the resource title (or a list of) [1].
>>>>>
>>>>> So for instance, keystone_user requires the tenant project1 from
>>>>> domain1, then the resource name must be 'project1::domain1' because
>>>>> otherwise there is no way to specify 'domain1':
>>>>>
>>> Yeah, I kept forgetting this is only about resource relationship/order
>>> within a given catalog.
>>> And therefore this is *not* about guaranteeing referred resources exist,
>>> for instance when created (or not) in a different puppet run/catalog.
>>>
>>> This might be obvious but it's easy (at least for me) to forget that
>>> when thinking of the resources list, in terms of openstack IDs for
>>> example inside self.instances!
>>>
>>>>> autorequire(:keystone_tenant) do
>>>>> self[:tenant]
>>>>> end
>>>> Not exactly. See https://review.openstack.org/#/c/226919/
>>>>
>>> That's nice and makes the implementation easier.
>>> Thanks.
>>>
>>>> For example::
>>>>
>>>> keystone_tenant {'some random tenant':
>>>> name => 'project1',
>>>> domain => 'domain1'
>>>> }
>>>> keystone_user {'some random user':
>>>> name => 'user1',
>>>> domain => 'domain1'
>>>> }
>>>>
>>>> How does keystone_user_role need to be declared such that the
>>>> autorequire for keystone_user and keystone_tenant work?
>>>>
>>>> keystone_user_role {'some random user@some random tenant': ...}
>>>>
>>>> In this case, I'm assuming this will work
>>>>
>>>> autorequire(:keystone_user) do
>>>> self[:name].rpartition('@').first
>>>> end
>>>> autorequire(:keystone_user) do
>>>> self[:name].rpartition('@').last
>>>> end
>>>>
>>>> The keystone_user require will be on 'some random user' and the
>>>> keystone_tenant require will be on 'some random tenant'.
>>>>
>>>> So it should work, but _you have to be absolutely consistent in using
>>>> the title everywhere_.
>> Ok, so it seems I found a puppet pattern that could enable us to not
>> depend on the particular syntax used on the title to retrieve the
>> resource. If one use "isnamevar" on multiple parameters, then using
>> "uniqueness_key" on the resource enable us to retrieve the resource in
>> the catalog, whatever the title of the resource is.
>>
>> I have a working example in this change
>> https://review.openstack.org/#/c/226919/ for keystone_tenant with name,
>> and domain as the keys. All of the following work and can be easily
>> retrieved using [domain, keys]
>>
>> keystone_domain { 'domain_one': ensure => present }
>> keystone_domain { 'domain_two': ensure => present }
>> keystone_tenant { 'project_one::domain_one': ensure => present }
>> keystone_tenant { 'project_one::domain_two': ensure => present }
>> keystone_tenant { 'meaningless_title_one': name => 'project_less', domain
>> => 'domain_one', ensure => present }
>>
>> This will raise a error:
>>
>> keystone_tenant { 'project_one::domain_two': ensure => present }
>> keystone_tenant { 'meaningless_title_one': name => 'project_one', domain
>> => 'domain_two', ensure => present }
>>
>> As puppet will correctly find that they are the same resource.
>
> Great!
>
>>
>>>> That is, once you have chosen to give something
>>>> a title, you must use that title everywhere: in autorequires (as
>>>> described above), in resource references (e.g. Keystone_user['some
>>>> random user'] ~> Service['myservice']), and anywhere the resource will
>>>> be referenced by its title.
>>>>
>>> Yes the title must the same everywhere it's used but only within a given
>>> catalog.
>>>
>>> No matter how the dependent resources are named/titled as long as they
>>> provide the necessary resources.
>>>
>>> For instance, given the following resources:
>>>
>>> keystone_user {'first user': name => 'user1', domain => 'domain_A', ...}
>>> keystone_user {'user1::domain_B': ...}
>>> keystone_user {'user1': ...} # Default domain
>>> keystone_project {'project1::domain_A': ...}
>>> keystone_project {'project1': ...} # Default domain
>>>
>>> And their respective titles:
>>> 'first user'
>>> 'user1::domain_B'
>>> 'user1'
>>> 'project1::domain_A'
>>> 'project1'
>>>
>>> Then another resource to use them, let's say keystone_user_role.
>>> Using those unique titles one should be able to do things like these:
>>>
>>> keystone_user_role {'first user@project1::domain_A':
>>> roles => ['role1]
>>> }
>>>
>>> keystone_user_role {'admin role for user1':
>>> user => 'user1'
>>> project => 'project1'
>>> roles => ['admin'] }
>>>
>>> That's look cool but the drawback is the names are different when
>>> listing. That's expected since we're allowing meaningless titles.
>>>
>>> $ puppet resource keystone_user
>>>
>>> keystone_user { 'user1::Default':
>>> ensure => 'present',
>>> domain_id => 'default',
>>> email => 't...@default.com',
>>> enabled => 'true',
>>> id => 'fb56d86a21f54b09aa435b96fd321eee',
>>> }
>>> keystone_user { 'user1::domain_B':
>>> ensure => 'present',
>>> domain_id => '79beff022efd4011b9a036155f450af8',
>>> email => 'user1@domain_B.com',
>>> enabled => 'true',
>>> id => '2174faac46f949fca44e2edab3d53675',
>>> }
>>> keystone_user { 'user1::domain_A':
>>> ensure => 'present',
>>> domain_id => '9387210938a0ef1b3c843feee8a00a34',
>>> email => 'user1@domain_A.com',
>>> enabled => 'true',
>>> id => '1bfadcff825e4c188e8e4eb6ce9a2ff5',
>>> }
>>>
>>> Note: I changed the domain field to domain_id because it makes more
>>> sense here
>>>
>>> This is fine as long as when running any catalog, a same resource with a
>>> different name but same parameters means the same resource.
>>>
>>> If everyone agrees with such behavior, then we might be good to go.
>>>
>>> The exceptions must be addressed on a per case basis.
>>> Effectively, there are cases in Openstack where several objects with the
>>> exact same parameters can co-exist, for instance with the trust (See
>>> commit message in [1] for examples). In the trust case running the same
>>> catalog over and over will keep adding the resource (not really
>>> idempotent!). I've actually re-raised the issue with Keystone developers
>>> [2].
>>>
>>> [1] https://review.openstack.org/200996
>>> [2] https://bugs.launchpad.net/keystone/+bug/1475091
>>>
>> For the keystone_tenant resource name, and domain are isnamevar
>> parameters. Using "uniqueness_key" method we get the always unique,
>> always the same, couple [<domain>, <name>], then, when we have found the
>> resource we can associate it in prefetch[10] and in autorequire without
>> any problem. So if we create a unique key by using isnamevar on the
>> required parameters for each resource that need it then we get rid of
>> the dependence on the title to retrieve the resource.
>>
>> Example of resource that should have a composite key:
>> - keystone_user: name and domain should be isnamevar. Then all the
>> question about the parsing of title would go away with robust key
>> finding.
>>
>> - user_role with username, user_domain_name, project_name,
>> project_domain_name, domain as its elements.
>>
>> When any of the keys are not filled they default to nil. Nil for domain
>> would be associated to default_domain.
>>
>> The point is to go away from "title parsing" to "composite key
>> matching". I'm quite sure it would simplify the code in a lot of
>> places and solve the concerns raised here.
>
> How does resource naming work at the Puppet manifest level? For example:
>
> keystone_user {'some user':
> name => 'someuser',
> domain => 'domain',
> }
>
> Would I use
>
> some_resource { 'name':
> requires => Keystone_user['some user'],
> }
>
> or ???
Well, it's very consistent. Here what I have tested. Given you have
that in you manifest:
keystone_tenant { 'meaningless_title_one':
name => 'project_one',
domain => 'domain_two',
ensure => present
}
Then you can either:
file { '/tmp/test':
require => Keystone_tenant['meaningless_title_one'],
ensure => present
}
Or:
file { '/tmp/test':
require => Keystone_tenant['project_one::domain_two'],
ensure => present
}
Which is very, very nice.
>
>>
>>>>> Alternatively, as Sofer suggested (in a discussion we had), we could
>>>>> poke the catalog to retrieve the corresponding resource(s).
>>>> That is another question I posed in
>>>> https://review.openstack.org/#/c/226919/:
>>>>
>>>> I guess we can look up the user resource and tenant resource from the
>>>> catalog based on the title? e.g.
>>>>
>>>> user = puppet.catalog.resource.find(:keystone_user, 'some random
>>>> user')
>>>> userid = user[:id]
>>>>
>>>>> Unfortunately, unless there is a way around, that doesn't work because
>>>>> no matter what autorequire wants a title.
>>>> Which I think we can provide.
>>>>
>>>> The other tricky parts will be self.instances and self.prefetch.
>>>>
>>>> I think self.instances can continue to use the 'name::domain' naming
>>>> convention, since it needs some way to create a unique title for all
>>>> resources.
>>>>
>>>> The real work will be in self.prefetch, which will need to compare all
>>>> of the parameters/properties to see if a resource declared in a manifest
>>>> matches exactly a resource found in Keystone. In this case, we may have
>>>> to 'rename' the resource returned by self.instances to make it match the
>>>> one from the manifest so that autorequires and resource references
>>>> continue to work.
>>>>
>>>>>
>>>>> So it seems for the scoped domain resources, we have to stick together
>>>>> the name and domain: '<name>::<domain>'.
>>>>>
>>>>> [1]
>>>>> https://github.com/puppetlabs/puppet/blob/master/lib/puppet/type.rb#L2003
>>>>>
>>>>>>>>> But, If that's indeed not possible to have them both,
>>>>>>> There are cases where having both won't be possible like the trusts,
>>>>>>> but
>>>>>>> why not for the resources supporting it.
>>>>>>>
>>>>>>> That said, I think we need to make a choice, at least to get
>>>>>>> started, to
>>>>>>> have something working, consistently, besides exceptions. Other options
>>>>>>> to be added later.
>>>>>> So we should go we the meaningful one first for consistency, I think.
>>>>>>
>>>>>>>>> then I would keep only the meaningful name.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> As a side note, someone raised an issue about the delimiter being
>>>>>>>>> hardcoded to "::". This could be a property of the resource. This
>>>>>>>>> would enable the user to use weird name with "::" in it and assign
>>>>>>>>> a "/"
>>>>>>>>> (for instance) to the delimiter property:
>>>>>>>>>
>>>>>>>>> Keystone_tenant { 'foo::blah/bar::is::cool': delimiter => "/",
>>>>>>>>> ... }
>>>>>>>>>
>>>>>>>>> bar::is::cool is the name of the domain and foo::blah is the project.
>>>>>>>> That's a good idea. Please file a bug for that.
>>>>>>>>
>>>>>>>>>> Finally
>>>>>>>>>> ------
>>>>>>>>>> Thanks for reading that far!
>>>>>>>>>> To choose, please provide feedback with more pros/cons, examples and
>>>>>>>>>> your vote.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Gilles
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> PS:
>>>>>>>>>> [1] https://groups.google.com/forum/#!topic/puppet-dev/CVYwvHnPSMc
>>>>>>>>>>
>>>>> __________________________________________________________________________
>>>>>
>>>>> OpenStack Development Mailing List (not for usage questions)
>>>>> Unsubscribe:
>>>>> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>>
>>>> __________________________________________________________________________
>>>> OpenStack Development Mailing List (not for usage questions)
>>>> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>> __________________________________________________________________________
>>> OpenStack Development Mailing List (not for usage questions)
>>> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>> [10]: there is a problem in puppet with the way it handle prefetch and
>> composite namevar. I still have to open the bug though. In the
>> meantime you will find in
>> https://review.openstack.org/#/c/226919/5/lib/puppet/provider/keystone_tenant/openstack.rb
>> a idiom that works.
>>
>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
--
Sofer Athlan-Guyot
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
