duritong / puppet-trocla

puppet integration for trocla
11 stars 18 forks source link

module doesn't deploy a proper default trocla configuration #4

Closed anarcat closed 10 years ago

anarcat commented 10 years ago

I was expecting:

include trocla

do just "do the right thing". The readme actually made me thing that:

include trocla::master

... would also work, but then by reading the manifests, I came to understand we actually need to:

include trocla::config

... but that doesn't actually work either, because the default configuration is wrong.

I actually had to do:

class { 'trocla::config':
  adapter => 'YAML',
  adapter_options => { 'file' => '/var/lib/puppet/server_data/trocla.yml' },
}

It would be nice if those adapter options were the default!

duritong commented 10 years ago

Yes, the adapter should probably default to YAML and the default file should go into the data dir of your puppet master. I wonder if we can get that while compiling on the master.

And yes, the trocla class is not really usefull at all.

anarcat commented 10 years ago

so one of two things, either we make the trocla class call ::config with the right parameters:

diff --git a/manifests/init.pp b/manifests/init.pp
index cf5223e..560d701 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -1,6 +1,8 @@
-#Main definition class for trocla. Just calls master
+# Main definition class for trocla. Just trocla::config, call that
+# directly if you need to override defaults
 class trocla {
-
-  include trocla::master
-
+  class { 'trocla::config':
+    adapter => 'YAML',
+    adapter_options => { 'file' => '/var/lib/puppet/server_data/trocla.yml' },
+  }
 }

... or we just ditch all that and make the trocla class make more sense:

class trocla (
  $adapter         = 'YAML',
  $keysize         = 16,
  $adapter_options = { 'file' => '/var/lib/puppet/server_data/trocla.yml' },
) {
# Deploy default config file and link it for trocla cli lookup
  file{
    "${settings::confdir}/troclarc.yaml":
      ensure  => present,
      content => template('trocla/troclarc.yaml.erb'),
      owner   => root,
      group   => puppet,
      mode    => '0640';
    '/etc/troclarc.yaml':
      ensure => link,
      target => "${settings::confdir}/troclarc.yaml";
  }
  include trocla::master
}

what is it going to be? :)

duritong commented 10 years ago

thanks for your ideas. I pushed various changes that include improved documentation and also a trocla::yaml class, as you proposed.

I hope that these changes address your concerns. Let me know if you are happy with it or have further suggestions.