Support for multi-instances per host

[rhoml]

It is a common practice to have more than one instance of redis per host. We should move the config.pp into a define type so we can create multiple instances pointing into different ports.

Thoughts?

[spo0nman]

Yes! I support this. I am actually in the process of hacking this into the module. maybe we can collaborate.

[rhoml]

Absolutely, maybe rolling out small changes into a dev branch?

Sent from my iPhone

On 22 Sep. 2016, at 21:45, Pankaj notifications@github.com wrote:

Yes! I support this. I am actually in the process of hacking this into the module. maybe we can collaborate.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[spo0nman]

@rhoml do you have a branch already that i can chekout?

[arioch]

I'd prefer having an $instance parameter in the main class which the config class could read, parse and deploy using a resource defined type.

Pseudo code:

class redis (
  $instance = {},
) {
}

class redis::config {
  create_resources(::redis::instance, $::redis::instance, {})
}

define redis::instance (
  $instance_name,
  $instance_config_dir,
  $instance_config_file,
  $instance_ip,
  $instance_port,
) {
}

(Update) Or even:

define redis::instance (
  $name,
  $config_dir,
  $config_file,
  $options = {
    $ip => undef,
    $port => undef,
    $max_mem => undef,
  }
) {
}

[rhoml]

@spo0nman No I don't have any working branch atm, been busy.

@arioch yeah, I'd like to see something similar to elastic/puppet-elasticsearch https://github.com/elastic/puppet-elasticsearch/blob/master/manifests/instance.pp

[spo0nman]

@arioch how would we make sure that all the instances are individually configurable? e.g if redis::config is the top layer, and multiple instances on a single host will need instance specific overrides for max_memory etc.

[arioch]

@spo0nman by using redis::config for its original purpose: to configure generic stuff in /etc/redis.

Instances could have their own directory structure, f.i. /opt/redis/instance_1 managed by redis::instance.

[spo0nman]

@arioch the updated one looks good. let me write something around this idea.

[spo0nman]

Hi,

small update: I have a decent working version now with these changes. I had to make a few changes to accomplish this. I have a rather large patch, I'll commit it to my fork for your review tomorrow.

[petems]

@spo0nman Hey, @arioch has added me to the repo to help with maintenance, I look forward to your PR! 👍

[spo0nman]

Hello!

I have pushed[1] my changes to my fork. It is a large change, I took the idea of @arioch and created a define called redis::instance, which is called multiple times from redis::config.

I added a new variable called instances, this is expected to be a hash of the form:

$instances = {
 'instance_1' ={ 
         'port' = 'portnumber',
         'maxmemory' ='maxmemory'
     },
 'instance_2' ={ 
         'port' = 'portnumber',
         'maxmemory' ='maxmemory'
     }
}

The defile allows us to 'overload' common redis::variables for all the instances and 'import' sane defaults from params.pp. It also allows us to overload defaults for all the instances on a host for example redis::maxmemory: is default 2gb for the host but is increased in instance_1. The commit has a lot more details.

for multiple instances we need multiple redis-server processes, so i decided to decorate the process name with instance) name.

hieradata/redis.yaml

redis::bind: 0.0.0.0
redis::maxmemory: 2gb
redis::config_dir: '/etc/redis/'
redis::slaveof: 'redis-101'
redis::sentinel::discover_master: true
redis::sentinel::multiple_instances: true
redis::sentinel::members:
    - redis-101
    - redis-102
    - redis-104
redis::instances:
    instance_1:
        port: 7000
        maxmemory: 4gb
    instance_2:
        port: 7001
    instance_3:
        port: 7002

A somewhat related but maybe unrelated feature in the commit is the sentinel_discover: since sentinel is not configured alone, we can use sentinel::discover_master flag to ask the "members" about an instance master. discover_master is set to false by default, in that case redis_host is used. (if you guys feel this is not good, we can take it out, but I use this in my infra and find it very useful.) more details on this in the commit comment.

1. spo0nman@62ebea6

[spo0nman]

@petems I am currently struggling with sparklemotion/nokogiri#1445 and could not run the rake tests on my changes. As soon as I have rake running. I will run the tests.

[spo0nman]

#128

#129

[petems]

Hi @spo0nman! I'm looking over your changes, I think some would change the intentions of this module and what falls under it's scope, but others I think could be broken up into individual features that could be merged. I should have some cycles to look through stuff in december, hopefully I can merge it for you then! 👍

[spo0nman]

@petems sounds good. Let's sync up in December and I can separate out the unnecessary.

[petems]

This is now merged as part of #200

class { '::redis':
  default_install => false,
}

redis::instance {'redis1':
  port     => '7777',
}

redis::instance {'redis2':
  port     => '8888',
}

Ta-dah! 😄

[demueller]

Is there a way to do this?:

redis::instance { '::redis':
  default_install => false,
}
redis::instance {'redis1':
  port     => '7777',
}

redis::instance {'redis2':
  port     => '8888',
}

Using this style we are getting error:

Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Error while evaluating a Function Call, Could not find template '' at /etc/puppetlabs/code/environments/development/modules/redis/manifests/instance.pp:294:46

[petems]

@demueller that should just be:

 class { '::redis':
      default_install => false,
    }
    redis::instance {'redis1':
      port                => '7777',
    }
    redis::instance {'redis2':
      port                => '8888',
    }