[cloud] ssh shim part 4: detect stopped vm, and terminate mutagen sessions #352

savil · 2022-12-09T20:46:40Z

Summary

This PR adds logic for the sshshim to check for a live VM. If the VM is not reachable,
then it should terminate the sync session. We label each sync with the machineID
so that cloud-shells to the same vm get the same label for their mutagen sync.

To test for liveness, I currently execute a echo "alive" command in the VM. There
are likely better ways to do this. Suggestions welcome!

NOTE:
For now, I pass-thru the ssh/scp command if the arguments indicate that the server
is not a devbox-vm. In the next PR, I'll see if we can have a dedicated mutagen daemon
via setting MUTAGEN_DATA_DIRECTORY, in which case we can more strongly impose
that the ssh/scp command can be executed only with a liveVM. UPDATE: this works!

NOTE:
I also discovered that I need to manually empty-out the MUTAGEN_PROMPTER env-var.
This is done, because mutagen otherwise will expect a certain (interactive?) prompt
in the arguments to the mutagen CLI. Mutagen does set this env-var on all ssh/scp executions, but I'm a bit unclear on why this is hooked up.

How was it tested?

ensure no active mutagen daemon (next PR should take away this step)
manually start a vm (command omitted due to closed source, but happy to share)
DEVBOX_FEATURE_SSH_SHIM=1 DEVBOX_VM=username@vmaddress devbox cloud shell
cd ~/.config/devbox/ssh/shims and tail -f logs.txt to monitor the shim.
exit the cloud shell
fly m stop <id> to stop the vm
touch foo.txt in the devbox project folder to trigger a sync action
wait for the logs.txt to show the mutagen sync terminate command.
~/.cache/mutagen/bin/mutagen sync list to confirm the session is terminated.

savil · 2022-12-09T20:46:56Z

Current dependencies on/for this PR:

main
- PR [cloud] ssh shim part 1: generate shim files #348
  - PR [cloud] ssh shim part 2: invoke ssh #349
    - PR [cloud] ssh shim part 3: invoke scp #351
      - PR [cloud] ssh shim part 4: detect stopped vm, and terminate mutagen sessions #352 👈
        
        PR [cloud] ssh shim part 5: run own mutagen daemon #355
        
        PR [midcobra] add executionID #358
        
        PR [midcobra] split Telemetry middleware into sentry and segment middlewares #361
        
        PR [sshshim] use midcobra to execute command with sentry middleware #363

This comment was auto-generated by Graphite.

cloud/mutagen/wrapper.go

gcurtis · 2022-12-13T16:01:41Z

cloud/mutagen/wrapper.go

-		newEnv = append(newEnv, fmt.Sprintf("%s=%s", k, v))
+// envAsKeyValueStrings prepares the env-vars in key=value format to add to the command to be run
+//
+// panics if os.Environ() returns an array with any element not in key=value format


Can this happen?

it really cannot, hence I thought it was acceptable to have the panic.

cloud/mutagen/wrapper.go

gcurtis · 2022-12-13T16:45:53Z

cloud/mutagen/wrapper.go

 	for _, cmdEnv := range cmd.Env {
-		if strings.HasPrefix(cmdEnv, "MUTAGEN_SSH") {
+		if strings.HasPrefix(cmdEnv, "MUTAGEN") {
 			envPrint = fmt.Sprintf("%s\n", cmdEnv)


It looks like this will always print the last MUTAGEN_* env var instead of all of them. Is that intentional?

haha, yup, it was for this PR. In one of the subsequent PRs, I fixed it to print all of them.

cloud/mutagenbox/doc.go

gcurtis · 2022-12-13T17:05:09Z

cloud/openssh/sshshim/mutagen.go

+
+	err := cmd.Run()
+	if err != nil {
+		if err.Error() == "exit status 255" {


If you unwrap this to an exec.ExitError you can check the exit code directly.

ah yes, I had meant to do this, but forgot to update the code. Updated!

gcurtis · 2022-12-13T17:06:14Z

cloud/openssh/sshshim/mutagen.go

+
+func checkActiveVM(vmAddr string) (bool, error) {
+
+	cmd := exec.Command("ssh", vmAddr, "echo 'alive'")


You may want to use exec.CommandContext so you can enforce a timeout on the health check.

good call! Added

…sions

…is inherited

… comment

… to check ExitCode

This was referenced Dec 9, 2022

[cloud] ssh shim part 3: invoke scp #351

Merged

[cloud] ssh shim part 2: invoke ssh #349

Merged

[cloud] ssh shim part 1: generate shim files #348

Merged

savil force-pushed the savil/cloud-ssh-4 branch from a0daaf8 to 1c84d2e Compare December 9, 2022 20:49

savil requested review from loreto and gcurtis and removed request for loreto December 9, 2022 20:50

savil marked this pull request as ready for review December 9, 2022 20:50

savil mentioned this pull request Dec 10, 2022

[cloud] ssh shim part 5: run own mutagen daemon #355

Merged

savil force-pushed the savil/cloud-ssh-3 branch from 94078a7 to 5f8aff3 Compare December 12, 2022 16:43

savil force-pushed the savil/cloud-ssh-4 branch from 78d54e9 to 9d0aa27 Compare December 12, 2022 16:43

savil force-pushed the savil/cloud-ssh-3 branch from 5f8aff3 to 04fe82f Compare December 12, 2022 16:52

savil force-pushed the savil/cloud-ssh-4 branch from 9d0aa27 to 7162db4 Compare December 12, 2022 16:52

savil force-pushed the savil/cloud-ssh-3 branch from 04fe82f to b78cf72 Compare December 12, 2022 16:59

savil force-pushed the savil/cloud-ssh-4 branch from 7162db4 to f7f504b Compare December 12, 2022 16:59

Base automatically changed from savil/cloud-ssh-3 to main December 12, 2022 17:11

savil force-pushed the savil/cloud-ssh-4 branch 2 times, most recently from 84b95fd to daa80bf Compare December 12, 2022 18:42

This was referenced Dec 12, 2022

[midcobra] add executionID #358

Merged

[midcobra] split Telemetry middleware into sentry and segment middlewares #361

Closed

[sshshim] use midcobra to execute command with sentry middleware #363

Closed

gcurtis approved these changes Dec 13, 2022

View reviewed changes

savil added 6 commits December 13, 2022 12:15

[cloud] ssh shim part 4: detect stopped vm, and terminate mutagen ses…

48af125

…sions

add os.Environ to merging custom env-vars

eea3b2c

bug fix: ensure MUTAGEN_PROMPTER is not set, but rest of environment …

a133e0b

…is inherited

rename mutagenbox.TerminateSessionsForMachine for clarity; add better…

b8666d7

… comment

minor code review fixes

4a730f6

adding timeout to context for ssh-ing for live VM; use ExitError type…

6524f8e

… to check ExitCode

savil force-pushed the savil/cloud-ssh-4 branch from 4613c2a to 6524f8e Compare December 13, 2022 20:35

savil merged commit ba4a4ac into main Dec 13, 2022

savil deleted the savil/cloud-ssh-4 branch December 13, 2022 20:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[cloud] ssh shim part 4: detect stopped vm, and terminate mutagen sessions #352

[cloud] ssh shim part 4: detect stopped vm, and terminate mutagen sessions #352

Uh oh!

savil commented Dec 9, 2022 •

edited

Loading

Uh oh!

savil commented Dec 9, 2022 •

edited

Loading

Uh oh!

Uh oh!

gcurtis Dec 13, 2022

Uh oh!

savil Dec 13, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gcurtis Dec 13, 2022

Uh oh!

savil Dec 13, 2022

Uh oh!

Uh oh!

gcurtis Dec 13, 2022

Uh oh!

savil Dec 13, 2022

Uh oh!

gcurtis Dec 13, 2022

Uh oh!

savil Dec 13, 2022

Uh oh!

Uh oh!


		func checkActiveVM(vmAddr string) (bool, error) {

		cmd := exec.Command("ssh", vmAddr, "echo 'alive'")

[cloud] ssh shim part 4: detect stopped vm, and terminate mutagen sessions #352

[cloud] ssh shim part 4: detect stopped vm, and terminate mutagen sessions #352

Uh oh!

Conversation

savil commented Dec 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How was it tested?

Uh oh!

savil commented Dec 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

savil commented Dec 9, 2022 •

edited

Loading

savil commented Dec 9, 2022 •

edited

Loading