Default values for WAIT_STATE are wrong in the upstart wait-for-state job

[Rarylson Freitas]

In Ubuntu 14.04, the wait-for-state job uses the env vars WAIT_STATE="started" or WAIT_STATE="stopped". if the GOAL env var is set to 'start' or 'stop'.

However, according to the upstart cookbook, the desired states for a already started/stopped job are 'start/running' and 'stop/waiting'. Maybe a misunderstood has occurred was the meaning of signals was job states (for example, after a job reaches the start/running state, it emits the started signal).

The strange thing is that the waiting/running wait states were proposed at the first implementations of this job, and not the stopped/started states: https://lists.ubuntu.com/archives/upstart-devel/2011-February/001405.html.

This bug can make some developers to write upstart jobs that return errors (like the GlusterFS developers, that forgot the WAIT_STATE="running" condition).

References:

- http://upstart.ubuntu.com/cookbook/#job-states
- http://upstart.ubuntu.com/cookbook/#events-not-states
- https://bugs.launchpad.net/ubuntu/+source/glusterfs/+bug/1465382

I'm attaching a patch too.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "wait-for-state.conf.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Serge Hallyn (serge-hallyn)]

Hi,

could you please show a specific example which breaks currently?

[Rarylson Freitas (rarylson)]

Of course.

I actually only know two examples:

- mounting-glusterfs.conf: The glusterfs-client package uses the command "wait-for-state WAIT_FOR=static-network-up WAITER=mounting-glusterfs" without passing "WAIT_STATE=running";
    - So, this command doesn't works;
    - See: https://bugs.launchpad.net/ubuntu/+source/glusterfs/+bug/1465382 and https://bugzilla.redhat.com/show_bug.cgi?id=1231983;

- nova-compute.conf: The nova-compute package uses the comand wait-for-state, but passes the WAIT_STATE=running option ("running", in my opinion, should be the default instead of "started"):

  # If libvirt-bin is installed, always wait for it to start first
  if status libvirt-bin; then
    start wait-for-state WAIT_FOR=libvirt-bin WAIT_STATE=running WAITER=nova-compute
  fi

So, it works.

Again, in the link http://upstart.ubuntu.com/cookbook/#job-states, we can sse that 'start/running' and 'stop/waiting' are the correct states, and not the 'start/started' and 'stop/stopped'.

[Michael Terry (mterry)]

Moved the bug to be against Trusty, since current Ubuntus don't use a system upstart.

[Martin Pitt (pitti)]

Changing the default behaviour in 14.04 in our central init seems quite risky to me. Can you please explain in some more detail/with an example which packages are affected here?

In particular, bug 1465382 has a patch which seems to be a more local workaround for this one, right? Or do these two need to go together?

[Martin Pitt (pitti)]

Closing for now as there hasn't been a response in several months. Please reopen if you still want to go ahead with this as an SRU, and then please add a test case and a regression potential analysis.

[Rarylson Freitas (rarylson)]

Hi,

I'm sorry for don't response these questions in the last months.

However, in the last days, I had some time and I understood better the situation.

I'll split my post in three parts.

1 - Technical background

1.1 - Lack of documentation

The `wait-for-start` job is not documented. Because of this, it's more difficult to understand what should be its expected behavior.

See: https://bugs.launchpad.net/ubuntu/+source/upstart/+bug/962047

1.2 - Upstart job states

According do the Upstart Cookbook:

> States are exposed to users via the status field in the output of the initctl status command

See: http://upstart.ubuntu.com/cookbook/#job-states

`waiting` and `running` are the two most common states of a job (they are the initial and final states after running the `initctl start` command). There are other ones (intermediate states), like the `running` and `starting` states.

Notes: `started` and `stopped` are not job states (they are events, but not states). So, running `initctl status` will never print the `started` or `stopped` strings.

1.3 - `wait-for-state.conf` uses `started`/`stopped` as the default expected states

I don't know if it's a buggy behavior or if it's intentional. In the code of the `wait-for-state.conf` file, we have:

```
env WAIT_STATE="started"
env TARGET_GOAL="start"

[...]

    if [ "$WAIT_STATE" = "stopped" ] ; then
        TARGET_GOAL="stop"
    fi
```

And `WAIT_STATE` is used in this line:

```
    # Already running/stopped?
    status $WAIT_FOR | grep -q "$TARGET_GOAL/$WAIT_STATE" && exit 0
```

1.4 - Test case - Apport

So, if we use (for example) `WAIT_FOR=apport` and keep the default `TARGET_GOAL`/`WAIT_STATE` values, wait-for-state will not return true when apport is already started.

If apport is stopped:

```
$ stop apport
apport stop/waiting
$ time start wait-for-state WAIT_FOR=apport WAITER=apport
wait-for-state stop/waiting

real 0m0.035s
user 0m0.000s
sys 0m0.006s
$ echo $?
0
$ status apport
apport start/running
$ time start wait-for-state WAIT_FOR=apport WAITER=apport
start: Job failed to start

real 0m30.019s
user 0m0.000s
sys 0m0.004s
$ echo $?
1
```

This is because wait-for-start grep's for "start/started" instead of "start/running".

However, if we explicitly pass WAIT_STATE=running, it will work!!!

```
$ status apport
apport start/running
$ time start wait-for-state WAIT_FOR=apport WAITER=apport WAIT_STATE=running
wait-for-state stop/waiting

real 0m0.018s
user 0m0.000s
sys 0m0.004s
$ echo $?
0
```

This is because I think the default values should be `running`/`waiting` instead of `started`/`stopped`. So the code would be idempotent (it will return 0, even if the job was already started).

2 - Nova compute, GlusterFS and plymouth-shutdown cases

The `nova-compute.conf` job (`nova-compute` package from `deb http://ubuntu-cloud.archive.canonical.com/ubuntu trusty-updates/kilo main`) has the following lines in their `pre-start script` stanza:

```
  # If libvirt-bin is installed, always wait for it to start first
  if status libvirt-bin; then
    start wait-for-state WAIT_FOR=libvirt-bin WAIT_STATE=running WAITER=nova-compute
  fi
```

Because they explicitly pass `WAIT...