Enhancing expect

Bug #1253833 reported by Removed by request
6
Affects Status Importance Assigned to Milestone
upstart
New
Undecided
Unassigned

Bug Description

I'm using Ubuntu 14.04 dev with upstart 1.11-0ubuntu1 and I'm missing the ability to use an expect stanza for more complex jobs. Here is an example upstart script:

exec sh -c 'sleep 5 &'

As I know it is not possible to make "sleep 5" as the main process for this job. expect fork/daemon will only trigger on fork() and expect stop only if "sleep 5" would raise a SIGSTOP to itself which is not possible here. Maybe expect stop could be enhanced if this would not cause any other problems or an expect stanza for child processes could be added.

Revision history for this message
Steve Langasek (vorlon) wrote :

> Here is an example upstart script:

> exec sh -c 'sleep 5 &'

I don't get it. What exactly is this an example of? You should obviously write 'exec sleep 5' here, and all of the 'expect' options are then meaningless. Please explain what it is you're trying to accomplish here.

Changed in upstart:
status: New → Incomplete
Revision history for this message
Removed by request (removed3425744) wrote :

It's just a testcase. But to make things easier lets call this line as follows:

exec really_important_command_wrapper_with_locking_and_such_stuff -c 'sleep 5 &'

And the wrapper makes really important things including calling "sleep 5" in the background. Now it would be useful to make "sleep 5" to the main process for this job so that job dependencies can trigger correctly.

Changed in upstart:
status: Incomplete → New
Revision history for this message
Dimitri John Ledkov (xnox) wrote : Re: [Bug 1253833] [NEW] Enhancing expect

On 21 November 2013 23:32, Sworddragon <email address hidden> wrote:
> Public bug reported:
>
> I'm using Ubuntu 14.04 dev with upstart 1.11-0ubuntu1 and I'm missing
> the ability to use an expect stanza for more complex jobs. Here is an
> example upstart script:
>
> exec sh -c 'sleep 5 &'
>
>
> As I know it is not possible to make "sleep 5" as the main process for this job. expect fork/daemon will only trigger on fork() and expect stop only if "sleep 5" would raise a SIGSTOP to itself which is not possible here. Maybe expect stop could be enhanced if this would not cause any other problems or an expect stanza for child processes could be added.
>

Actually above statement is not entirely correct, "expect fork" and
"expect daemon" stop tracking pids on exec, and take the execed pid as
the final. Therefore if one does:

$ cat /etc/init/foo.conf
expect daemon
exec sh -c 'sleep 1000 &'

$ sudo start foo
foo start/running, process 6933

$ ps -axf | grep 6933
root 6933 0.0 0.0 4352 372 ? S 01:38 0:00 sleep 1000

You will notice that it is possible to track sleep process correctly,
in your hypothetical minimal example.
It would be better, if more realistic example of processes & wrapper
scripts involved is given.
Preferably those that are actually in use today by open source projects.

On the other hand, I do see requests for tracking PIDs that may change
(e.g. processes that can re-exec when needed, e.g. upstart itself, or
nginx upgrade method). I am thinking whether we should consider all
processes in the started process group as main process pids, and use
expect stanzas only to establish readiness state. In that case SIGSTOP
raised from any process in the process group, would be a valid
readiness indication. This would be consistent with killing process,
as at the moment we kill all process in the started process group.

Regards,

Dmitrijs.

Changed in upstart:
status: New → Incomplete
Revision history for this message
Removed by request (removed3425744) wrote :

> Actually above statement is not entirely correct, "expect fork" and
> "expect daemon" stop tracking pids on exec, and take the execed pid as
> the final. Therefore if one does:
>
> $ cat /etc/init/foo.conf
> expect daemon
> exec sh -c 'sleep 1000 &'
>
> $ sudo start foo
> foo start/running, process 6933
>
> $ ps -axf | grep 6933
> root 6933 0.0 0.0 4352 372 ? S 01:38 0:00 sleep 1000

Ah, you are right here. I had my thougts on such a wrapper script which I'm using which is calling system() for the sleep command. In that case it wouldn't work anymore.

> In that case SIGSTOP
> raised from any process in the process group, would be a valid
> readiness indication.

You mean something like this?

expect stop
exec sh -c 'sleep 1000 & kill -19 $!'

Changed in upstart:
status: Incomplete → New
Revision history for this message
Dimitri John Ledkov (xnox) wrote : Re: [Bug 1253833] Re: Enhancing expect

On 22 November 2013 02:15, Sworddragon <email address hidden> wrote:
>> Actually above statement is not entirely correct, "expect fork" and
>> "expect daemon" stop tracking pids on exec, and take the execed pid as
>> the final. Therefore if one does:
>>
>> $ cat /etc/init/foo.conf
>> expect daemon
>> exec sh -c 'sleep 1000 &'
>>
>> $ sudo start foo
>> foo start/running, process 6933
>>
>> $ ps -axf | grep 6933
>> root 6933 0.0 0.0 4352 372 ? S 01:38 0:00 sleep 1000
>
> Ah, you are right here. I had my thougts on such a wrapper script which
> I'm using which is calling system() for the sleep command. In that case
> it wouldn't work anymore.
> fo

I'm confused. I gave example, on how to track your process.... you are
still not providing a realistic example.

>
>> In that case SIGSTOP
>> raised from any process in the process group, would be a valid
>> readiness indication.
>
> You mean something like this?
>

No, I do not. You are taking my words out of context. None of what I
said in that paragraph is implemented. "expect stop" tracks the first
pid only (sh, in below job) and emits started signal when that process
(sh, again in the below job) raises stop.

> expect stop
> exec sh -c 'sleep 1000 & kill -19 $!'
>

this is incorrect, as the process tracked is "sh", and nothing raises
sigstop on the "sh" process.

> ** Changed in: upstart
> Status: Incomplete => New
>

Please don't change status, without providing requested information.
Requested information is full examples of jobs tracked & wrappers, not
over-engineered example to track "sleep".
Maybe even better: state what your problem is, that you ended up going
down the path of adding wrapper scripts and yet continue to use
sigstop.

Regards,

Dmitrijs.

Revision history for this message
Steve Langasek (vorlon) wrote : Re: [Bug 1253833] [NEW] Enhancing expect

On Fri, Nov 22, 2013 at 01:49:50AM -0000, Dmitrijs Ledkovs wrote:
> On the other hand, I do see requests for tracking PIDs that may change
> (e.g. processes that can re-exec when needed, e.g. upstart itself, or
> nginx upgrade method).

In fact, there's an open bug report about this, because cups re-execs itself
when daemonizing, which is why the cups upstart job currently runs in the
foreground and uses a horrible poll in the post-start. I have a patch to
change upstart to *not* stop counting forks after a re-exec; but it also
depends on the fix for upstart to count exits.

> I am thinking whether we should consider all processes in the started
> process group as main process pids, and use expect stanzas only to
> establish readiness state. In that case SIGSTOP raised from any process
> in the process group, would be a valid readiness indication. This would
> be consistent with killing process, as at the moment we kill all process
> in the started process group.

I don't see how this would work. If the lead process is still alive, then
the SIGSTOP will be raised to that lead process, not to upstart itself. If
the lead process isn't alive, upstart will notice it has died and won't wait
around for some other process in the pgroup to raise SIGSTOP...

Revision history for this message
Removed by request (removed3425744) wrote :

> you are
> still not providing a realistic example.

I have no example that matches "Preferably those that are actually in use today by open source projects.". I'm just writing my own scripts as needed which uses such a wrapper implementation. Currently it is an old script with outdated code and maybe I could refactor it to avoid the troubles I currently have with Upstart. But now we have the question: Why should we keep on a static construct and give the work to the user if we can simplify a lot here?

> No, I do not. You are taking my words out of context.

No, you have just heavily misunderstood me :)

> None of what I
> said in that paragraph is implemented.

I know.

> this is incorrect, as the process tracked is "sh", and nothing raises
> sigstop on the "sh" process.

This was just an example/question for your hypothetical implementation.

Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Excerpts from Steve Langasek's message of 2013-11-22 03:01:22 UTC:
> On Fri, Nov 22, 2013 at 01:49:50AM -0000, Dmitrijs Ledkovs wrote:
> > On the other hand, I do see requests for tracking PIDs that may change
> > (e.g. processes that can re-exec when needed, e.g. upstart itself, or
> > nginx upgrade method).
>
> In fact, there's an open bug report about this, because cups re-execs itself
> when daemonizing, which is why the cups upstart job currently runs in the
> foreground and uses a horrible poll in the post-start. I have a patch to
> change upstart to *not* stop counting forks after a re-exec; but it also
> depends on the fix for upstart to count exits.
>

Note that sshd also forks and re-execs (then parent exits) this way when
not in foreground mode and it receives SIGHUP.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.