Init Script Problems

Bug #809646 reported by gholt
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Invalid
Undecided
Unassigned
swift (Ubuntu)
Triaged
Wishlist
Unassigned

Bug Description

I'm not sure if this is the right spot to report this issue, but since the packaging work isn't in it's own project I'll try here.

The branch I'm talking about is:

https://code.launchpad.net/~openstack-ubuntu-packagers/swift/ubuntu

The problem is that the init scripts proposed(?) there do not support 'shutdown' where a service quits accepting new requests and gracefully exits after finishing any active requests. They also do not support 'status'. And the 'reload' as implemented is not graceful and will refuse connections for a second. The swift-init script itself supports these features.

Also, at least with swift-account, there seems to be a leftover "/usr/bin/swift-init account-server $1".

Here's an example of what I'm referring to:

http://bazaar.launchpad.net/~openstack-ubuntu-packagers/swift/ubuntu/revision/37#debian/swift-account.init

Revision history for this message
gholt (gholt) wrote :

As a general note, I'm not sure why the changes are needed. Is there a shortcoming from the original init scripts that is being overcome?

Revision history for this message
Jay Pipes (jaypipes) wrote :

I'm also not sure why Thomas proposed the changes to the init scripts. Also, why is swift-init used in some of the init scripts (swift-object.init for example) and start-stop-daemon used in others...

Revision history for this message
Soren Hansen (soren) wrote :

I can't speak for Thomas per se, but the old init scripts had a number of shortcomings:

If python-swift has been uninstalled, but not purged, /usr/bin/swift-init will be missing, which will make the init script fail. Hence the check for its existence.

log_daemon_msg does The Right Thing[tm] in terms of providing some output about what is being done. Different distros format these things in different ways (or send them to different file descriptors or something entirely different). The log_*_msg functions defined by lsb are the right way to handle these differences.

I've removed the leftover calls to "swift-init ... $1".

As for stop vs. shutdown. I had no clue swift-init's stop wasn't graceful. I'll replace the calls to stop with calls to shutdown.

I've added back support for "status".

As for why some of the scripts use start-stop-daemon and some don't... I'm not sure. Perhaps Thomas can provide some clarity.

Revision history for this message
Thomas Goirand (thomas-goirand) wrote : Re: [Bug 809646] Re: Init Script Problems

On 07/13/2011 03:49 PM, Soren Hansen wrote:
> As for why some of the scripts use start-stop-daemon and some don't...
> I'm not sure. Perhaps Thomas can provide some clarity.

As much as I can remember, so scripts were able to daemonize by
themselves, and generate a PID file. For these, I didn't use
start-stop-daemon. For the others, I had unfortunately no other way. I
wish all this was written with more consistency so that we would never
need start-stop-daemon (it's not as if writing a PID and doing a fork()
call was hard, is it? :) ).

Thomas

Revision history for this message
Soren Hansen (soren) wrote :

2011/7/13 Thomas Goirand <email address hidden>:
> On 07/13/2011 03:49 PM, Soren Hansen wrote:
>> As for why some of the scripts use start-stop-daemon and some don't...
>> I'm not sure. Perhaps Thomas can provide some clarity.
> As much as I can remember, so scripts were able to daemonize by
> themselves, and generate a PID file. For these, I didn't use
> start-stop-daemon. For the others, I had unfortunately no other way. I
> wish all this was written with more consistency so that we would never
> need start-stop-daemon (it's not as if writing a PID and doing a fork()
> call was hard, is it? :) ).

Everything used swift-init until I merged your changes. If they failed
to daemonise appropriately, did their init scripts simply never
return? (On a side note: If that's the case, that should have been
filed as an upstream bug. These things should behave consistently. I
want the packaging to add an additional layer of integration, not be a
collection of workarounds to get stuff to work.)

--
Soren Hansen        | http://linux2go.dk/
Ubuntu Developer    | http://www.ubuntu.com/
OpenStack Developer | http://www.openstack.org/

Revision history for this message
gholt (gholt) wrote :

Ah, the existence check and the log*msg stuff make sense, just wish we (well, you guys) didn't have to duplicate the command list every time but I don't really see a way around that.

As far as 'shutdown' I meant that the command keyword should be added to the list, not replacing 'stop' with it. Both commands exist.

Similarly with 'restart' and 'reload'.

It would be confusing for the init script keywords to do one thing and then a different thing if swift-init is used directly.

You can run swift-init with no args to see what it supports, though obviously the init scripts don't need to support all that.

I'm not sure either on the daemonizing thing. Let us know if it's broken; we haven't had problems in our use of it so it's probably something a bit more subtle to us.

Revision history for this message
Soren Hansen (soren) wrote :

2011/7/13 gholt <email address hidden>:
> As far as 'shutdown' I meant that the command keyword should be added to
> the list, not replacing 'stop' with it. Both commands exist.

I understand.

"stop" is mandatory, so it's what people default to calling when they
want to shut something down. Don't you think letting existing
connections finish is the better default action?

Revision history for this message
Thomas Goirand (thomas-goirand) wrote :

On 07/13/2011 10:05 PM, gholt wrote:
> Ah, the existence check and the log*msg stuff make sense, just wish we
> (well, you guys) didn't have to duplicate the command list every time
> but I don't really see a way around that.
>
> As far as 'shutdown' I meant that the command keyword should be added to
> the list, not replacing 'stop' with it. Both commands exist.

I don't think that "shutdown" is a standard parameter for a init.d
script. Why do you want to add it? Does it do anything different than
"stop"? I see: "shutdown: allow current requests to finish on supporting
servers". But then, how do we implement this in a "normal" script that
doesn't use swift-init?

> Similarly with 'restart' and 'reload'.

I believe that all init.d scripts I wrote have the following:

restart|force-reload|reload)
        $0 stop
        sleep 1
        $0 start
;;

so they do have restart and reload. Is there anything I missed?

> I'm not sure either on the daemonizing thing. Let us know if it's
> broken; we haven't had problems in our use of it so it's probably
> something a bit more subtle to us.

To what I tested, some daemon just wont create PID, or wont go in the
background, and all sorts of "jokes". I cared having the package working
and uploaded in SID, but truth is, Soren is right, I should have
reported it. I just thought there was some weird reasons behind it.

Now, I think we should get rid of swift-init on all startup scripts
completely if possible.

Thomas

Revision history for this message
gholt (gholt) wrote :

Soren, I do think init.d 'stop' should do what swift-init 'stop' does, which is the 'stop now, closing any connections' action. The 'shutdown' is optional for the init.d scripts, but it is used pretty often during upgrades and such.

Thomas, you're right that 'shutdown' isn't a standard parameter of the init.d. I'm not sure on the policy for adding another option. As far as the difference, 'stop' will exit immediately closing any connections, 'shutdown' will stop listening immediately and exit once existing connections have been satisfied (long running uploads/downloads etc. can take quite some time).

For swift-init, 'restart' will do the equivalent of a 'stop' and 'start' with as little time in between as possible.

For swift-init, 'reload' will do the equivalent of a 'shutdown' and 'start' with as little time in between as possible.

If you want to get rid of swift-init completely for your packaging here, you're certainly welcome to. You can call swift-proxy-server directory for instance. SIGHUP is the graceful shutdown, SIGTERM is the stop, that sort of thing. I wouldn't recommend that path, but I'm also not the packager: those who do the work, get the final say. :)

Revision history for this message
Jay Pipes (jaypipes) wrote :

On Wed, Jul 13, 2011 at 11:32 AM, gholt <email address hidden> wrote:
> If you want to get rid of swift-init completely for your packaging here,
> you're certainly welcome to. You can call swift-proxy-server directory
> for instance. SIGHUP is the graceful shutdown, SIGTERM is the stop, that
> sort of thing. I wouldn't recommend that path, but I'm also not the
> packager: those who do the work, get the final say. :)

FWIW, this is what Glance's scripts do. I have a version of swift-init
for Glance (glance-control) that is essentially swift-init before the
recent improvements to it. I use glance-control heavily in testing (it
provides an easy way to, for instance, pass in test config files and
start and stop multiple servers. And I leave the daemonization to the
upstart scripts for the distro-supported packaging...

-jay

Revision history for this message
Thomas Goirand (thomas-goirand) wrote :

On 07/13/2011 11:32 PM, gholt wrote:
> Soren, I do think init.d 'stop' should do what swift-init 'stop' does,
> which is the 'stop now, closing any connections' action. The 'shutdown'
> is optional for the init.d scripts, but it is used pretty often during
> upgrades and such.
>
> Thomas, you're right that 'shutdown' isn't a standard parameter of the
> init.d. I'm not sure on the policy for adding another option. As far as
> the difference, 'stop' will exit immediately closing any connections,
> 'shutdown' will stop listening immediately and exit once existing
> connections have been satisfied (long running uploads/downloads etc. can
> take quite some time).

Adding non-standard stuff in an init.d script is fine, as much as I know.

> For swift-init, 'restart' will do the equivalent of a 'stop' and 'start'
> with as little time in between as possible.
>
> For swift-init, 'reload' will do the equivalent of a 'shutdown' and
> 'start' with as little time in between as possible.

We are free to implement what we want for a reload. However, again,
swift-init is a bad idea, IMHO. As much as I understand, it was never
designed to begin with, it was just written by a developer for
convenience. I think that instead of having init.d script calling
swift-init, we should have the opposite way: swift-init should become a
userland tool that calls the init.d scripts when the user asks. What do
you think?

> If you want to get rid of swift-init completely for your packaging here,
> you're certainly welcome to. You can call swift-proxy-server directory
> for instance. SIGHUP is the graceful shutdown, SIGTERM is the stop, that
> sort of thing. I wouldn't recommend that path, but I'm also not the
> packager: those who do the work, get the final say. :)

As wrote Soren, the big issue with swift-init is that it does some
printings on screen, which it shouldn't: lsb-base functions should do
that and decide how, depending on the distribution, device type output,
etc. Think about the fact that maybe, someone one day might want
lsb-base to do its output on a web page (why not?), or maybe even light
up some hardware LEDs, and you get the idea. lsb-base is an abstraction
layer that we really should use, and which is part of so many distributions.

Thomas

Revision history for this message
Thomas Goirand (thomas-goirand) wrote :

On 07/13/2011 10:28 PM, Soren Hansen wrote:
> 2011/7/13 gholt <email address hidden>:
>> As far as 'shutdown' I meant that the command keyword should be added to
>> the list, not replacing 'stop' with it. Both commands exist.
>
> I understand.
>
> "stop" is mandatory, so it's what people default to calling when they
> want to shut something down.

Well, more when you go on the shutdown runlevel, and the system is
calling each init.d scripts! People can adapt to different kind of
commands, but insserv can't.

> Don't you think letting existing
> connections finish is the better default action?

Unfortunately, I don't. If you have a remote connection that is taking a
lot of time until done, you really don't want it to prevent your server
from rebooting (said otherwise: you don't want to wait forever).

Thomas

Revision history for this message
Thierry Carrez (ttx) wrote :

For future cases, packaging bugs can be directly filed against ubuntu/swift.
Follow the "Report a bug" link here: https://bugs.launchpad.net/ubuntu/+source/swift

Changed in swift (Ubuntu):
assignee: nobody → Soren Hansen (soren)
Changed in swift:
status: New → Invalid
Soren Hansen (soren)
Changed in swift (Ubuntu):
status: New → Confirmed
importance: Undecided → Low
Soren Hansen (soren)
Changed in swift:
assignee: Soren Hansen (soren) → nobody
Changed in swift (Ubuntu):
assignee: Soren Hansen (soren) → nobody
Revision history for this message
James Page (james-page) wrote :

The majority of this thread appears to be about the init scripts which are no longer used in the Ubuntu packaging, which uses upstart configurations which call out to swift-init.

Custom commands are not possible in upstart per-say; the right way todo those would be to introduce additional configurations for shutdown/reload etc.

Changed in swift (Ubuntu):
status: Confirmed → Triaged
importance: Low → Wishlist
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.