sprinkle-tool / sprinkle

Sprinkle is a software provisioning tool you can use to build remote servers with. eg. to install a Rails, or Sinatra stack on a brand new slice directly after its been created
https://github.com/sprinkle-tool/sprinkle
MIT License
1.15k stars 138 forks source link

Feature: Need way better way to verify matching files when using erb templates #121

Closed chrisolsen closed 11 years ago

chrisolsen commented 11 years ago

I can't seem to find a good way to verify matching files on the server after using the transfer method with render: true.

package :thin_configuration do
  description "Nginx as Reverse Proxy Configuration for Thin"

  requires :nginx

  locals = {
    thin_cluster_count: attributes[:thin][:cluster_count] || ask("Thin Cluster Count: "),
    server_addr:        attributes[:thin][:server_addr]   || ask("Server Url(s): "),
    root_path:          attributes[:thin][:root_path]     || ask("App root path: "),
  }

  src  = package_path(__FILE__, 'thin.conf')
  dest = '/etc/nginx/conf.d/thin.conf'

  transfer(src, dest, sudo: true, locals: locals) do
    post :install, 'rm -f /etc/nginx/sites-available/default'
    post :install, '/etc/init.d/nginx restart'
  end

  verify do
    # src is only the template so this will always fail
    # matches_local src, dest, render: true, locals: locals  # would work
    matches_local src, dest
  end
end
joshgoebel commented 11 years ago

Does package_path read in the template contents?

joshgoebel commented 11 years ago

I'm feeling a need to completely rethink file rendering to be more like puppet (much more explicit):

    file '/etc/apache2/apache2.conf',
      :sudo => true,
      :content => template(template_dir.join('apache2.conf.erb'), binding),
      :mode => '644'

Then at least you could quickly stash the md5 in a local variable and use it for an md5 check inside verify... and transfer would stick to scping files... the way to push a single file into place would be to use "file".

joshgoebel commented 11 years ago

Ultimately there needs to be a little more cross talk between installers and verifiers... like an auto-verify option...but then it's possible that not every step along the way needs to be verified... hate to start sticking autoverify: true on everything... it sure would help in cases like this though.

chrisolsen commented 11 years ago

Does package_path read in the template contents? Close, it is just a custom helper method that returns the full template path

Ultimately there needs to be a little more cross talk between installers and verifiers I agree

joshgoebel commented 11 years ago

If it returns only the path then aren't you missing a :render?

chrisolsen commented 11 years ago

I am, looks like I forgot it when I was manually reverting things back after making some attempts to do the check. Sorry for the confusion.

chrisolsen commented 11 years ago

What do you think of the commented out suggestion? Would that work?

matches_local src, dest, render: true, locals: locals
joshgoebel commented 11 years ago

The right way to do something like you are asking:


package :nginx_conf do
  @nginx_port = 8080
  nginx_config = render("nginx.conf")
  file '/etc/nginx.conf', 
    :contents => nginx_config

  verify do
    md5_of_file "/etc/nginx.conf", md5(nginx_config)
  end
end
joshgoebel commented 11 years ago

transfer :render will be depreciated in 0.8 and file is the new way to do these type of things.

joshgoebel commented 11 years ago

Or maybe 0.9 since this wont likely hit the gem until later.

joshgoebel commented 11 years ago

We could support passing locals as well, though I think you should look at the new defaults functionality we're adding... I wonder if it could help simplify your setup any.

joshgoebel commented 11 years ago

The branch I'm working on: https://github.com/sprinkle-tool/sprinkle/tree/file_installer

joshgoebel commented 11 years ago

And it's back in master now.

joshgoebel commented 11 years ago

The latest render() now supports passing locals as the second argument... see rendering.rb. Closing this issue.

koenpunt commented 11 years ago

Although keep in mind if supplying that second argument, the instance variables from the package won't be bound to the template anymore.

joshgoebel commented 11 years ago

Yeah, I think that would be understood.

joshgoebel commented 11 years ago

We'd have to jump thru more hoops if we wanted locals to be additive instead of replacing context.