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

autorequire(:keystone_tenant) do
   self[:tenant]
end

Not exactly.  See https://review.openstack.org/#/c/226919/

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



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

Reply via email to