[openstack-dev] [puppet][keystone] Choose domain names with 'composite namevar' or 'meaningless name'?
Rich Megginson
rmeggins at redhat.com
Tue Oct 6 21:45:32 UTC 2015
On 10/06/2015 02:36 PM, Sofer Athlan-Guyot wrote:
> Rich Megginson <rmeggins at redhat.com> writes:
>
>> On 09/30/2015 11:43 AM, Sofer Athlan-Guyot wrote:
>>> Gilles Dubreuil <gilles at 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 <gilles at 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 <gilles at 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 at 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!
> Unfortunately, this cannot be done in the current state of the module,
> or at least I cannot see anymore how. The problem lies in the default
> domain behavior. We cannot discriminate between a meaningless title
> which requires a name and domain parameter and a title whose name will
> be the title and the domain will be the default domain.
>
> This example illustrates the problem:
>
> This creates "project_one" in the "default" domain, project_one is *not*
> meaningless and domain and name are *not* required:
>
> keystone_tenant { 'project_one': ensure => present }
>
> Then this is really the same resource:
>
> keystone_tenant { 'meaningless': name => 'project_one', domain => 'Default', ensure => present }
>
> but puppet cannot detect it as we calculate the default domain later in
> resource creation. Then the catalog convergences, this will fail as the
> openstack cli will detect the duplication.
>
> TLDR: To recap, it seemed it was possible to create unique key
> irrespective of the "title" of the resource.
>
> For keystone_tenant, keystone_user, and keystone_user_role this would
> have made the retrieval of needed resources more robust and simpler:
> - autorequire would always have been <name>::<domain>
> - prefetching was working
> - you would have a cascading effect were user_role wouldn't have needed
> to parse everything but delegate it to the keystone_tenant,
> keystone_user.
>
> So here's what was working :
>
> keystone_domain { 'domain_one': ensure => present }
> keystone_domain { 'domain_two': ensure => present }
> keystone_domain { 'domain_less: 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 }
>
> All those resources could be autorequired by '<name::domain>'. For
> instance this would work as well:
>
> keystone_tenant { 'meaningless': name => 'project_one', domain => 'domain_one', ensure => present }
> file {'/tmp/toto': ensure => present, require => Keystone_tenant['project_one::domain_one'] }
> file {'/tmp/tata': ensure => present, require => Keystone_tenant['meaningless'] }
>
> At the catalog compilation puppet would detect double entries:
>
> keystone_tenant { 'project_one::domain_one': ensure => present }
> keystone_tenant { 'meaningless': name => 'project_one', domain => 'domain_one', ensure => present }
>
> would fails with a "Cannot alias ..."
>
> But after digging into user_role and user resources I stumble into the
> default domain behavior problem. I tried this trick to be able to get
> the default domain at the first catalog compilation phase:
>
> def self.title_patterns
> default_domain = ->(_){ provider(:openstack).default_domain }
> [
> [
> /^(.+)::(.+)$/,
> [
> [:name],
> [:domain]
> ]
> ],
> [
> /^(.+)$/,
> [
> [:name],
> [:domain, default_domain]
> ]
> ]
> ]
> end
>
> But this fails as well when openstack is not yet installed.
> self.title_patterns arrives way too early for any resource to have been
> created, let alone a full keystone installation.
It fails because default_domain will
1) look up default_domain_id from /etc/keystone/keystone.conf, which
will fail if that file does not exist
2) look up the domain from the id using `openstack domain list`
However, it will work if you require the use of the hardcoded string
'Default'.
So, it will work if we say "You must always specify the domain except if
the domain is 'Default'". This is different from the current proposal,
which says "You must always specify the domain except if the domain is
the default_domain_id set in /etc/keystone/keystone.conf".
Is this an acceptable restriction? I would think yes, since
- If you don't care about domains, the default is going to be 'Default'
- If you do care about domains, you're going to have to use domains in a
lot of places anyway.
>
> In the end, I think that this is a blocker and becomes way too hackish.
>
> Ref:
> - https://review.openstack.org/#/c/226919/
> - https://tickets.puppetlabs.com/browse/PUP-5302
>
>
>>>>> 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 at 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 => 'test at Default.com',
>>>> enabled => 'true',
>>>> id => 'fb56d86a21f54b09aa435b96fd321eee',
>>>> }
>>>> keystone_user { 'user1::domain_B':
>>>> ensure => 'present',
>>>> domain_id => '79beff022efd4011b9a036155f450af8',
>>>> email => 'user1 at domain_B.com',
>>>> enabled => 'true',
>>>> id => '2174faac46f949fca44e2edab3d53675',
>>>> }
>>>> keystone_user { 'user1::domain_A':
>>>> ensure => 'present',
>>>> domain_id => '9387210938a0ef1b3c843feee8a00a34',
>>>> email => 'user1 at 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 ???
>>
>>>>>> 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-request at 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-request at 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-request at 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-request at lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
More information about the OpenStack-dev
mailing list