Log EMR cluster StateChangeReason on transient mode failure

[Oguzhan Unlu (oguzhanunlu)]

In transient mode (run-transient) the cluster is launched with its steps embedded and the runner only waits for it to terminate. When the cluster terminates with errors — provisioning, quota, IAM or networking errors, or a bootstrap failure — the ClusterTerminatedWaiter returns a generic "transitioned to Failure" error and the underlying StateChangeReason was never surfaced, leaving transient runs blind to why the cluster died.

On waiter failure, re-describe the cluster and surface Cluster.Status.StateChangeReason Code and Message both in the logs and in the returned error, mirroring the persistent path added in #98.

ref: https://snplow.atlassian.net/browse/PDP-2682

[Ian Streeter (istreeter)]

The following is unashamedly copy and pasted out of a conversation with Claude. See if you think these are legitimate complaints:

One thing worth considering before we close this off: when a step fails (rather than the cluster failing to provision), EMR terminates the cluster with TERMINATED_WITH_ERRORS. The waiter treats that as a failure and returns an error, so the new StateChangeReason logging will fire — you'll see something like code=STEP_FAILURE in the logs, which is better than nothing. But because we exit immediately at that point, GetFailedStepIDs() and displayFailedStepsLogs() are never reached. So in transient mode, --log-failed-steps effectively does nothing — the flag is accepted but the step-level log output is silently skipped. Worth deciding whether that's in scope here or a separate follow-up.

A couple of smaller things:

• If the re-describe call itself fails (derr != nil), the error is silently dropped and we fall through with the original generic waiter message. A log.Warnf("could not re-describe cluster to get StateChangeReason: %v", derr) would at least tell you why you're seeing the generic message rather than leaving you wondering whether the re-describe was even attempted.

• The nesting inside the if — if desc, derr := ...; derr == nil && desc.Cluster != nil && desc.Cluster.Status != nil — is a bit dense for something that's already inside an error branch. Given that clusterStateChangeReason is already extracted into its own function, pulling the re-describe logic into a small helper (something like fetchStateChangeReason(svc, clusterID)) would make it easier to test and read. Not a blocker.

• The bootstrap-failure retry gap is still present (transient mode won't retry on bootstrap failures the way up does) — but that's probably a separate ticket.

[Oguzhan Unlu (oguzhanunlu)]

So in transient mode, --log-failed-steps effectively does nothing

I checked this concern by reproducing a run-transient execution with --log-failed-steps flag. I can see logs of the failed step, hence the concern isn't valid.

See the transient cluster's events history for a full picture. The cluster provisions fine and a step fails, resulting in TERMINATED, not TERMINATED_WITH_ERRORS (claude's claim).

I pushed a new commit to address the smaller things listed except retry. I think that's a separate ticket.

[Ian Streeter (istreeter)]

OK thanks for checking. It seems Claude was wrong on this one. The logs you are shared are exactly what we need.