Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watches key in config_hash should expect an array of hashes? #83

Closed
miroswan opened this issue Feb 18, 2015 · 8 comments
Closed

Watches key in config_hash should expect an array of hashes? #83

miroswan opened this issue Feb 18, 2015 · 8 comments

Comments

@miroswan
Copy link

Hey, so. Looking at the consul configuration syntax, it looks like the watches value should be an array of hashes. Each watcher is a hash and indexed in an array. I'm not sure how to configure my watches using this puppet module since the watches key in the configuration hash is expecting a hash value instead of an array value. If this is by design, would you mind briefly explaining how to configure this key properly? Otherwise, this is probably a bug.

Thanks.

@solarkennedy
Copy link
Contributor

According to the tests it is a hash:
https://github.com/solarkennedy/puppet-consul/blob/master/spec/classes/init_spec.rb#L247

It looks like the default is an empty hash and we are validating that it is as hash:
https://github.com/solarkennedy/puppet-consul/blob/master/manifests/init.pp#L62-L73

But it looks like we lack docs on this. Can you make a PR that adds a docstring in the init.pp and maybe and example in README.md?

Being a hash you would configure it like this:

class { consul:
  ...
  watches => {
    'haproxy' => {
      'key'     => 'services/haproxy/',
      'handler' => 'rebuild_haproxy.sh',
    }
    'sshd' => {
      'key'     => 'services/sshd',
      'handler' => 'rebuild_sshd.sh',
    }
  }
  services => {}
  checks => {}
}

Or in hiera it would be in yaml form.

@miroswan
Copy link
Author

This causes a dependency loop:

class { 'consul':
  config_hash => {
    'data_dir'   => '/cust/consul',     
    'datacenter' => 'devint',
    'log_level'  => 'INFO',
    'node_name'  => "${fqdn}"
  },
  watches     => {
    'services'       => {
      'type'    => 'services',
      'handler' => 'sudo python /usr/local/bin/reacktor services'
    },
    'httpd_service'  => {
      'type'    => 'service',
      'service' => 'httpd',
      'handler' => 'sudo python /usr/local/bin/reacktor service --service httpd'
    },
    'tomcat_service' => {
      'type'    => 'service',
      'service' => 'tomcat',
      'handler' => 'sudo python /usr/local/bin/reacktor service --service tomcat'
    }
  }
}

err: Could not apply complete catalog: Found 1 dependency cycle:
(File[/etc/consul/watch_httpd_service.json] => Class[Consul::Run_service] => Service[consul] => Class[Consul::Run_service] => Class[Consul] => Consul::Watch[httpd_service] => File[/etc/consul/watch_httpd_service.json])

@solarkennedy
Copy link
Contributor

I'm having trouble reproducing your issue.

So I've added a test case with exactly the input you provided.
https://github.com/solarkennedy/puppet-consul/blob/master/spec/classes/init_spec.rb#L287

Here are the test results:
https://travis-ci.org/solarkennedy/puppet-consul/builds/51933146

@solarkennedy
Copy link
Contributor

@miroswan can you confirm you are running the latest version of the code?

@miroswan
Copy link
Author

I'm running v0.4.6. I ended up overriding the config.json resource and templating it so I can avoid the aforementioned issue and gain a bit more more flexibility. Aside from the watches class param, the consul module has been working out just fine for me. If the tests are passing, I suppose it's fine to mark this one as closed for now. Possible PEBCAK on my end.

@solarkennedy
Copy link
Contributor

Possibly. Can you try the tip of master and confirm that whatever bug that you are hitting is fixed? That way we can close this issue with confidence.

@aj-jester
Copy link

@miroswan Your dependency issue (whether it was really an issue or not) might be fixed with this PR #152

Can you pull down the latest master and try it again?

@hopperd
Copy link
Contributor

hopperd commented Mar 25, 2016

Closing as this is an old issue and seems to be fixed. If not please re-open the issue.

@hopperd hopperd closed this as completed Mar 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants