On 12/21/2010 07:05 PM, deet wrote:> Hello.
> I recently tried to improve a module I have which is used to create
> one or more instances of mysql per node. The original module had
> lot''s of code repetition to get around gaps in my skills. The
new
> improved module has less code repetition but doesn''t seem as clean
as
> I imagined it in my head:)
>
> The env is puppet 2.6.4, facter 1.5.8, solaris 10 x86 and nodes.pp
> files not external nodes.
>
> I still find myself thinking shell scripting and not managed
> resources. So while the module does what I want it to I would
> appreciate any feedback to help me stay in line with good practices
> and approaching the problem in a more puppet like fashion.
>
> I put the module at https://github.com/someword/mysql-module and
> included an example-nodes.pp file in the top level which illustrates
> what a node would look like.
General remarks:
1. Comments: You call everything "classes" whereas they are defines.
There''s a fundamental difference.
2. Variables: You set variables inside those defines and expect them to
be exported to various other scopes (I assume). Don''t. Variable scoping
is a delicate subject. All dynamic scoping may go away in a future
release. In defines, all required state should be passed as parameters.
Do exploit default parameter values. NEVER try to set variables in
defines (or even classes) and expect them to be exported to calling scopes.
> The three key areas where I feel could be improved are:
>
> 1. In the mysql::instance define the use of an "if/elsif/else"
clause
> to decide which version class to load to get the particular variables
> for that version.
> When i used the notation
> include "mysql::$major"
>
> the right class got included but the variables set in their were
> not readily available for use in the mysql::instance define. Also I
> tried calling mysql::$major { "$instance": } but that failed
syntax
> checking.
See above about juggling variables around. You do not want to do it at all!
I believe that ideally, nothing outside mysql::v4 should need to know
the package name or version. If I err here, this may or may not be a
saner approach:
class mysql::v4_constants {
$package = "..."
$version = ...
...
}
Wherever you need it, you must include the class then and can access
$mysql::v4_constants::package etc. I don''t see how that''s
really useful,
though.
> 2. It''s possible for 2 mysql instances on one node to use the
same
> version of software which causes duplicate resources. To get around
> this I used an if statement in the package.pp manifest to check if the
> resource is defined so that it wouldn''t get duplicated. Having to
> take this step makes me think I''m not doing something correctly.
The package resource should not be used in a define. Instead, create a
class that takes care of installing the package and include that from
the define.
class mysql::package {
package { "mysql": ensure => installed }
}
define mysql::instance() {
include mysql::package
...
}
This way, you have ensured that the package resource is defined exactly
once. That''s what classes are for in the first place.
> 3. Resource ordering.
> I rely on alot of resource ordering to ensure the correct
> application of resources. I''m wondering if their are
cleaner/better
> approaches to ensure the desired ordering with out explicitly stating
> them.
No. It''s good style to explicitly state such interdependencies.
However, a quick grep yields zero before and require parameters in your
module - where are those orderings you rely on?
>
> Any input to help broaden my puppet horizons is welcome.
> TIA. deet.
I believe you should review the documentation wrt. define vs. class.
Both concepts have their places and it is very important to understand
what the respective uses are.
HTH,
Felix
--
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.