Tim Mooney
2012-May-24 20:53 UTC
[Puppet Users] advice on module/class refactoring for defines
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.Mooney@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 -- You received this message because you are subscribed to the Google Groups "Puppet Users" group. To post to this group, send email to puppet-users@googlegroups.com. To unsubscribe from this group, send email to puppet-users+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/puppet-users?hl=en.
Nan Liu
2012-May-25 00:43 UTC
Re: [Puppet Users] advice on module/class refactoring for defines
On Thu, May 24, 2012 at 1:53 PM, Tim Mooney <Tim.Mooney@ndsu.edu> wrote:> > 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 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 -- You received this message because you are subscribed to the Google Groups "Puppet Users" group. To post to this group, send email to puppet-users@googlegroups.com. To unsubscribe from this group, send email to puppet-users+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/puppet-users?hl=en.
jcbollinger
2012-May-25 12:50 UTC
[Puppet Users] Re: advice on module/class refactoring for defines
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 -- You received this message because you are subscribed to the Google Groups "Puppet Users" group. To post to this group, send email to puppet-users@googlegroups.com. To unsubscribe from this group, send email to puppet-users+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/puppet-users?hl=en.
Tim Mooney
2012-May-25 16:19 UTC
Re: [Puppet Users] Re: advice on module/class refactoring for defines
In regard to: [Puppet Users] Re: advice on module/class refactoring for...:> > 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.Agreed, that''s something I was hoping to avoid. Thanks, Tim -- Tim Mooney Tim.Mooney@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 -- You received this message because you are subscribed to the Google Groups "Puppet Users" group. To post to this group, send email to puppet-users@googlegroups.com. To unsubscribe from this group, send email to puppet-users+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/puppet-users?hl=en.
Tim Mooney
2012-May-25 16:21 UTC
Re: [Puppet Users] advice on module/class refactoring for defines
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. :-) Tim -- Tim Mooney Tim.Mooney@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 -- You received this message because you are subscribed to the Google Groups "Puppet Users" group. To post to this group, send email to puppet-users@googlegroups.com. To unsubscribe from this group, send email to puppet-users+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/puppet-users?hl=en.