Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update jenkins-slave.RedHat init.d script work bash < 4.0 #314

Merged
merged 2 commits into from
Oct 9, 2015
Merged

Update jenkins-slave.RedHat init.d script work bash < 4.0 #314

merged 2 commits into from
Oct 9, 2015

Conversation

andrewegel
Copy link

(First pull request, please be gentle)

Related to issue #225

This change makes the init script work on CentOS-5.x - or really any host using bash less than version 4.0. This is the original error I was receiving:

# service jenkins-slave start
Starting Jenkins Slave...
-bash: -c: line 0: syntax error near unexpected token `>'
-bash: -c: line 0: `/usr/bin/java  -jar /home/jenkins-slave/swarm-client-1.22-jar-with-dependencies.jar -mode normal -executors 1  -name <redacted> -master <redacted> -labels '<redacted>' -fsroot '/home/jenkins-slave'  &>> /var/log/jenkins-slave/jenkins-slave.log &'

This change is more portable than the original command line. So even if this project doesn't want to support CentOS-5.x, this change still guards against shell "gotchas" like this.

@jenkinsadmin
Copy link

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@@ -37,7 +37,7 @@ slave_start() {

# --user in daemon doesn't prepare environment variables like HOME, USER, LOGNAME or USERNAME,
# so we let su do so for us now
$RUNUSER - $JENKINS_SLAVE_USER -c "$JAVA $JAVA_ARGS -jar $JENKINS_SLAVE_JAR $JENKINS_SLAVE_ARGS &>> $JENKINS_SLAVE_LOG &"
$RUNUSER - $JENKINS_SLAVE_USER -c "$JAVA $JAVA_ARGS -jar $JENKINS_SLAVE_JAR $JENKINS_SLAVE_ARGS >> $JENKINS_SLAVE_LOG 2>> $JENKINS_SLAVE_LOG &"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does >> $JENKINS_SLAVE_LOG 2>&1 work with your version of bash? That would be a bit DRYer.

@andrewegel
Copy link
Author

I like your way better - It does indeed work in bash < 4.0 - Pull request modified

@andrewegel
Copy link
Author

@jhoblitt Is this diff to your liking? I would really like to get this change in so we may use this puppet class stock in our infrastructure where I work, and this is the only change needed to get Jenkins "swarm" slaves functioning on CentOS-5.x in our infrastructure.

@andrewegel
Copy link
Author

@jhoblitt & @rtyler Any reason this change hasn't been merged? We (team I work on) would like to use this puppet module, and this change is preventing us from getting this module working on CentOS 5.8 slaves. This seems like a pretty minor change to get it working.

@jhoblitt jhoblitt added the enhancement New feature or request label Oct 9, 2015
@jhoblitt jhoblitt added this to the 1.6.0 - Kato milestone Oct 9, 2015
@jhoblitt
Copy link
Member

jhoblitt commented Oct 9, 2015

The PR passed the acceptance tests on the centos-65-x64 nodeset -- I'm going to go ahead and merge it.

jhoblitt pushed a commit that referenced this pull request Oct 9, 2015
Update jenkins-slave.RedHat init.d script work bash < 4.0
@jhoblitt jhoblitt merged commit 723897f into voxpupuli:master Oct 9, 2015
@andrewegel
Copy link
Author

Thanks @jhoblitt

@jhoblitt
Copy link
Member

@andrew-sumner No, thank you both for work on fixing this and your patience in getting it merged.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants