Open Stack

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_.  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

> 
>>
>> 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