From 1a60469bb0892a6e8179d295422fe1ba567b3f19 Mon Sep 17 00:00:00 2001 From: Dan Carley Date: Tue, 31 Dec 2013 15:03:25 +0000 Subject: [PATCH 1/5] Remove extraenous subscribe from config file I'm 99% sure that a `subscribe` has no effect on a file. You can't notify a file resource to be evaluated - they are always evaluated. The chaining in `init.pp` ensures that `::install` always happens before `::config`. --- manifests/config.pp | 1 - 1 file changed, 1 deletion(-) diff --git a/manifests/config.pp b/manifests/config.pp index 408d0b3..19d7868 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -123,7 +123,6 @@ ensure => 'directory', owner => 'www-data', mode => '0775', - subscribe => Exec['graphite/install graphite-web'], } file { "${root_dir}/webapp/graphite/local_settings.py": From 778da32e3e1e735a7aecdcfbc603477b16b9527d Mon Sep 17 00:00:00 2001 From: Dan Carley Date: Tue, 31 Dec 2013 15:03:52 +0000 Subject: [PATCH 2/5] Install to Python virtualenv Install Graphite, Carbon, Whisper, and (nearly) all dependencies into a virtualenv using the `stankevich/python` module. This retains the same sandboxing that `--install-option`/`--prefix` was doing, but with the added benefit of: - Allowing the version param to upgrade and downgrade the packages without writing the `pip` logic ourselves. - Isolating dependencies from the system's Python/pip paths. Absolute binary paths are used to reference the virtualenv without needing to `source bin/activate` each time. Cairo is shimmed into the virtualenv from the system package because of the reasons described in the class doc. I've rebased the dependencies against Graphite's `requirements.txt` to reference specific versions and dropped: - LDAP support: because it has compile dependencies and I'm not aware of anyone currently using this functionality from the module. - pysqlite2: because Python >=2.6 has this builtin. I'm using an earlier version of the `python` module because of the following bug which causes the virtualenv to be recreated on every run: - https://github.com/stankevich/puppet-python/issues/46 Tests will be committed separately because they depend on refactoring currently in unmerged PR #16. --- Modulefile | 2 ++ README.md | 18 +++++++++++-- Vagrantfile | 12 ++++++++- manifests/config.pp | 2 +- manifests/deps.pp | 41 ++++++++++++++++++++--------- manifests/install.pp | 37 +++++--------------------- templates/upstart/carbon-cache.conf | 2 +- templates/upstart/graphite-web.conf | 2 +- 8 files changed, 66 insertions(+), 50 deletions(-) diff --git a/Modulefile b/Modulefile index 3579cc5..d7e4954 100644 --- a/Modulefile +++ b/Modulefile @@ -5,4 +5,6 @@ license 'MIT' summary 'Module to manage the Graphite monitoring tool' source 'https://github.com/gds-operations/puppet-graphite' project_page 'https://github.com/gds-operations/puppet-graphite' + dependency 'puppetlabs/stdlib', '>= 2.5.0' +dependency 'stankevich/python', '>= 1.2.0' diff --git a/README.md b/README.md index 15274e7..32a9bab 100644 --- a/README.md +++ b/README.md @@ -6,9 +6,23 @@ Status](https://secure.travis-ci.org/gds-operations/puppet-graphite.png)](http:/ # Usage -Nice and simple, mainly because it's not yet very configurable. +You will need Python, Python's development headers/libs, pip and virtualenv +installed. If you're not already managing these you can use the `python` +module, which is included as a dependency: - include graphite +```puppet +class { 'python': + pip => true, + dev => true, + virtualenv => true, +} +``` + +Then for the simplest possible configuration: + +```puppet +include graphite +``` ## Configuration diff --git a/Vagrantfile b/Vagrantfile index e677c74..d80494a 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -9,9 +9,19 @@ DEBIAN_FRONTEND=noninteractive apt-get -qy update if [ ! -e /etc/puppet/modules/stdlib ]; then puppet module install puppetlabs-stdlib --modulepath /etc/puppet/modules fi +if [ ! -e /etc/puppet/modules/python ]; then + puppet module install stankevich-python --modulepath /etc/puppet/modules +fi puppet apply --verbose -e "node default { \ - class { 'graphite': root_dir => '/var/lib/graphite' } \ + class { 'python': \ + pip => true, \ + dev => true, \ + virtualenv => true, \ + } \ + class { 'graphite': \ + root_dir => '/var/lib/graphite', \ + } \ }" EOM diff --git a/manifests/config.pp b/manifests/config.pp index 19d7868..b52b083 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -100,7 +100,7 @@ } exec { 'init-db': - command => '/usr/bin/python manage.py syncdb --noinput', + command => "${root_dir}/bin/python manage.py syncdb --noinput", cwd => "${root_dir}/webapp/graphite", creates => "${root_dir}/storage/graphite.db", subscribe => File["${root_dir}/storage"], diff --git a/manifests/deps.pp b/manifests/deps.pp index ec91bbc..dae2a4b 100644 --- a/manifests/deps.pp +++ b/manifests/deps.pp @@ -1,21 +1,36 @@ # == Class: graphite::deps # -# Class to install all graphite dependency packages +# Class to create a Python virtualenv and install all graphite dependencies +# within that sandbox. +# +# With the exception of pycairo which can't be installed by pip. It is +# installed as a system package and symlinked into the virtualenv. This is a +# slightly hacky alternative to `--system-site-packages` which would mess +# with Graphite's other dependencies. # class graphite::deps { + $root_dir = $::graphite::root_dir - ensure_packages([ - 'python-ldap', - 'python-cairo', - 'python-django', - 'python-twisted', - 'python-django-tagging', - 'python-simplejson', - 'python-memcache', - 'python-pysqlite2', - 'python-support', - 'python-pip', + python::virtualenv { $root_dir: } -> + python::pip { [ 'gunicorn', - ]) + 'twisted==11.1.0', + 'django==1.4.10', + 'django-tagging==0.3.1', + 'python-memcached==1.47', + 'simplejson==2.1.6', + ]: + virtualenv => $root_dir, + } + + ensure_packages(['python-cairo']) + file { "${root_dir}/lib/python2.7/site-packages/cairo": + ensure => link, + target => '/usr/lib/python2.7/dist-packages/cairo', + require => [ + Python::Virtualenv[$root_dir], + Package['python-cairo'], + ], + } } diff --git a/manifests/install.pp b/manifests/install.pp index eef8554..f2ec8e6 100644 --- a/manifests/install.pp +++ b/manifests/install.pp @@ -6,37 +6,12 @@ $root_dir = $::graphite::root_dir $ver = $::graphite::version - package { 'whisper': - ensure => $ver, - provider => pip, - } - - $carbon_pip_args = [ - "--install-option=\"--prefix=${root_dir}\"", - "--install-option=\"--install-lib=${root_dir}/lib\"", - ] - $carbon_args = join($carbon_pip_args, ' ') - - exec { 'graphite/install carbon': - command => "/usr/bin/pip install ${carbon_args} carbon==${ver}", - creates => "${root_dir}/bin/carbon-cache.py", - } - - $graphite_pip_args = [ - "--install-option=\"--prefix=${root_dir}\"", - "--install-option=\"--install-lib=${root_dir}/webapp\"" - ] - $graphite_web_args = join($graphite_pip_args, ' ') - - exec { 'graphite/install graphite-web': - command => "/usr/bin/pip install ${graphite_web_args} graphite-web==${ver}", - creates => "${root_dir}/webapp/graphite/manage.py", - } - - file { '/var/log/carbon': - ensure => directory, - owner => www-data, - group => www-data, + python::pip { [ + "whisper==${ver}", + "carbon==${ver}", + "graphite-web==${ver}" + ]: + virtualenv => $root_dir, } } diff --git a/templates/upstart/carbon-cache.conf b/templates/upstart/carbon-cache.conf index 2322b2d..563870e 100755 --- a/templates/upstart/carbon-cache.conf +++ b/templates/upstart/carbon-cache.conf @@ -12,4 +12,4 @@ pre-start exec rm -f '<%= @root_dir %>'/storage/carbon-cache.pid chdir '<%= @root_dir %>' env GRAPHITE_STORAGE_DIR='<%= @root_dir %>/storage' env GRAPHITE_CONF_DIR='<%= @root_dir %>/conf' -exec python '<%= @root_dir %>/bin/carbon-cache.py' --debug start +exec <%= @root_dir %>/bin/carbon-cache.py --debug start diff --git a/templates/upstart/graphite-web.conf b/templates/upstart/graphite-web.conf index 7793902..eef28f2 100644 --- a/templates/upstart/graphite-web.conf +++ b/templates/upstart/graphite-web.conf @@ -11,4 +11,4 @@ chdir '<%= @root_dir %>/webapp' env PYTHONPATH='<%= @root_dir %>/webapp' env GRAPHITE_STORAGE_DIR='<%= @root_dir %>/storage' env GRAPHITE_CONF_DIR='<%= @root_dir %>/conf' -exec /usr/bin/gunicorn_django -b127.0.0.1:<%= @port %> -w2 graphite/settings.py +exec <%= @root_dir %>/bin/gunicorn_django -b127.0.0.1:<%= @port %> -w2 graphite/settings.py From b0eac9b420a041666871a78fd73209c0a07c3d46 Mon Sep 17 00:00:00 2001 From: Dan Carley Date: Wed, 1 Jan 2014 22:37:18 +0000 Subject: [PATCH 3/5] Prevent carbon/graphite from being reinstalled Caused by and workaround taken from: - https://github.com/graphite-project/carbon/issues/86 --- manifests/install.pp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/manifests/install.pp b/manifests/install.pp index f2ec8e6..11c2735 100644 --- a/manifests/install.pp +++ b/manifests/install.pp @@ -11,7 +11,8 @@ "carbon==${ver}", "graphite-web==${ver}" ]: - virtualenv => $root_dir, + virtualenv => $root_dir, + environment => ["PYTHONPATH=${root_dir}/lib:${root_dir}/webapp"], } } From 18b431ec87a07a01f47be4daa3870b17628384f3 Mon Sep 17 00:00:00 2001 From: Dan Carley Date: Thu, 2 Jan 2014 11:24:00 +0000 Subject: [PATCH 4/5] Add tests for virtualenv changes New fixture. Changed paths. No longer testing every package in `::deps`, since it's a bit pointless. --- .fixtures.yml | 1 + .../classes/graphite/graphite__config_spec.rb | 4 +-- spec/classes/graphite/graphite__deps_spec.rb | 31 +++++++++---------- .../graphite/graphite__install_spec.rb | 21 +++++++++---- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/.fixtures.yml b/.fixtures.yml index e6e9d61..28aab38 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -1,5 +1,6 @@ fixtures: repositories: stdlib: git://github.com/puppetlabs/puppetlabs-stdlib.git + python: git://github.com/stankevich/puppet-python.git symlinks: graphite: "#{source_dir}" diff --git a/spec/classes/graphite/graphite__config_spec.rb b/spec/classes/graphite/graphite__config_spec.rb index 1ca1a10..0bd177a 100644 --- a/spec/classes/graphite/graphite__config_spec.rb +++ b/spec/classes/graphite/graphite__config_spec.rb @@ -34,7 +34,7 @@ with_content(/chdir '\/this\/is\/root'/). with_content(/GRAPHITE_STORAGE_DIR='\/this\/is\/root\/storage'/). with_content(/GRAPHITE_CONF_DIR='\/this\/is\/root\/conf'/). - with_content(/python '\/this\/is\/root\/bin\/carbon-cache.py'/). + with_content(/exec \/this\/is\/root\/bin\/carbon-cache.py/). with_mode('0555') } end @@ -111,7 +111,7 @@ end it { - should contain_exec('init-db').with_command('/usr/bin/python manage.py syncdb --noinput'). + should contain_exec('init-db').with_command('/opt/graphite/bin/python manage.py syncdb --noinput'). with_cwd('/opt/graphite/webapp/graphite') } diff --git a/spec/classes/graphite/graphite__deps_spec.rb b/spec/classes/graphite/graphite__deps_spec.rb index 25137fd..940cf18 100644 --- a/spec/classes/graphite/graphite__deps_spec.rb +++ b/spec/classes/graphite/graphite__deps_spec.rb @@ -1,22 +1,19 @@ require 'spec_helper' -describe 'graphite::deps', :type => :class do +describe 'graphite', :type => :class do - [ - 'python-ldap', - 'python-cairo', - 'python-django', - 'python-twisted', - 'python-django-tagging', - 'python-simplejson', - 'python-memcache', - 'python-pysqlite2', - 'python-support', - 'python-pip', - 'gunicorn', - ].each do |pkg| - it { - should contain_package(pkg) - } + context 'root_dir' do + let(:params) {{ + :root_dir => '/this/is/root', + }} + + it { should contain_python__virtualenv('/this/is/root') } + it { should contain_python__pip('gunicorn').with_virtualenv('/this/is/root') } + + it { should contain_package('python-cairo').with_ensure('present') } + it { should contain_file('/this/is/root/lib/python2.7/site-packages/cairo'). + with_ensure('link'). + with_target('/usr/lib/python2.7/dist-packages/cairo'). + with_require(['Python::Virtualenv[/this/is/root]', 'Package[python-cairo]']) } end end diff --git a/spec/classes/graphite/graphite__install_spec.rb b/spec/classes/graphite/graphite__install_spec.rb index 4a8e429..6182be7 100644 --- a/spec/classes/graphite/graphite__install_spec.rb +++ b/spec/classes/graphite/graphite__install_spec.rb @@ -1,11 +1,20 @@ require 'spec_helper' +shared_examples "pip_package" do |package| + it { should contain_python__pip("#{package}==1.2.3"). + with_virtualenv('/this/is/root'). + with_environment(["PYTHONPATH=/this/is/root/lib:/this/is/root/webapp"]) } +end + describe 'graphite', :type => :class do - let(:version) { '0.9.12' } + context 'root_dir and version' do + let(:params) {{ + :version => '1.2.3', + :root_dir => '/this/is/root', + }} - it { should contain_package('whisper').with_ensure(version) } - it { should contain_exec('graphite/install carbon'). - with_command("/usr/bin/pip install --install-option=\"--prefix=/opt/graphite\" --install-option=\"--install-lib=/opt/graphite/lib\" carbon==#{version}") } - it { should contain_exec('graphite/install graphite-web'). - with_command("/usr/bin/pip install --install-option=\"--prefix=/opt/graphite\" --install-option=\"--install-lib=/opt/graphite/webapp\" graphite-web==#{version}") } + it_should_behave_like "pip_package", "whisper" + it_should_behave_like "pip_package", "carbon" + it_should_behave_like "pip_package", "graphite-web" + end end From bbe815f523943a2c067ad4f70a499b39773f514d Mon Sep 17 00:00:00 2001 From: Dan Carley Date: Thu, 2 Jan 2014 11:46:03 +0000 Subject: [PATCH 5/5] Virtualenv support for carbon-aggregator Use binary from virtualenv to load correct search paths. --- spec/classes/graphite/graphite__config_spec.rb | 2 +- templates/upstart/carbon-aggregator.conf | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/classes/graphite/graphite__config_spec.rb b/spec/classes/graphite/graphite__config_spec.rb index 0bd177a..9a37644 100644 --- a/spec/classes/graphite/graphite__config_spec.rb +++ b/spec/classes/graphite/graphite__config_spec.rb @@ -25,7 +25,7 @@ with_content(/chdir '\/this\/is\/root'/). with_content(/GRAPHITE_STORAGE_DIR='\/this\/is\/root\/storage'/). with_content(/GRAPHITE_CONF_DIR='\/this\/is\/root\/conf'/). - with_content(/python '\/this\/is\/root\/bin\/carbon-aggregator.py'/). + with_content(/exec \/this\/is\/root\/bin\/carbon-aggregator.py/). with_mode('0555') } end diff --git a/templates/upstart/carbon-aggregator.conf b/templates/upstart/carbon-aggregator.conf index 65ee06c..a18a8c7 100755 --- a/templates/upstart/carbon-aggregator.conf +++ b/templates/upstart/carbon-aggregator.conf @@ -10,7 +10,7 @@ respawn chdir '<%= @root_dir %>' env GRAPHITE_STORAGE_DIR='<%= @root_dir %>/storage' env GRAPHITE_CONF_DIR='<%= @root_dir %>/conf' -exec python '<%= @root_dir %>/bin/carbon-aggregator.py' \ +exec <%= @root_dir %>/bin/carbon-aggregator.py \ <% if @aggregation_rules_ensure == 'present' -%> --rules='<%= @root_dir %>/conf/aggregation-rules.conf' \ <% end -%>