<?xml version="1.0" encoding="UTF-8"?>
<issue>
  <id>1460</id>
  <project name="Puppet" id="1"/>
  <tracker name="Feature" id="2"/>
  <status name="Closed" id="5"/>
  <priority name="Normal" id="4"/>
  <author name="Peter Meier" id="171"/>
  <assigned_to name="Peter Meier" id="171"/>
  <category name="Red Hat" id="8"/>
  <fixed_version name="0.24.6" id="19"/>
  <subject>enhance redhat puppetmaster init.d script to easy start puppetmaster as a mongrel cluster</subject>
  <description>If you'd like to run puppetmaster in a mongrel cluster you have to start X instances of
puppetmaster with servertype set to mongrel. With this patch you can add more than one port
to the sysconfig file and puppetmaster will automatically be started with severtype set to
mongrel.
You can now even specify an alternate port in the sysconfig file on which puppetmaster
should be run as webrick based server. See sysconfig file for more documentation.</description>
  <start_date>2008-07-29</start_date>
  <due_date></due_date>
  <done_ratio>0</done_ratio>
  <estimated_hours></estimated_hours>
  <custom_fields>
    <custom_field name="Affected version" id="11">0.24.5</custom_field>
    <custom_field name="Keywords" id="12">red hat init script mongrel cluster mongrel_cluster</custom_field>
    <custom_field name="Branch" id="13"></custom_field>
  </custom_fields>
  <created_on>Tue Jul 29 10:09:18 +0000 2008</created_on>
  <updated_on>Mon Oct 20 23:38:34 +0000 2008</updated_on>
  <changesets>
    <changeset revision="b53509b4538cf581d2d306b43c11f729ba9a2568">
      <user name="James Turnbull" id="27"/>
      <comments>Fixed #1460 - enhance redhat puppetmaster init.d script to easy start puppetmaster as a mongrel cluster</comments>
      <committed_on>Mon Oct 20 23:37:56 +0000 2008</committed_on>
    </changeset>
  </changesets>
  <journals>
    <journal id="4082">
      <user name="Peter Meier" id="171"/>
      <notes>commit &quot;c202da&quot;:http://github.com/duritong/puppet/commit/c202da54da409e08efb6225e2190d465bd481ca4 
pushed: &quot;ticket/1460&quot;:http://github.com/duritong/puppet/tree/ticket/1460</notes>
      <details>
      </details>
    </journal>
    <journal id="4151">
      <user name="James Turnbull" id="27"/>
      <notes>If lutter is a happy camper then I'm fine with this.</notes>
      <details>
        <detail old="" name="category_id" property="attr" new="8"/>
        <detail old="10" name="status_id" property="attr" new="11"/>
        <detail old="" name="assigned_to_id" property="attr" new="9"/>
        <detail old="" name="12" property="cf" new="red hat init script mongrel cluster mongrel_cluster"/>
      </details>
    </journal>
    <journal id="4400">
      <user name="James Turnbull" id="27"/>
      <notes>jamtur01 wrote:
&gt; If lutter is a happy camper then I'm fine with this.

David?</notes>
      <details>
      </details>
    </journal>
    <journal id="5032">
      <user name="Peter Meier" id="171"/>
      <notes>I'd like to see that in 0.24.6, this (old) patch will probably fail due to some newer patches (like #1646) but I can rebase my work to the latest version of the branch.</notes>
      <details>
        <detail old="" name="fixed_version_id" property="attr" new="19"/>
      </details>
    </journal>
    <journal id="5061">
      <user name="James Turnbull" id="27"/>
      <notes>immerda - please re-base but you'll need to be quick...</notes>
      <details>
      </details>
    </journal>
    <journal id="5062">
      <user name="David  Lutterkort" id="9"/>
      <notes>James asked me to summarize what we discussed on IRC yesterday:

* there's no need to introduce PUPPETMASTER_PID_DIR as a variable in sysconfig
* the way RETVAL is accumulated (RETVAL=$? || $RETVAL) is wrong; it needs to be 'ret=$?; [ $ret != 0 ] &amp;&amp; RETVAL=$ret'
* make sure that 'status' still works and lists all the instances in the multiport case (similar to 'service nfs status')
</notes>
      <details>
      </details>
    </journal>
    <journal id="5065">
      <user name="Peter Meier" id="171"/>
      <notes>lutter wrote:
&gt; James asked me to summarize what we discussed on IRC yesterday:
&gt; 
&gt; * there's no need to introduce PUPPETMASTER_PID_DIR as a variable in sysconfig
&gt; * the way RETVAL is accumulated (RETVAL=$? || $RETVAL) is wrong; it needs to be 'ret=$?; [ $ret != 0 ] &amp;&amp; RETVAL=$ret'
&gt; * make sure that 'status' still works and lists all the instances in the multiport case (similar to 'service nfs status')

I rebased the patch and addressed the problems you mentioned in commit: commit &quot;32f40e9&quot;:http://github.com/duritong/puppet/commit/32f40e933cfd1d109d2fc93fd14e811a93b2fa3b the commit have been pushed to &quot;tickets/0.24.x/1460&quot;:http://github.com/duritong/puppet/commits/tickets/0.24.x/1460

One thing that appeared on the same time on the mailing list is, that this feature enhancement won't work on rhel/centos versions prior to 5. I have addressed that in a comment where the settings are done to start puppetmaster as mongrel cluster.

The main problem for this issue is on both sides (rhel/centos rc-functions and puppetmaster):

- every puppetmaster instance has to be started with a different pid file. otherwise the cluster setup won't work and only one puppetmaster instance will be started.
- rc-functions prior to rhel/centos 5 doesn't support calling killproc (to stop) and status with a specific pidfile. Therefor we're not able to address each instance of the cluster directly over its pidfile. And as rc-functions assumes only one daemon to be started at once, we have no possibility to address the different instances with the rc-functions.

I dunno if we'd like to introduce such a version incompatibility, on the other side I think most people are running puppetmaster in a mongrel cluster and therefor an init.d-script which is aware of this possibility would be nice. An option would be to provide a different init-script for the different versions. so an old only single instance script for versions prior to 5 and the enhanced script for versions &gt;= 5. On the other side I would see the comment in sysconfig to be sufficient. But I don't know the packaging rules for fedora/rhel for such cases.

I would therefor propose we'll introduce this patch into 0.24.6. However I'm not the person to decide that, so I'll leave this ticket on &quot;Needs design decision&quot;. But I think that we can talk about that even if the patch is already in some rc for 0.24.6 and still can remove it later, not?</notes>
      <details>
      </details>
    </journal>
    <journal id="5066">
      <user name="James Turnbull" id="27"/>
      <notes>Are you sure this is based on 0.24.x?  Looks like it's branched from master to me.</notes>
      <details>
      </details>
    </journal>
    <journal id="5071">
      <user name="David  Lutterkort" id="9"/>
      <notes>immerda wrote:
&gt; 
&gt; I rebased the patch and addressed the problems you mentioned in commit: commit &quot;32f40e9&quot;:http://github.com/duritong/puppet/commit/32f40e933cfd1d109d2fc93fd14e811a93b2fa3b the commit have been pushed to &quot;tickets/0.24.x/1460&quot;:http://github.com/duritong/puppet/commits/tickets/0.24.x/1460

Looks good; two small questions/suggestions:

(1) There's a lot of uses of the expression
&lt;pre&gt;
  [ -n &quot;$PUPPETMASTER_PORTS&quot; ] &amp;&amp; [ ${#PUPPETMASTER_PORTS[@]} -gt 1 ]
&lt;/pre&gt;
it would be more readable if you set somewhere at the beginning of the file
&lt;pre&gt;
  [ -n &quot;$PUPPETMASTER_PORTS&quot; -a ${#PUPPETMASTER_PORTS[@]} -gt 1 ] &amp;&amp; multiport=yes || multiport=no
&lt;/pre&gt;
and in the rest of the script just did tests like '[ $multiport = yes]'

(BTW, the use of &amp;&amp; in the test works, but it's a little ugly)

(2) Did you check if the stock status command does anything useful in the
    multiport case ? It works for nfs - I don't have an issue with
    puppetmaster_status, but less code is always better ;)

&gt; One thing that appeared on the same time on the mailing list is, that this feature enhancement won't work on rhel/centos versions prior to 5. I have addressed that in a comment where the settings are done to start puppetmaster as mongrel cluster.

Yeah, that's quite a headache; but I think documenting that on RHEL &lt; 5
you're SOL is good enough. I don't want to get into the game of shipping
different init files and the ensuing packaging ugliness.

If somebody wants this badly enough for RHEL &lt; 5, they are more than
welcome to send a patch ;)

All in all, I think this is ready to be checked in, though I really would like to
see (1) addressed.
</notes>
      <details>
      </details>
    </journal>
    <journal id="5072">
      <user name="Peter Meier" id="171"/>
      <notes>jamtur01 wrote:
&gt; Are you sure this is based on 0.24.x?  Looks like it's branched from master to me.

ups yeah you're right. sorry for that, I'm somehow I mess this branch stuff always a bit up, let's put 0.25 out :P. But yeah for sure this is for 0.24.x I took now the 0.24.x branch for the new commit.

lutter wrote:
&gt; [...]
&gt; Looks good; two small questions/suggestions:
&gt; 
&gt; (1) There's a lot of uses of the expression
&gt; [...]
&gt; it would be more readable if you set somewhere at the beginning of the file
&gt; [...]
&gt; and in the rest of the script just did tests like '[ $multiport = yes]'
&gt; 
&gt; (BTW, the use of &amp;&amp; in the test works, but it's a little ugly)

ok, I addressed that in the new commit.

&gt; (2) Did you check if the stock status command does anything useful in the
&gt;     multiport case ? It works for nfs - I don't have an issue with
&gt;     puppetmaster_status, but less code is always better ;)

hmm what exactly do you mean with stock status command?

Anyway I did some tests with the status command: I killed for example one instance and the exit code of the status cmd was  then != 0 and it also reported the right process as not running, so I assume it to be working as a status command should?.
 
&gt; &gt; One thing that appeared on the same time on the mailing list is, that this feature enhancement won't work on rhel/centos versions prior to 5. I have addressed that in a comment where the settings are done to start puppetmaster as mongrel cluster.
&gt; 
&gt; Yeah, that's quite a headache; but I think documenting that on RHEL &lt; 5
&gt; you're SOL is good enough. I don't want to get into the game of shipping
&gt; different init files and the ensuing packaging ugliness.

ok then I'm happy. :)

&gt; If somebody wants this badly enough for RHEL &lt; 5, they are more than
&gt; welcome to send a patch ;)

:)
 
&gt; All in all, I think this is ready to be checked in, though I really would like to
&gt; see (1) addressed.

pushed &quot;0e03ffe&quot;:http://github.com/duritong/puppet/commit/0e03ffe48a43cb6f22eb77497ae3042a4a97abdf to &quot;tickets/0.24.x/1460&quot;:http://github.com/duritong/puppet/tree/tickets%2F0.24.x%2F1460</notes>
      <details>
      </details>
    </journal>
    <journal id="5073">
      <user name="Peter Meier" id="171"/>
      <notes></notes>
      <details>
        <detail old="11" name="status_id" property="attr" new="9"/>
        <detail old="9" name="assigned_to_id" property="attr" new="27"/>
      </details>
    </journal>
    <journal id="5090">
      <user name="James Turnbull" id="27"/>
      <notes>This is still branched from master.</notes>
      <details>
        <detail old="9" name="status_id" property="attr" new="15"/>
        <detail old="27" name="assigned_to_id" property="attr" new="171"/>
      </details>
    </journal>
    <journal id="5096">
      <user name="Peter Meier" id="171"/>
      <notes>jamtur01 wrote:
&gt; This is still branched from master.

hmm this time I did for sure: git-checkout origin/0.24.x -b ticket/1460

as well have a look @ the changelog for example:

http://github.com/duritong/puppet/tree/tickets%2F0.24.x%2F1460/CHANGELOG

this is imho definately not from master.</notes>
      <details>
      </details>
    </journal>
    <journal id="5097">
      <user name="Peter Meier" id="171"/>
      <notes>rebased to james' 0.24.x branch</notes>
      <details>
      </details>
    </journal>
    <journal id="5098">
      <user name="James Turnbull" id="27"/>
      <notes>Pushed in commit:&quot;b53509b4538cf581d2d306b43c11f729ba9a2568&quot; in branch 0.24.x</notes>
      <details>
        <detail old="15" name="status_id" property="attr" new="5"/>
      </details>
    </journal>
  </journals>
</issue>
