set GOTRACEBACK=crash when running cockroach

[davepacheco]

By default, when Go encounters a fatal runtime error, it dumps some state (some Goroutine stack traces) and exits the process with a non-zero status code. It'd be nice to get core files for these fatal failures. Such core files proved invaluable for debugging #1146 and friends. Go provides a mechanism to dump core on fatal failures by setting GOTRACEBACK=crash in the environment. This causes it to dump its state like normal and then raise SIGABRT. This is how I generated most of the core files that I used to debug those issues.

Getting a core file also requires having configured coreadm properly and potentially updating ulimit -c. These are properties of the developer's environment and I think we should not take ownership of these. In production, I assume we have already configured them or will do so.

There are two places where this needs to be changed:

• in omicron-test-utils: this covers both the test suite and the omicron-dev db-run developer tool
• in the SMF manifest: this covers production

This change:

• updates the dev/db CockroachDB starter infrastructure:
  • add support for setting environment variables, preserving the current default behavior of propagating the current environment into the cockroach process
  • uses that to set GOTRACEBACK=crash by default (this can be overridden)
  • print out the environment variables used
  • tests the above on illumos systems only (because testing this is platform-specific
    -- I'm using pargs -e PID to get the environment of the child process)
  • separately adds GOTRACEBACK=crash to the one-off invocation that we use to run cockroach version
• updates the SMF manifest to set GOTRACEBACK=crash on deployed systems

[smklein]

Getting a core file also requires having configured coreadm properly and potentially updating ulimit -c. These are properties of the developer's environment and I think we should not take ownership of these. In production, I assume we have already configured them or will do so.

Should we priority boost #1597 ? Is this ultimately the sled agent's job?

[davepacheco]

Getting a core file also requires having configured coreadm properly and potentially updating ulimit -c. These are properties of the developer's environment and I think we should not take ownership of these. In production, I assume we have already configured them or will do so.

Should we priority boost #1597 ?

I do think this is critical for MVP. It's valuable as soon as it can be done. I'm not sure whether it's urgent enough to displace other work.

Is this ultimately the sled agent's job?

Will move this to #1597.

[davepacheco]

Also just to be clear this PR is not in any way blocked on #1597.

[smklein]

Nitpick, can we declare this first, and use it to call builder.env? Just to "set the values once"

[davepacheco]

Good call.

[davepacheco]

Fixed in e71b56b.