advice on module/class refactoring for defines

[Tim Mooney]

All-

We have several modules that have defines within them that the style guide
(sections 11.1 & 11.4) and puppet-lint suggest should be in their own
file. I'm working on fixing them and have a question about how things
should be refactored.

A stripped down example:

The node:

node 'foo.bar.edu' {
include oracledb::instantclient
}


file modules/oracledb/manifests/instantclient.pp:

class oracledb::instantclient($version='11.1') {

include ldconfig

# other non-relevant stuff here

ldconfig::config { 'oracle-instantclient':
content => template('oracledb/instantclient.ld.conf.erb'),
}

}


file modules/ldconfig/manifests/init.pp:

class ldconfig {

exec { '/sbin/ldconfig':
refreshonly => true,
alias => 'ldconfig',
}


define config($content) {
file { "/etc/ld.so.conf.d/${name}.conf":
ensure => file,
owner => 'root',
group => 'root',
mode => '0644',
content => $content,
notify => Exec['ldconfig'],
}
}
}


Note the define for config() within the init.pp for ldconfig. That's now
out of favor. Moving that define into it's own file in
modules/ldconfig/manifests/config.pp is straightforward, but it leaves
me with questions about where the exec should be.

- if I add a "requires => Class['ldconfig']" to the file within the
config() define, I now have a circular dependency that puppet errors on.

- if I don't include the "requires => Class['ldconfig']" but I'm careful
to keep the "include ldconfig" in every class that also uses
ldconfig::config, things "work", but that seems like the wrong way to
do it, as it only works if I do both things, because of the implicit
dependency on the exec.

- I've tried moving the exec inside the define, like this:

file modules/ldconfig/manifests/config.pp:

define ldconfig::config($content) {

exec { '/sbin/ldconfig':
refreshonly => true,
alias => 'ldconfig',
}

file { "/etc/ld.so.conf.d/${name}.conf":
ensure => file,
owner => 'root',
group => 'root',
mode => '0644',
content => $content,
notify => Exec['ldconfig'],
}
}

but that results in duplicate definitions of the exec:

err: Could not retrieve catalog from remote server: Error 400 on SERVER:
Duplicate declaration: Exec[/sbin/ldconfig] is already declared in file
/etc/puppet/modules/ldconfig/manifests/config.pp at line 6; cannot
redeclare at /etc/puppet/modules/ldconfig/manifests/config.pp:6 on node
foo.bar.edu

- I've considered giving the exec an alias that is unique per define,
e.g. something like

exec { '/sbin/ldconfig':
refreshonly => true,
alias => "ldconfig-${name}",
}

and then doing a 'notify => Exec["ldconfig-${name}"]', but that too
seems pretty hackish, though it may be my fall-back position if
someone doesn't have a more elegant way to handle this.

Any thoughts on how this should be re-organized?

Thanks,

Tim
--
Tim Mooney Tim.M...@ndsu.edu
Enterprise Computing & Infrastructure 701-231-1076 (Voice)
Room 242-J6, IACC Building 701-231-8541 (Fax)
North Dakota State University, Fargo, ND 58105-5164

[Nan Liu]

I would say include 'ldconfig' in define ldconfig::config, then the
define is self contained, and you don't need remember include ldconfig
everywhere.

Thanks,

Nan

[jcbollinger]

On May 24, 7:43 pm, Nan Liu <n...@puppetlabs.com> wrote:
> I would say include 'ldconfig' in define ldconfig::config, then the
> define is self contained, and you don't need remember include ldconfig
> everywhere.

+1

That's all around the best available option. It's much nicer than the
original version, even, because users don't have to separately include
class 'ldconfig' before they can use the define.


John

[Tim Mooney]

In regard to: [Puppet Users] Re: advice on module/class refactoring for...:

Agreed, that's something I was hoping to avoid.

[Tim Mooney]

In regard to: Re: [Puppet Users] advice on module/class refactoring for...:

>> Note the define for config() within the init.pp for ldconfig. That's now
>> out of favor. Moving that define into it's own file in
>> modules/ldconfig/manifests/config.pp is straightforward, but it leaves
>> me with questions about where the exec should be.
>

> I would say include 'ldconfig' in define ldconfig::config, then the
> define is self contained, and you don't need remember include ldconfig
> everywhere.

Thanks Nan. Not certain why that wasn't obvious to me before you pointed
it out, but it's perfect. :-)