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

sensu-puppet-add heartbeat feature #596

Merged
merged 2 commits into from
Feb 23, 2017
Merged

sensu-puppet-add heartbeat feature #596

merged 2 commits into from
Feb 23, 2017

Conversation

derkgort
Copy link
Contributor

add support for setting the rabbitmq heartbeat value #559

@jaxxstorm
Copy link
Contributor

@derkgort there's a bunch of unrelated changes which have slipped in here which have caused the tests to fail. Can you please reassess?

@derkgort
Copy link
Contributor Author

Yeah sorry. That was not my best work...
I'll retry later this week..

@derkgort
Copy link
Contributor Author

I fixed my error from before.
In the other pull request (#559) I was asked to make the rabbitmq_heartbeat default undef.
Should I still do this?

@jaxxstorm
Copy link
Contributor

@derkgort yes, but you'll also need to update the tests so they pass. Thanks for cleaning this up.

@derkgort
Copy link
Contributor Author

The travis checks pass. Do I need to check something else?

@jaxxstorm
Copy link
Contributor

Nope, this looks good, i will merge it after my Christmas break :)

@jaxxstorm jaxxstorm merged commit afd74cb into sensu:master Feb 23, 2017
@derkgort derkgort deleted the feature/heartbeat branch February 24, 2017 11:32
@dzeleski
Copy link
Contributor

dzeleski commented Mar 15, 2017

@derkgort I think im running into an issue with this change, previously the following config would work:

         version: '0.27.0-1'
         install_repo: true
         purge: true
         rabbitmq_cluster:
          - host: 'plxlb-sensu01.domain.lan'
            port: 5672
            vhost: '/sensu'
            user: 'sensu'
            password: 'secret'
            heartbeat: 30
            prefetch: 50
          - host: 'plxlb-sensu02.domain.lan'
            port: 5672
            vhost: '/sensu'
            user: 'sensu'
            password: 'secret'
            heartbeat: 30
            prefetch: 50
          - host: 'plxlb-sensu03.domain.lan'
            port: 5672
            vhost: '/sensu'
            user: 'sensu'
            password: 'secret'
            heartbeat: 30
            prefetch: 50

Im now getting the following error on both rhel and windows:

Could not evaluate: no implicit conversion of String into Integer
Source:/Stage[main]/Sensu::Rabbitmq::Config/Sensu_rabbitmq_config[tv99988mwl02.domain.lan]
File:/etc/puppetlabs/code/environments/lab/modules/sensu/manifests/rabbitmq/config.pp
Line:125

Im thinking something between the normal rabbitmq and rabbitmq_cluster config is getting messed up? If you look here: https://github.com/derkgort/sensu-puppet/blob/5ff634aba010e6e161d765380d204e5b09cf9767/manifests/rabbitmq/config.pp#L111-L123 It looks like there is an override to deal with the regular rabbitmq config options when a cluster is specified.

EDIT: If you look at your heartbeat provider and type it differs from prefetch in a few ways. Im going to try and mess with it to see if I can get this fixed but building providers and types is not something I work with often.

@dzeleski
Copy link
Contributor

@derkgort @jaxxstorm Please see the following changes:

dzeleski@e223f96

I basically copied what was working for prefetch.

@derkgort
Copy link
Contributor Author

@dzeleski
This looks like a better version. I contains more checking, which is good.
I don't know about the default change. There was some discussion about with my change.

I'd suggest to send a PR. That's what I did to get it changed.

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

Successfully merging this pull request may close these issues.

3 participants