[update-status] Handle svcs in-transition and unrecognized states

[karencfv]

svcs has two additional "states" that are not in the list of official states for SMF service instances. From man svcs

Absent or unrecognized states are denoted by a question mark
(?) character. An asterisk (*) is appended for instances in
transition, unless the NSTA or NSTATE column is also being
displayed.

This PR adds two additional variants to SvcState (and related parsing) to account for these additional "states": Unrecognized and InTransition. Additionally, only the Unrecognized variant is added to SvcEnabledNotOnlineState. An unrecognised state should raise an alarm, whereas an "in-transition" state should not.

This should probably be merged until the release branch is cut? It's be nice to give this a little bit of time on the dogfood rack.

Fixes: #10997

[karencfv]

This PR looks a bit big, but it really isn't. Most of it is just API migration code.

[davepacheco]

Suggested change

	// in svcs) is excluded because it is a momentary state
	// in svcs) is excluded because it is a momentary state

[davepacheco]

This seems okay for what we're trying to do with this consumer, but it doesn't seem great for a general-purpose layer because it doesn't tell you anything about what state it's currently in or what it's going to. It means you lose all information about its state while it's transitioning.

I believe that at the SMF layer (in the database, visible with svcprop), this information is exposed as state and next_state, and the asterisk gets appended if next_state is not NULL:

$ svcprop -p restarter ssh
restarter/logfile astring /var/svc/log/network-ssh:default.log
restarter/contract count 71
restarter/start_pid count 432
restarter/start_method_timestamp time 1780716452.355913000
restarter/start_method_waitstatus integer 0
restarter/auxiliary_state astring dependencies_satisfied
restarter/next_state astring none
restarter/state astring online
restarter/state_timestamp time 1780716452.357390000

We could similarly expose both here. Or we could add the current and next state to the Transitioning variant?

Or maybe it would also be okay to simply ignore the asterisk? On the grounds that if it's offline*, then it is offline, even though it's transitioning. But that seems likely to lead to false positives while things are starting up. I think it matters to our consumer whether something is offline or offline* because the first is a problem and the second isn't.

Or might we also have false positives today if something is offline and not transitioning yet because its dependencies are still being started? In which case we just need to treat this at a higher level as transient. Or report the state_timestamp too, and only consider something broken if its in one of our broken states and its state hasn't changed recently (as a form of hysteresis)?

As I write that, I wonder if we're going to keep playing whack-a-mole with false positives unless we do something like that.

[karencfv]

Or might we also have false positives today if something is offline and not transitioning yet because its dependencies are still being started? In which case we just need to treat this at a higher level as transient. Or report the state_timestamp too, and only consider something broken if its in one of our broken states and its state hasn't changed recently (as a form of hysteresis)?

At this point it's starting to feel like we should completely port the svcs -x logic with a few tweaks for our specific use case using scuffle to directly interact with the property groups. Then we wouldn't have to shell out to svcs and do our best to wrangle the output. I'm thinking that this could also be useful for the FM project.

From what I can understand from the explain.c file is that the only time it cares whether there is a state in transition is in the OFFLINE case https://github.com/illumos/illumos-gate/blob/043d968df0a5ae9a27ccc0e22250576f37ecc044/usr/src/cmd/svc/svcs/explain.c#L1824-L1839 . And even there it only cares whether it's starting, otherwise it just calls it DC_TRANSITION regardless of what state it's going to. This is close to what we want but probably not?

Do you think it's worth the effort to do this work? Frankly, it's starting to feel just wrong to continue trying to get so much out of a command line output.

[karencfv]

On the other hand, it could be a lot of work, especially if we want to include information about why things failed, dependencies etc. Where do you think this fits into the current priorities we have?

[davepacheco]

I'd be interested for @jgallagher's opinion. I don't think I'd rewrite all this now to solve this problem. I'm still feeling like we need some kind of hysteresis regardless (so that transient situations don't raise the alarm), and that once we have that, a lot of these details aren't so critical right now.