ddev ssh should return the exit code that is returned from docker-compose and not add extra output

[MichaelSchmidt1729]

Is there an existing issue for this?

• I have searched the existing issues

Run a Diagnostic and Paste Link Here

No response

Current Behavior

When I exit from ddev ssh after having written a command in the container I get an error: "Failed to ddev ssh web: exit status 1" or "Failed to ddev ssh web: exit status 127".

This does not occur if I have not typed a command other than "exit" (i.e. entered and exited again immediately)

Expected Behavior

no error

Steps To Reproduce

see above

Anything else?

No response

[rfay]

Thanks for taking the time to create this issue. See also related

• Exiting ddev ssh results in a reported error even though there is none #1681

Basically ddev ssh exits with the return code of the last command used. In your case I imagine the last command returned a 127 ?

Here's the behavior I see:

$ ddev ssh
rfay@d9-web:/var/www/html$ echo hi
hi
rfay@d9-web:/var/www/html$ exit
logout
rfay@rfay-m1-air:~/workspace/d9$ ddev ssh
rfay@d9-web:/var/www/html$ ls
composer.json  composer.lock  vendor  web
rfay@d9-web:/var/www/html$
logout
rfay@rfay-m1-air:~/workspace/d9$ ddev ssh
rfay@d9-web:/var/www/html$ ls /nothing
ls: cannot access '/nothing': No such file or directory
rfay@d9-web:/var/www/html$
logout
Failed to ddev ssh web: exit status 2
rfay@rfay-m1-air:~/workspace/d9$

[rfay]

It could report better info, like maybe "last command from ddev ssh returned 127"

[jonaseberle]

I think ddev ssh should not echo anything about its exit status.

We are in this general area of command handling and exit codes I think: #3518

I would be in favor of completely removing the Failed to ddev ssh web: exit status XX from ddev ssh and ddev exec.

That ddev ssh does set an exit code can be ignored in my opinion. I think @MichaelSchmidt1729 only wondered about the additional output.

Nota bene: logout/exit (without parameter) shell built-ins capture the last command's exit code and set the current shell's exit code to that before it shuts down. At least on most shell implementations I think. That's totally normal behaviour that can be observed with a plain sh or ssh <host>, too. It does not seem to make a lot of sense for interactive shells, but is of course a requirement for scripts (with or without an implicit exit at the end) so here we go.

[rfay]

People do things like this though:
echo "ls" | ddev ssh

I'm not sure why they do that exactly instead of using ddev exec, but it does affect your logic @jonaseberle .

[jonaseberle]

@rfay Removing that Failed to ddev ssh web: exit status XX would still be the right thing to do IMHO.

[MichaelSchmidt1729]

Thank you! I did not notice the issue. But I agree: "Removing that Failed to ddev ssh web: exit status XX would still be the right thing to do IMHO" A) because of logic B) to not confuse newbies like myself Bye Michael Am Mi., 30. März 2022 um 16:18 Uhr schrieb Jonas Eberle < ***@***.***>:

…

@rfay <https://github.com/rfay> Removing that Failed to ddev ssh web: exit status XX would still be the right thing to do IMHO. — Reply to this email directly, view it on GitHub <#3738 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADA37SA6GLSPILCN4QIGJ63VCRPC7ANCNFSM5SBHHBBQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[tyler36]

I would prefer Failed to ddev ssh web: exit status XX be removed.
I find it confusing and have to remind myself what it means. It happens alot when I run the tinker REPL commands.

I think an option to enable/disable might be a good comprimise. I would definately set it to disabled,

[jonaseberle]

No need for an option IMHO. People should check the exit code properly if they care about it ;)

[rfay]

IIRC the problem was figuring out how to return the proper exit code. Shouldn't be that hard.

[rfay]

This isn't related to this issue, please open a new one when you have an unrelated issue. The problem here is you have something in your .ssh directory that is not a valid key.

[DigitalFrontiersMedia]

AFAICT, this issue was closed by pull #1697 merged into v1.21.4. Running the same test @rfay from Mar 30, 2022, I get no additional output upon exit from ddev ssh using HEAD / v1.21.4:

(base) stephen@MacBook-Pro:dfm-bdw [14:43:55] (master): ddev version
 ITEM             VALUE                                   
 DDEV version     vHEAD-83a8896                           
 architecture     amd64                                   
 db               drud/ddev-dbserver-mariadb-10.4:v1.21.4 
 dba              phpmyadmin:5                            
 ddev-ssh-agent   drud/ddev-ssh-agent:v1.21.4             
 docker           20.10.17                                
 docker-compose   v2.14.0                                 
 docker-platform  docker-desktop                          
 mutagen          0.16.0                                  
 os               darwin                                  
 router           drud/ddev-router:v1.21.4                
 web              drud/ddev-webserver:v1.21.4             
 
(base) stephen@MacBook-Pro:dfm-bdw [14:47:38] (master): ddev ssh
stephen@dfm-bdw-web:/var/www/html$ ls /nothing
ls: cannot access '/nothing': No such file or directory
stephen@dfm-bdw-web:/var/www/html$ logout
(base) stephen@MacBook-Pro:dfm-bdw [14:52:17] (master):

[DigitalFrontiersMedia]

Also noting that this is a duplicate of #1681 unless I am misunderstanding.

[rfay]

I think this is the situation we're talking about:

$ ddev ssh d9
rfay@d9-web:/var/www/html$ ls /xxx
ls: cannot access '/xxx': No such file or directory
rfay@d9-web:/var/www/html$ exit
logout
Failed to ddev ssh web: exit status 2
rfay@rfay-tag1-m1:~/workspace/ddev$ echo $?
1

It did not fail in this situation, we just exited.

Not only is the reporting about "failed to ddev ssh web" incorrect, but the return value should actually be 0.

[DigitalFrontiersMedia]

That is exactly what I get with the changes from #1697:

(base) stephen@MacBook-Pro:dfm-bdw [19:07:52] (master): ddev ssh
stephen@dfm-bdw-web:/var/www/html$ ls /xxx
ls: cannot access '/xxx': No such file or directory
stephen@dfm-bdw-web:/var/www/html$ exit
logout
(base) stephen@MacBook-Pro:dfm-bdw [19:10:47] (master): echo $? 
0

[rfay]

It's possible something got lost? Could you please try current HEAD? You can do that with brew install --HEAD ddev or several other ways. #1697 was in 2019... But I don't really see why it's incorrect in any way.

So the change reversion happened in

• Don't let app.Exec() run against non-running container, fixes #1858 #2830

Which was fixing

• app.Exec() doesn't check for running site/container #1858

But I don't know why it touched ssh.go for what it was trying to fix.

It seems like you're doing great on your exploration! All you have to do now is understand the context of the 3 issues and make it right again! Thanks!

[rfay]

One little detail... It's possible to use ddev ssh like this: echo "ls" | ddev ssh, in which case it should absolutely return the docker-compose return value. It will be interesting to see if that's possible.

[DigitalFrontiersMedia]

Okay. I see. I WAS using HEAD. But I had made the same changes as you had put in #1697 and then I saw #1697, assumed it was still in the codebase, didn't notice the date, didn't test against clean HEAD, so I never saw the regression.

I'm guessing you reversed #1697 and touched ssh.go because someone might ddev ssh on a non-running container as easily as ddev exec except that you already checked for that at line 31 of ssh.go:

		status, _ := app.SiteStatus()
		if status != ddevapp.SiteRunning {
			util.Failed("Project is not currently running. Try 'ddev start'.")
		}

And it, indeed, provides the correct output if it is attempted when the container is not running. So I believe #2830 shouldn't have been done, unless you were thinking of that specific piping of args use-case issue. Will try to spend a little time noodling on that.

[rfay]

I think you'll be able to figure out a path.

Also note that

• Change DockerCompose version to 2.15.1 to test #4565
  may affect this due to a docker-compose issue that went in not long ago.

But yes, I think it would work to check for running service first, then accept the result, rather than depending on the result of the exec to say whether the service is running.

[DigitalFrontiersMedia]

I seem to have hit a chicken-egg problem. Theoretically, you could check in ssh.go if there are values being piped in, and if so, go ahead and return the error (if any); if there are no piped values, don't bother to return an error message. Like so:

		if err != nil && isPiped {
			util.Failed("Failed to ddev ssh %s: %v", serviceType, err)
		}

However, this doesn't work because I can't figure out a way to detect if values have been piped in without robbing that store from the shell, i.e. if I try to detect piped values, then no piped values are actually piped and run.

This:

		// Determine if values were piped into the command.
		var reader = bufio.NewReader(cmd.InOrStdin())
		pipedValues, _ := reader.ReadString('\n')
		isPiped := len(pipedValues) > 0
		
		// Use bash for our containers, sh for 3rd-party containers
		// that may not have bash.
		shell := "bash"
		if !nodeps.ArrayContainsString([]string{"web", "db", "dba", "solr"}, serviceType) {
			shell = "sh"
		}
		err = app.ExecWithTty(&ddevapp.ExecOpts{
			Service: serviceType,
			Cmd:     shell + " -l",
			Dir:     sshDirArg,
		})
		if err != nil && isPiped {
			util.Failed("Failed to ddev ssh %s: %v", serviceType, err)
		}

will never return the error for echo "ls /non-existent" | ddev ssh because the act of reading stdin here appears to consume it before it can be piped and executed.

I tried looking at doing something along these lines further along the chain in ExecWithTty but seemed like it would need to go deeper to what is returned by dockerutil.ComposeWithStreams. However, at these locations, it seems wrong to mess with the return values when the issue seems to really be with what should occur at only the ssh.go scope.

I tried making copies of stdin to read the copy instead, but that yielded no love. But I'm not sure if that was because I'm new to Go and don't know what I'm doing or if that's just not the way the stdin works.

I think I need some advice/direction here to proceed further.

[rfay]

Thanks for working on it! I'll try to take a look in the next couple of days. Maybe you could put up your best effort as a PR first?

[DigitalFrontiersMedia]

Absolutely.

[DigitalFrontiersMedia]

NOTE: In addition to not solving the problem, ddev ssh "hangs" because it appears to be waiting for the stdin input read that was added. Weird.

[rfay]

:)

[DigitalFrontiersMedia]

Also, just confirming that adding a fmt.Println("pipedValues = ", pipedValues) in there correctly outputs the piped in values (when piped in).