hubspotdevops / puppet-nexus

Puppet module for Sonatype Nexus
MIT License
24 stars 92 forks source link

Fix for issue25 #26

Closed sharkannon closed 10 years ago

sharkannon commented 10 years ago

Fix for service issue when doing a puppet run.

tmclaugh commented 10 years ago

What version are you using? I don't see this in our 2.8.0 setup and I remember explicitly setting the user to root to resolve a different issue.

sharkannon commented 10 years ago

I'm using 3.7.1 (Also used 3.6) and had this issue. I can actually see exactly why it's failing from a SERVICE perspective, the application isn't running as ROOT so as far as it's concerned it's not running.

tmclaugh commented 10 years ago

Sorry, meant what version of Nexus. We run the nexus process as $nexus_root and do not see this issue.

sharkannon commented 10 years ago

Running 2.9.1 of nexus.. I'm just talking about the logic of the "status".. maybe running as root in an older version was a "bug" in nexus.

tmclaugh commented 10 years ago

Okay. I'll have to sort this out. :-/

tmclaugh commented 10 years ago

How about this off the top of my head. Pass in $version to nexus::package and then what's below. Try and test it?


if $version !~ /\d.*/ or versioncmp($version, '2.9.0') >= 0 {
  status_line = "env run_as_user=${nexus_user} /etc/init.d/nexus status"
} else {
  status_line = 'env run_as_user=root /etc/init.d/nexus status'
}

service{ 'nexus':
  ensure => running,
  enable => true,
  status => $status_line,
  require => [File['/etc/init.d/nexus'],
              File_line['nexus_NEXUS_HOME'],
              File_line['nexus_RUN_AS_USER'],]
}
sharkannon commented 10 years ago

Hem, still seems kinda strange that you have to set the run_as user to root for status. I was using 2.8 before with this configuration and it worked fine. Can you send me a copy of your /etc/init.d/nexus script? I'm curious as to why it should run like that.

tmclaugh commented 10 years ago

The run_as_user was introduced here in https://github.com/hubspotdevops/puppet-nexus/commit/467b2518ffe809748b41e51b2f057fa005787ff1. We must've been using the 2.7 series here then. I wish I explained the issue better. :-/ drop versioncmp() to check for 2.8.0 and greater and we can move on. Not going to hubnt down what was wrong with 2.7.

sharkannon commented 10 years ago

All good man, was just trying to help fix the issue without a stupid "If version" hack in there :) Specially since the "status" command of an init script should run as the same user as the command itself executes.. was trying to track down where the actual problem was :)

tmclaugh commented 10 years ago

Yeah, I recalled adding it to address a specific issue but looks like it is no longer a problem. I have #31 opened to add this. If you want to update your PR with the change be my guest and I'll merge this PR. I have a meeting right now so I won't merge anything for almost an hour. :)

sharkannon commented 10 years ago

My pull shouldn't have the version comparison, I never touched that

tmclaugh commented 10 years ago

Was just asking if you wanted to make the suggested change in your branch and I'd commit that.

sharkannon commented 10 years ago

Sorry, not sure which one you mean.

tmclaugh commented 10 years ago

Issue resolved in #31 so closing.