Make ow ssh produce a pod you can log into, and not leak it if it can't - #79
Open
vohonen wants to merge 1 commit into
Open
Make ow ssh produce a pod you can log into, and not leak it if it can't#79vohonen wants to merge 1 commit into
ow ssh produce a pod you can log into, and not leak it if it can't#79vohonen wants to merge 1 commit into
Conversation
…an't Two related failures. On a RunPod account you don't administer, `ow ssh` gives you a machine you cannot reach, and then leaves it running. Authorise the caller's key: - `ow ssh` never passed `PUBLIC_KEY` to `create_pod`, so it relied entirely on the key being registered in the RunPod account settings. On a shared account that assumption fails and you get a pod nobody can log into. - `entrypoint.sh` already honours `PUBLIC_KEY` and writes it to `authorized_keys`, so plumbing it through is enough. It travels as an explicit `public_key=` argument rather than inside `env`, because `start_worker` rebuilds `env` from `os.environ` in dev mode and would drop it. - New `--pubkey`, defaulting to `<key-path>.pub`. If no public key is found we warn and carry on rather than failing, since relying on account-registered keys is the only thing that works for callers without a local public key, and `entrypoint.sh` overwrites `authorized_keys` rather than appending to it. Don't leave a pod billing. There were three ways to lose one: - `wait_for_ssh` had no `try`/`except` around it, so a timeout propagated out before `terminate()`. - `bootstrap_remote` called `sys.exit()` on any failing step, which skipped the caller's terminate entirely. This one is the nastiest, because the machine is reachable and healthy, so nothing looks wrong. A missing `unison` in the image is enough to trigger it, in every mode rather than just `--sync`. It now raises `RemoteBootstrapError` and lets the caller decide. - Both paths now go through one `_terminate_after_failure` helper, which is a no-op for `--existing` connections, since a machine we did not create is not ours to terminate. `wait_for_ssh` also discarded stderr, so a rejected key was indistinguishable from a slow boot: `BatchMode=yes` fails instantly and the loop just retried for the full 180 s. It now keeps the last SSH error, reports it, and says outright when the failure looks like authentication rather than boot time. Verified against live RunPod pods, both halves: - provisioning connects on the first attempt, where it previously could not authenticate at all; - pointing `--pubkey` at a key we do not hold the private half of reports `Permission denied (publickey,password)` and says it is authentication rather than a slow boot, then terminates the pod on the spot. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related failures. On a RunPod account you don't administer,
ow sshgivesyou a machine you cannot reach, and then leaves it running.
Authorise the caller's key:
ow sshnever passedPUBLIC_KEYtocreate_pod, so it relied entirely onthe key being registered in the RunPod account settings. On a shared account
that assumption fails and you get a pod nobody can log into.
entrypoint.shalready honoursPUBLIC_KEYand writes it toauthorized_keys, so plumbing it through is enough. It travels as an explicitpublic_key=argument rather than insideenv, becausestart_workerrebuilds
envfromos.environin dev mode and would drop it.--pubkey, defaulting to<key-path>.pub. If no public key is found wewarn and carry on rather than failing, since relying on account-registered
keys is the only thing that works for callers without a local public key, and
entrypoint.shoverwritesauthorized_keysrather than appending to it.Don't leave a pod billing. There were three ways to lose one:
wait_for_sshhad notry/exceptaround it, so a timeout propagated outbefore
terminate().bootstrap_remotecalledsys.exit()on any failing step, which skipped thecaller's terminate entirely. This one is the nastiest, because the machine is
reachable and healthy, so nothing looks wrong. A missing
unisonin the imageis enough to trigger it, in every mode rather than just
--sync. It now raisesRemoteBootstrapErrorand lets the caller decide._terminate_after_failurehelper, which is ano-op for
--existingconnections, since a machine we did not create is notours to terminate.
wait_for_sshalso discarded stderr, so a rejected key was indistinguishablefrom a slow boot:
BatchMode=yesfails instantly and the loop just retried forthe full 180 s. It now keeps the last SSH error, reports it, and says outright
when the failure looks like authentication rather than boot time.
Verified against live RunPod pods, both halves:
authenticate at all;
--pubkeyat a key we do not hold the private half of reportsPermission denied (publickey,password)and says it is authentication ratherthan a slow boot, then terminates the pod on the spot.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com