Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parameters to configure hdb in openldap::server::database #66

Merged
merged 1 commit into from
Jul 7, 2015

Conversation

amateo
Copy link
Contributor

@amateo amateo commented May 27, 2015

I have modified openldap::server::database and openldap_database provider so you could use most of the parameters supported in hdb (and bdb) databases documented at man slapd-hdb

t << "olcDbCachesize: #{resource[:dbcachesize]}\n" if resource[:dbcachesize] and resource[:backend] =~ /^(hdb|mdb|bdb)$/
t << "olcDbCachefree: #{resource[:dbcachefree]}\n" if resource[:dbcachefree] and resource[:backend] =~ /^(hdb|mdb|bdb)$/
t << "olcDbCheckpoint: #{resource[:dbcheckpoint]}\n" if resource[:dbcheckpoint] and resource[:backend] =~ /^(hdb|mdb|bdb)$/
##t << "olcDbConfig: #{resource[:dbconfig]}\n" if resource[:dbconfig] and resource[:backend] =~ /^(hdb|mdb|bdb)$/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this commented out?

@mcanevet
Copy link
Member

@amateo great PR, could you please rebase on master to pull my recent patches for Puppet4 to see if it still works with your code?

@raphink
Copy link
Member

raphink commented May 27, 2015

I think that's a whole lot of properties added to this type. I'd rather see a dboptions property that takes a hash, since anyway convergence on these properties mean a full flush of the resource from a template generated from all properties.

@amateo
Copy link
Contributor Author

amateo commented May 27, 2015

I can change them to a hash. I don't mind. What would you prefer?

@mcanevet
Copy link
Member

@amateo I think a hash for at least for db* properties would be better and would reduce the code.

@amateo
Copy link
Contributor Author

amateo commented May 28, 2015

I'm modifying this. I have a doubt about how to handle absent options. Lets suppose you already have a database with this options:

dn: olcDatabase={4}hdb
objectClass: olcHdbConfig
olcDatabase: {4}hdb
olcDbDirectory: /var/lib/ldap/
olcSuffix: dc=mydomain,dc=com
...
olcDbCacheSize: 100000
olcDbCheckpoint: 5120 10
olcDbConfig: {0}set_cachesize 0 2097152 0
olcDbConfig: {1}set_lk_max_objects 1500
olcDbConfig: {2}set_lk_max_locks 1500
olcDbConfig: {3}set_lk_max_lockers 1500
olcDbIDLcacheSize: 300000
olcDbIndex: objectClass eq
olcDbCacheFree: 1
olcDbDNcacheSize: 0

I'm mapping all olcDbXXX attributes to a dboptions hash, like this:

openldap_database { 'dc=mydomain,dc=com':
  ensure    => 'present',
  backend   => 'hdb',
  dboptions => {'cachefree' => '1', 'cachesize' => '100000', 'checkpoint' => '5120 10', 'dbconfig' => ['{0}set_cachesize 0 2097152 0', '{1}set_lk_max_objects 1500', '{2}set_lk_max_locks 1500', '{3}set_lk_max_lockers 1500'], 'dncachesize' => '0', 'idlcachesize' => '300000', 'index' => 'objectClass eq'},
  directory => '/var/lib/ldap',
...
}

So, lets suppose that now I change my puppet code to:

openldap_database { 'dc=mydomain,dc=com':
  ensure    => 'present',
  backend   => 'hdb',
  dboptions => {'cachesize' => '100000', 'checkpoint' => '5120 10', 'dbconfig' => ['{0}set_cachesize 0 2097152 0', '{1}set_lk_max_objects 1500', '{2}set_lk_max_locks 1500', '{3}set_lk_max_lockers 1500'], 'dncachesize' => '0', 'idlcachesize' => '300000', 'index' => 'objectClass eq'},
  directory => '/var/lib/ldap',
...
}

removing the cachefree value.

What do you prefer? That the provider removes the olcDbCachefree attribute or that it leaves it? In the first case, the provider just takes care that programmed values are in the ldap entry for the object. In the second one, it takes care that they are correct and that they are the only defined in the ldap entry.

@mcanevet
Copy link
Member

@amateo you can maybe add an additional parameter that allows to choose between minimal or inclusive like what Puppetlabs do for user type : https://docs.puppetlabs.com/references/latest/type.html#user-attribute-membership

@amateo
Copy link
Contributor Author

amateo commented Jun 1, 2015

One comment about this PR: the dboptions hash parameter, accepts index as a key (since db index options is map to olcDbIndex ldif attribute. This could conflict with openldap_dbindex provider if you use synctype=inclusive.

To solve this one solution is forcing to use synctype=minimum (it is the default) if you want to use openldap_dbindex too. Another solution is excluding index from attributes mapped to dboptions, forcing to use openldap_dbindex for that purpose. Another solution could be removing openldap_index (in the same way that limits are specific to a database so they are properties of databases).

What do you think?

@mcanevet
Copy link
Member

mcanevet commented Jun 1, 2015

@amateo great PR, thanks a lot.
I think deprecate openldap_index could be a good solution.
I may not have time to review this huge PR in the next few days but maybe @raphink will.

@amateo
Copy link
Contributor Author

amateo commented Jul 7, 2015

Hello @mcanevet. Will you merge this PR?

@mcanevet
Copy link
Member

mcanevet commented Jul 7, 2015

@amateo acceptance tests pass. So let's merge this!
Thanks

mcanevet added a commit that referenced this pull request Jul 7, 2015
Add parameters to configure hdb in openldap::server::database
@mcanevet mcanevet merged commit 3e2ff01 into voxpupuli:master Jul 7, 2015
@amateo
Copy link
Contributor Author

amateo commented Jul 7, 2015

Thanks

@amateo amateo deleted the feature/database branch July 9, 2015 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants