Page MenuHomeFreeBSD

Shutdown jails in reverse order if jail_list is specified
ClosedPublic

Authored by feld on Mar 17 2015, 7:26 PM.

Details

Reviewers
jamie
allanjude
Summary

You can enable specific jails in /etc/rc.conf via jail_list=""
and the order in which you list them affects the order they are started.
However, the order is not reversed during shutdown which can break
dependencies between jails/services and prevent a clean shutdown of a
user's environment.

Diff Detail

Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

feld updated this revision to Diff 4262.Mar 17 2015, 7:26 PM
feld retitled this revision from to Shutdown jails in reverse order if jail_list is specified.
feld updated this object.
feld edited the test plan for this revision. (Show Details)
feld added reviewers: jamie, allanjude.
jamie edited edge metadata.Mar 17 2015, 8:01 PM

It does the right thing in its place. I would change the "_ _j" loop variable should just be _j though - or if you really want the double underscore add it to the local declaration.

I'd also like to see a similar reversal done in the _ALL case. In the usual case (jid not specified) jls will list jails in the order created, so reversing its output is still the right thing to do.

(sorry for the ugly/incorrect "_ _j", but I'm working around some cute wiki-ish autoformatting)

feld updated this revision to Diff 4264.Mar 17 2015, 8:18 PM
feld edited edge metadata.

Use _j per @jamie

Reverse jails for normal "_ALL" shutdown, too.

jamie accepted this revision.Mar 17 2015, 8:28 PM
jamie edited edge metadata.
This revision is now accepted and ready to land.Mar 17 2015, 8:28 PM
feld added a comment.Mar 17 2015, 8:41 PM

The syntax is completely valid from my testing/emulating in a shell script, but I currently do not have a machine with multiple jails where I can truly validate this is working as intended.

Can I get a confirmation or sign off from someone who has a server with multiple jails before we jump to committing this?

It works just as expected:

denethor# service jail stop gritton tct vol afcm backtest
Stopping jails: backtest afcm vol tct gritton.

allanjude edited edge metadata.Mar 18 2015, 1:25 AM
In D2088#9, @jamie wrote:

It works just as expected:
denethor# service jail stop gritton tct vol afcm backtest
Stopping jails: backtest afcm vol tct gritton.

If the user provides a list, are we sure we want to reverse it?

jamie added a comment.Mar 18 2015, 1:54 AM

We need to reverse a list that comes from rc.conf, since there's only the one list for both directions. I think it would be confusing to reverse it in one situation and not in the other.

feld added a reviewer: peter.EditedMar 18 2015, 12:59 PM

Looking for more eyes on this...

To me it makes perfect sense that the list in rc.conf is reversed, and it seems sane to reverse ALL jails otherwise. But the idea that if I give:

# service jail stop jail1 jail2 jail3

and it shuts them down in order: jail3 jail2 jail1

... that might just cause a few broken keyboards.

I'd be curious to know how the Cluster jail maintenance is handled and if that might be an instance where this would be a POLA violation.

feld added a comment.Apr 20 2015, 3:13 PM

Just checking to see if anyone has thought any further about the potential consequences of this.

I'm not particularly invested in keeping the command line the same as rc.conf; if rc.conf reverses and the command line doesn't, that's fine by me.

feld added a comment.Jul 10 2015, 2:33 PM

Would it be better to make this optional?

jail_shutdown_reverse="YES" or something? Shiny new feature for people who care, doesn't cause any POLA violations to those who upgrade?

jamie added a comment.Jul 10 2015, 5:24 PM

There's a right way to do it, which is to use the natural ordering of the jail command (including the depend parameter). But that would require re-writing the rc script to run the jail command once for all the jails, instead of trying to get the ordering on its own. Unfortunately, backward compatibility (e.g. ez-jail wanting these pid files) makes the right way difficult.

So in the meantime, perhaps another knob to twiddle is the way to go. The old pre-conf rc script didn't reverse the order, so POLA is indeed on the side of making it optional.

peter removed a reviewer: peter.Aug 28 2015, 10:42 PM
feld closed this revision.Feb 10 2016, 4:14 PM

superceded by D5233