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

sentinel support broken? #166

Closed
KlavsKlavsen opened this issue Apr 26, 2017 · 7 comments
Closed

sentinel support broken? #166

KlavsKlavsen opened this issue Apr 26, 2017 · 7 comments
Labels
bug Something isn't working

Comments

@KlavsKlavsen
Copy link

When doing this:
class { 'redis::sentinel':
master_name => $master_name,
redis_host => $redis_master,
failover_timeout => $failover_timeout,
}

class { 'redis':
bind => $listen_ip,
slaveof => $redis_master ? {
"$fqdn" => undef,
default => "$redis_master 6379"
}
}

to setup my hosts - with one being set to master- and redis-sentinel setup on same host.. puppet dies with:
Duplicate declaration: Package[redis-server] - as its both defined in install.pp and in redis::sentinel

I assume someone have used redis with sentinel with this module before, and I'm just understanding the documentation wrong?

@KlavsKlavsen
Copy link
Author

This is puppet 4 master and my servers are ubuntu 16.04

@petems
Copy link
Member

petems commented Apr 26, 2017

@KlavsKlavsen I haven't looked at the sentinal work for a while, could you try the following change:

diff --git a/manifests/sentinel.pp b/manifests/sentinel.pp
index 617dfc4..298c647 100644
--- a/manifests/sentinel.pp
+++ b/manifests/sentinel.pp
@@ -190,11 +190,7 @@ class redis::sentinel (
   $client_reconfig_script = $::redis::params::sentinel_client_reconfig_script,
 ) inherits redis::params {
 
-  unless defined(Package[$package_name]) {
-    ensure_resource('package', $package_name, {
-      'ensure' => $package_ensure
-    })
-  }
+  require redis

@petems
Copy link
Member

petems commented Apr 26, 2017

I'm also adding in an acceptance test for a basic sentinel setup 👍

@KlavsKlavsen
Copy link
Author

that gives Class[Redis] is already declared; Duplicate declaration: Class[Redis] is already declared; cannot redeclare.. if I remove it - and don't insert "require redis" - then it runs.

Next is that redis::sentinel is adding an init script (kinda odd when in systemd land :) - and when systemd starts it - it says all is good.. except /usr/sbin/redis-sentinel binary does not exist.. I'm assuming the sentinel support actually works that way as sentinel was seperate from redis before 3.0? I assume module was suppose to create symlink /usr/bin/redis-sentinel - pointing to redis binary? (or just run redis with --sentinel option) ?

@KlavsKlavsen
Copy link
Author

Are you running sentinel cluster mode instead?

@petems
Copy link
Member

petems commented Apr 26, 2017

Basically, this is because of different packaging standards:

[root@master redskull-controller]# yum provides /bin/redis-sentinel
Loaded plugins: fastestmirror
Loading mirror speeds from cached hostfile
 * base: mirror.cov.ukservers.com
 * epel: mirror.logol.ru
 * extras: mirror.sax.uk.as61049.net
 * updates: mirrors.vooservers.com
redis-3.2.3-1.el7.x86_64 : A persistent key-value database
Repo        : @epel
Matched from:
Filename    : /bin/redis-sentinel

So you could use Puppet to create this symlink.

I would say I don't know enough about sentinel to accurately test this. I will open a PR to change the package requirement to be an include of redis.

@petems
Copy link
Member

petems commented May 3, 2017

I believe this was fixed by #171 and an additional acceptance test was added, if not I'll re-open

@petems petems closed this as completed May 3, 2017
@petems petems added the bug Something isn't working label May 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants