Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Merge pull request #32 from akumria/vhost-logroot

virtualhost logroot location
  • Loading branch information...
commit c2c75a62b7dafced8a6120a14bacfc8be7539282 2 parents c590e64 + 11ad3fc
James Turnbull authored June 16, 2012
19  manifests/vhost.pp
@@ -42,7 +42,8 @@
42 42
     $redirect_ssl       = $apache::params::redirect_ssl,
43 43
     $options            = $apache::params::options,
44 44
     $apache_name        = $apache::params::apache_name,
45  
-    $vhost_name         = $apache::params::vhost_name
  45
+    $vhost_name         = $apache::params::vhost_name,
  46
+    $logroot            = "/var/log/" + $apache::params::apache_name
46 47
   ) {
47 48
 
48 49
   include apache
@@ -67,13 +68,27 @@
67 68
     }
68 69
   }
69 70
 
  71
+  file {"${apache::params::vdir}/${priority}-${name}-$docroot":
  72
+    path => $docroot,
  73
+    ensure => directory,
  74
+  }
  75
+
  76
+  file {"${apache::params::vdir}/${priority}-${name}-$logroot":
  77
+    path => $logroot,
  78
+    ensure => directory,
  79
+  }
  80
+
70 81
   file { "${priority}-${name}.conf":
71 82
       path    => "${apache::params::vdir}/${priority}-${name}.conf",
72 83
       content => template($template),
73 84
       owner   => 'root',
74 85
       group   => 'root',
75 86
       mode    => '0755',
76  
-      require => Package['httpd'],
  87
+      require => [
  88
+          Package['httpd'],
  89
+          File["${apache::params::vdir}/${priority}-${name}-$docroot"],
  90
+          File["${apache::params::vdir}/${priority}-${name}-$logroot"],
  91
+      ]
77 92
       notify  => Service['httpd'],
78 93
   }
79 94
 
4  templates/vhost-default.conf.erb
@@ -21,9 +21,9 @@ NameVirtualHost <%= vhost_name %>:<%= port %>
21 21
     Order allow,deny
22 22
     allow from all
23 23
   </Directory>
24  
-  ErrorLog /var/log/<%= scope.lookupvar("apache::params::apache_name") %>/<%= name %>_error.log
  24
+  ErrorLog <%= logroot %>/<%= name %>_error.log
25 25
   LogLevel warn
26  
-  CustomLog /var/log/<%= scope.lookupvar("apache::params::apache_name") %>/<%= name %>_access.log combined
  26
+  CustomLog <%= logroot %>/<%= name %>_access.log combined
27 27
   ServerSignature Off
28 28
 </VirtualHost>
29 29
 

10 notes on commit c2c75a6

Jari Bakken
jarib commented on c2c75a6 July 30, 2012

This change is giving me a slight headache:

  • It forces anyone with more than one vhost to override $logroot, or Puppet will complain of File['/var/log/apache2'] being defined twice.
  • Often the parent directory of $docroot is a symlink that is managed outside Puppet (e.g. with Capistrano or other deployment tools). After this change, the user must create the parent directories of $docroot or the file resource here will fail, which means there's no way of keeping Puppet from reverting app deployments!

@jamtur01 or @akumria: Thoughts?

Anand Kumria

Hmm, I'm not seeing the same problem.

I also deploy, actually via puppetlabs/vcsrepo, things managed externally.

I use it as:

apache::vhost { 'www.example.com':
    priority           => '10',
    vhost_name         => 'w.x.y.z',
    port               => '80',
    docroot            => '/home/wwwexamplecom/docroot/',
    logroot            => '/home/wwwexamplecom',
    servername         => 'www.example.com',
    serveradmin        => 'webmaster@example.com',
    serveraliases      => ['example.com', ],
    template           => 'apache/vhost-example.conf.erb',
    configure_firewall => false,
}

apache::vhost { 'www.example.com-IPv6':
    priority           => '10',
    vhost_name         => 'a:b:c:w:x:y:z:d',
    port               => '80',
    docroot            => '/home/wwwexamplecom/docroot/',
    logroot            => '/home/wwwexamplecom',
    servername         => 'www.example.com',
    serveradmin        => 'webmaster@example.com',
    serveraliases      => ['example.com', ],
    template           => 'apache/vhost-example.conf.erb',
    configure_firewall => false,
    ensure_dirs        => false
}

Perhaps to solve the first problem you need to have ensure_dirs set to false?

Actually, I think in your case if you set ensure_dirs to false it will solve both issues?

Does that work for you?

Jari Bakken
jarib commented on c2c75a6 July 30, 2012

An ensure_dirs option would indeed solve both issues, but I can't see it in the current HEAD, so I guess I'll have to wait for your pull request to be merged :)

Personally I'd suggest making the ensure_dirs option false by default; then it won't break backwards compatibility and be less of a surprise for users who already have their own docroot resource.

Thanks!

Joe Topjian

I'm seeing the same behavior as @jarib but with docroots as well as logroots. Creating multiple vhosts that use the same docroot will yield an error. I also agree that ensure_dirs would solve the problem, but it seems more like a hack.

Wouldn't the better solution be to use virtual resources and realize a single directory resource?

If ensure_dirs is to be used, though, I agree that it should be false by default for compatibility. Also, maybe it should be broken out of the ipv6 pull request? Maybe it would make it to HEAD sooner... but unless I am mistaken, virtual resources should be the better way.

Jari Bakken
jarib commented on c2c75a6 July 31, 2012

Just to explain my second scenario:

A typical Rails deployment with Capistrano will set up things like this:

symlink: /webapps/my-app/current -> /webapps/my-app/releases/201208011200/
$docroot: /webapps/my-app/current/public 

So it seems that even with ensure_dirs set to false, the fact that File["${priority}-${name}.conf"] depends on File[$docroot] means that I must create the docroot with puppet.

On deployment, the symlink is updated to the correct timestamped directory. If puppet is then run some time after deploy, it will revert the symlink to whatever it thinks is right. So I can't see how to make this work as long as this module won't allow me to point the docroot at a non-existing directory.

EDIT: Ooops, never mind. Looks like ensure_dirs => false will remove the dependencies on $docroot, so the issue goes away.

I agree with @jtopjian that ensure_dirs seems somewhat hacky.

Anand Kumria
Jari Bakken
jarib commented on c2c75a6 July 31, 2012

Why isn't docroot set to '/webapps/my-app/current' ?

The mod_rails/passenger Apache module wants docroot pointed at Rails' public/ directory, whereas /webapps/my-app/current holds the actual application code. Though even if I could use that as docroot, Puppet would still need to know what release to point the symlink at, which is what breaks the intended separation between configuring the server (with Puppet) and deploying new versions of the app (with Capistrano).

Anyway, I realize ensure_dirs => false will do the trick. I don't have enough Puppet experience to tell whether virtual resources is a better solution.

Anand Kumria
Joe Topjian
Dan Bode

this actually looks like it should not have been merged. By default, everything in a defined resource type should be parameterized so that multiple instances can be declared.

Hunter Haugen
Owner

Virtual resources still have to be only defined a single time per node, usually via a parameterized class. But this would have to be done at the level of the original apache::vhost definitions (increasing the LOC and human overhead) otherwise you run the risk of creating duplicate resources in the same way that you are trying to avoid.

The use of defined() or ensure_resource() in #54 is a much more direct way to code around this problem of ambiguously-named potentially-conflicting resources.

Please sign in to comment.
Something went wrong with that request. Please try again.