Set permissions more sanely on log dir. Add tests.

Only sets permissions on the redis_logfile directory if the
directory has just been created. Otherwise, it will touch the
log file with ownership of the redis user.

I'm probably a bad person for not doing this in separate commits,
but this also adds in a test-kitchen test suite.

Author: DavidWittman

.gitignore

@@ -1 +1,4 @@
 *.swp
+.kitchen/
+.bundle
+.vagrant

.kitchen.yml

@@ -0,0 +1,25 @@
+---
+driver:
+  name: vagrant
+
+provisioner:
+  name: ansible_playbook
+  ansible_verbose: true
+  ansible_verbosity: 2
+  require_ruby_for_busser: false
+  require_chef_for_busser: true
+  hosts: all
+
+platforms:
+  - name: ubuntu-14.04
+  - name: centos-6.7
+    driver_config:
+      box: wittman/centos-6.7-ansible
+  - name: centos-7.2
+    driver_config:
+      box: wittman/centos-7.2-ansible
+
+suites:
+  - name: default
+  - name: logfile
+  - name: sentinel

.travis.yml

@@ -11,19 +11,19 @@ install:
 
 script:
   # Syntax check
-  - "ansible-playbook -i localhost, tests/test_server.yml --syntax-check"
-  - "ansible-playbook -i localhost, tests/test_sentinel.yml --syntax-check"
+  - "ansible-playbook -i localhost, test/test_server.yml --syntax-check"
+  - "ansible-playbook -i localhost, test/test_sentinel.yml --syntax-check"
   # Run role
-  - "ansible-playbook -i localhost, tests/test_server.yml --connection=local --sudo"
+  - "ansible-playbook -i localhost, test/test_server.yml --connection=local --sudo"
   # Idempotency check
   - >
-    ansible-playbook -i localhost, tests/test_server.yml --connection=local --sudo
+    ansible-playbook -i localhost, test/test_server.yml --connection=local --sudo
     | grep -q 'changed=0.*failed=0'
     && (echo 'Idempotency: PASS' && exit 0)
     || (echo 'Idempotency: FAIL' && exit 1)
-  - "ansible-playbook -i localhost, tests/test_sentinel.yml --connection=local --sudo"
+  - "ansible-playbook -i localhost, test/test_sentinel.yml --connection=local --sudo"
   - >
-    ansible-playbook -i localhost, tests/test_sentinel.yml --connection=local --sudo
+    ansible-playbook -i localhost, test/test_sentinel.yml --connection=local --sudo
     | grep -q 'changed=0.*failed=0'
     && (echo 'Idempotency: PASS' && exit 0)
     || (echo 'Idempotency: FAIL' && exit 1)

Gemfile

@@ -0,0 +1,5 @@
+source "https://rubygems.org"
+
+gem "test-kitchen"
+gem "kitchen-ansible"
+gem "kitchen-vagrant"

Gemfile.lock

@@ -0,0 +1,41 @@
+GEM
+  remote: https://rubygems.org/
+  specs:
+    faraday (0.9.2)
+      multipart-post (>= 1.2, < 3)
+    highline (1.7.8)
+    kitchen-ansible (0.0.36)
+      librarian-ansible
+      test-kitchen (~> 1.4)
+    kitchen-vagrant (0.19.0)
+      test-kitchen (~> 1.4)
+    librarian (0.1.2)
+      highline
+      thor (~> 0.15)
+    librarian-ansible (1.0.6)
+      faraday
+      librarian (~> 0.1.0)
+    mixlib-shellout (2.2.5)
+    multipart-post (2.0.0)
+    net-scp (1.2.1)
+      net-ssh (>= 2.6.5)
+    net-ssh (2.9.2)
+    safe_yaml (1.0.4)
+    test-kitchen (1.4.2)
+      mixlib-shellout (>= 1.2, < 3.0)
+      net-scp (~> 1.1)
+      net-ssh (~> 2.7, < 2.10)
+      safe_yaml (~> 1.0)
+      thor (~> 0.18)
+    thor (0.19.1)
+
+PLATFORMS
+  ruby
+
+DEPENDENCIES
+  kitchen-ansible
+  kitchen-vagrant
+  test-kitchen
+
+BUNDLED WITH
+   1.11.2

README.md

@@ -212,6 +212,7 @@ redis_syslog_facility: USER
 ## General configuration
 # Daemonize the redis server. Must be a string "yes" or "no".
 redis_daemonize: "yes"                                                          
+# Pidfile. If the directory does not exist, it will be created with the redis user as the owner. The redis user must have rwx permissions on this directory.
 redis_pidfile: /var/run/redis/{{ redis_port }}.pid
 # Number of databases to allow
 redis_databases: 16

defaults/main.yml

@@ -3,6 +3,7 @@
 redis_version: 2.8.9
 redis_install_dir: /opt/redis
 redis_user: redis
+redis_group: "{{ redis_user }}"
 redis_dir: /var/lib/redis/{{ redis_port }}
 redis_tarball: false
 # The open file limit for Redis/Sentinel

tasks/install.yml

@@ -36,8 +36,8 @@
   when: redis_tarball
 
 - name: extract redis tarball
-  shell: tar xf /usr/local/src/redis-{{ redis_version }}.tar.gz -C /usr/local/src
-         creates=/usr/local/src/redis-{{ redis_version }}
+  unarchive: src=/usr/local/src/redis-{{ redis_version }}.tar.gz dest=/usr/local/src
+             creates=/usr/local/src/redis-{{ redis_version }}/Makefile
 
 - name: compile redis
   command: make -j5

tasks/sentinel.yml

@@ -22,30 +22,43 @@
   service: name=sentinel_{{ redis_sentinel_port }} enabled=yes
   when: redis_as_service
 
-- name: check if sentinel log file exists
-  stat: path={{ redis_sentinel_logfile }}
-  register: sentinel_logfile_stat
+# Check then create log dir to prevent aggressively overwriting permissions
+- name: check if sentinel log directory exists
+  stat: path={{ redis_sentinel_logfile|dirname }}
+  register: sentinel_logdir
+  changed_when: false
+  when: redis_sentinel_logfile != '""'
 
-- name: ensure sentinel pidfile directory exists and has correct owner
-  file: path={{ redis_sentinel_pidfile|dirname }}
+- name: create sentinel log directory if it does not exist
+  file: state=directory
+        path={{ redis_sentinel_logfile|dirname }}
         owner={{ redis_user }}
-        state=directory
-        recurse=yes
+        group={{ redis_group }}
+  when:
+    - redis_sentinel_logfile != '""'
+    - not sentinel_logdir.stat.exists
 
-- name: ensure sentinel logfile directory exists and has correct owner
-  file: path={{ redis_sentinel_logfile|dirname }}
+- name: touch the sentinel log file
+  file: state=touch
+        path={{ redis_sentinel_logfile }}
         owner={{ redis_user }}
-        state=directory
-        recurse=yes
+        group={{ redis_group }}
   when: redis_sentinel_logfile != '""'
 
-- name: ensure that sentinel log file exists and is writable by redis
-  file: path={{ redis_sentinel_logfile }}
+- name: check if sentinel pid directory exists
+  stat: path={{ redis_sentinel_pidfile|dirname }}
+  register: sentinel_piddir
+  changed_when: false
+  when: redis_sentinel_pidfile != '""'
+
+- name: create sentinel pid directory if it does not exist
+  file: state=directory
+        path={{ redis_sentinel_pidfile|dirname }}
         owner={{ redis_user }}
-        group={{ redis_user }}
-        mode=0600
-        state=touch
-  when: sentinel_logfile_stat.stat.exists == False and redis_sentinel_logfile != '""'
+        group={{ redis_group }}
+  when:
+    - redis_sentinel_pidfile != '""'
+    - not sentinel_piddir.stat.exists
 
 - name: create sentinel config file
   template: src=redis_sentinel.conf.j2

tasks/server.yml

@@ -21,30 +21,43 @@
   service: name=redis_{{ redis_port }} enabled=yes
   when: redis_as_service
 
-- name: check if log file exists
-  stat: path={{ redis_logfile }}
-  register: logfile_stat
-  
-- name: ensure pidfile directory exists and has correct owner
-  file: path={{ redis_pidfile|dirname }}
+# Check then create log dir to prevent aggressively overwriting permissions
+- name: check if log directory exists
+  stat: path={{ redis_logfile|dirname }}
+  register: logdir
+  changed_when: false
+  when: redis_logfile != '""'
+
+- name: create log directory if it does not exist
+  file: state=directory
+        path={{ redis_logfile|dirname }}
         owner={{ redis_user }}
-        state=directory
-        recurse=yes
+        group={{ redis_group }}
+  when:
+    - redis_logfile != '""'
+    - not logdir.stat.exists
 
-- name: ensure logfile directory exists and has correct owner
-  file: path={{ redis_logfile|dirname }}
+- name: touch the log file
+  file: state=touch
+        path={{ redis_logfile }}
         owner={{ redis_user }}
-        state=directory
-        recurse=yes
+        group={{ redis_group }}
   when: redis_logfile != '""'
 
-- name: ensure that log file exists and is writable by redis
-  file: path={{ redis_logfile }}
+- name: check if pid directory exists
+  stat: path={{ redis_pidfile|dirname }}
+  register: piddir
+  changed_when: false
+  when: redis_pidfile != '""'
+
+- name: create pid directory if it does not exist
+  file: state=directory
+        path={{ redis_pidfile|dirname }}
         owner={{ redis_user }}
-        group={{ redis_user }}
-        mode=0600
-        state=touch
-  when: logfile_stat.stat.exists == False and redis_logfile != '""'
+        group={{ redis_group }}
+  when:
+    - redis_pidfile != '""'
+    - not piddir.stat.exists
 
 - name: create redis config file
   template: src=redis.conf.j2 dest=/etc/redis/{{ redis_port }}.conf

test/integration/default/default.yml

@@ -0,0 +1,4 @@
+---
+- hosts: localhost
+  roles:
+    - ansible-redis

test/integration/default/serverspec/redis_spec.rb

@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe 'Redis' do
+  describe service('redis_6379') do
+    it { should be_enabled }
+    it { should be_running }
+  end
+
+  describe port(6379) do
+    it { should be_listening.on('0.0.0.0').with('tcp') }
+  end
+
+  describe file('/etc/redis/6379.conf') do
+    it { should be_file }
+    it { should be_owned_by 'redis' }
+    its(:content) { should match /port 6379/ }
+  end
+
+  describe file('/var/run/redis/6379.pid') do
+    it { should be_file }
+    it { should be_owned_by 'redis' }
+    its(:size) { should_be > 0 }
+  end
+
+  describe file('/proc/sys/vm/overcommit_memory') do
+    it { should be_file }
+    it { should contain '1' }
+  end
+end

test/integration/default/serverspec/spec_helper.rb

@@ -0,0 +1,2 @@
+require 'serverspec'
+set :backend, :exec

test/integration/logfile/default.yml

@@ -0,0 +1,5 @@
+---
+- hosts: localhost
+  roles:
+    - role: ansible-redis
+      redis_logfile: "/var/log/redis.log"

test/integration/logfile/serverspec/log_spec.rb

@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe 'Redis' do
+  describe service('redis_6379') do
+    it { should be_enabled }
+    it { should be_running }
+  end
+
+  describe port(6379) do
+    it { should be_listening.with('tcp') }
+  end
+
+  describe file('/var/log/redis.log') do
+    it { should be_file }
+    it { should be_owned_by 'redis' }
+    its(:size) { should > 0 }
+  end
+
+  describe file('/var/log') do
+    it { should be_directory }
+    it { should_not be_owned_by('redis') }
+  end
+end

test/integration/logfile/serverspec/spec_helper.rb

@@ -0,0 +1,2 @@
+require 'serverspec'
+set :backend, :exec

test/integration/sentinel/default.yml

@@ -0,0 +1,6 @@
+---
+- hosts: localhost
+  roles:
+    - role: ansible-redis
+      redis_sentinel: true
+      redis_sentinel_logfile: "/var/log/redis_sentinel.log"

test/integration/sentinel/serverspec/sentinel_spec.rb

@@ -0,0 +1,24 @@
+require 'spec_helper'
+
+describe 'Redis' do
+  describe service('sentinel_26379') do
+    it { should be_enabled }
+    it { should be_running }
+  end
+
+  describe port(26379) do
+    it { should be_listening.on('0.0.0.0').with('tcp') }
+  end
+
+  describe file('/etc/redis/sentinel_26379.conf') do
+    it { should be_file }
+    it { should be_owned_by 'redis' }
+    its(:content) { should match /port 26379/ }
+  end
+
+  describe file('/var/run/redis/sentinel_26379.pid') do
+    it { should be_file }
+    it { should be_owned_by 'redis' }
+    its(:size) { should > 0 }
+  end
+end

test/integration/sentinel/serverspec/spec_helper.rb

@@ -0,0 +1,2 @@
+require 'serverspec'
+set :backend, :exec