panel icon indicating copy to clipboard operation
panel copied to clipboard

Wings mishandles signals

Open danny6167 opened this issue 2 years ago • 12 comments

Current Behavior

Wings currently mishandles stop signals in a number of ways.

^C isn’t actually ^C

^C should issue a SIGINT to the container (as per convention and the panels text) however it is handled specially by the panel. The panel translates this into {‘type’: ‘stop’, ‘value’: null } thus instructing wings to handle what should be a SIGINT as a generic container stop. Meaning when Stop() is called in power.go is skips most of the handling and just calls ContainerStop() with no signal specified. When no signal is specified ContainerStop() defaults to SIGTERM, not the SIGINT that was expected. For some applications this will result in a quick death without completing on exit actions such as saving game.

Most ^SIGNALS don’t work at all

Users are supposed to be able to enter ^SIGABRT, ^SIGINT, ^SIGTERM, etc however issues with wings codes prevents the signal from being processed in a format that is correct for the docker API calls. The syscall.<SIGNAL> is passed to the Terminate() function where it is converted to a string representation, and then (after some trimming) is passed to the ContainerKill() function. The docker API reference (https://docs.docker.com/engine/api/v1.18/#kill-a-container) says the API that this function calls expects a string in the format of “SIGINT”, “SIGTERM”, “SIGKILL”, but my testing also shows that it accepts strings such as “int”, “term”, “kill”. syscall.<SIGNAL>.String() returns strings such as “killed”, “interrupt”, or “terminate”. Current wings code trims the “ed” off “killed” and allows that signal to work, however none of the others are turned into valid strings for the API.

All signals are handled by Terminate()

The intended flow of wings code uses Terminate() to handle all signals however Terminate() make the assumptions that once it sends a signal that the container is dead. Not all signals would result in immediate termination of the container and (once the above issues are fixed) the container can stick around long after wings has marked the container as stopped.

Expected Behavior

The correct signal should be sent to the container. Wings should not assume that a container stopped after a signal was sent to it.

Steps to Reproduce

Use different signals as stop actions and observe signals received by the container using bash traps. Also witness errors in wings logs for ^SIGINT, ^SIGABRT, ^SIGTERM

Panel Version

1.11.3

Wings Version

1.11.6

Games and/or Eggs Affected

No response

Docker Image

No response

Error Logs

Output when using ^SIGINT
------
Stacktrace:
Error response from daemon: Cannot kill container: 5839fcb0-190e-4edf-88d1-605a657c2578: invalid signal: interrupt
github.com/pterodactyl/wings/environment/docker.(*Environment).Terminate
        /root/wings/environment/docker/power.go:302
github.com/pterodactyl/wings/environment/docker.(*Environment).Stop
        /root/wings/environment/docker/power.go:165
github.com/pterodactyl/wings/environment/docker.(*Environment).WaitForStop
        /root/wings/environment/docker/power.go:233
github.com/pterodactyl/wings/server.(*Server).HandlePowerAction
        /root/wings/server/power.go:141
github.com/pterodactyl/wings/router/websocket.(*Handler).HandleInbound
        /root/wings/router/websocket/websocket.go:363
github.com/pterodactyl/wings/router.getServerWebsocket.func3
        /root/wings/router/router_server_ws.go:85
runtime.goexit
        /usr/lib/go-1.18/src/runtime/asm_amd64.s:1571

Is there an existing issue for this?

  • [X] I have searched the existing issues before opening this issue.
  • [X] I have provided all relevant details, including the specific game and Docker images I am using if this issue is related to running a server.
  • [X] I have checked in the Discord server and believe this is a bug with the software, and not a configuration issue with my specific system.

danny6167 avatar May 21 '23 01:05 danny6167

I've got the same issue with my panel, Running the same versions for panel and wings. Docker is on version 24.0.2

yahell100 avatar Jun 13 '23 06:06 yahell100

My best guess is still that this is related to the fact that we use eval instead of exec in the entrypoint. It just took this long to get to where it was noticeable.

parkervcp avatar Jun 13 '23 13:06 parkervcp

My best guess is still that this is related to the fact that we use eval instead of exec in the entrypoint. It just took this long to get to where it was noticeable.

The use of eval is an issue, but it is separate to the issues described in this report. The issues I've reported are entirely valid on their own. Swapping to an image that uses exec will result in the signal reaching the process, but you will still only ever be able to send it a SIGTERM due to the issues I reported in this issue.

I've had a few people say it worked a few versions back but nobody could tell me exactly what version they claim it broke. My best guess is that are referring to the version before https://github.com/pterodactyl/panel/issues/4555 was fixed. If people aren't really understanding what's going on they would have seen the container stop and believe it was working as expected.

danny6167 avatar Jun 24 '23 13:06 danny6167

I've had a few people say it worked a few versions back but nobody could tell me exactly what version they claim it broke. My best guess is that are referring to the version before #4555 was fixed. If people aren't really understanding what's going on they would have seen the container stop and believe it was working as expected.

It worked with the last version from Dane. After Matthew starts with updates, it stop working. But it could also be a Docker update. So according to my feeling after

But it is absolutely slowly a NoGo, that Matthew here nothing fixed or times writes something to it.

likes to take the donations and then sticks his head in the sand. Just my 2 cents

gOOvER avatar Jun 24 '23 14:06 gOOvER

It worked with the last version from Dane. After Matthew starts with updates, it stop working. But it could also be a Docker update. So according to my feeling after

Just to clear, you're saying Panel version 1.10.1 with wings 1.7.0 is the Pterodactyl release you claim worked as you expected it to ? If so, I'll spin up an environment running this version in the next couple of days and test it.

But it is absolutely slowly a NoGo, that Matthew here nothing fixed or times writes something to it.

likes to take the donations and then sticks his head in the sand. Just my 2 cents I can understand how some might feel that way, and I totally respect that opinion, but leaving it out of the issue tracker may be more productive for us all.

We have a half ready patch that resolves these issues, Once I can get it clean and PR ready Ill post it.

danny6167 avatar Jun 24 '23 14:06 danny6167

It worked with the last version from Dane. After Matthew starts with updates, it stop working. But it could also be a Docker update. So according to my feeling after

Just to clear, you're saying Panel version 1.10.1 with wings 1.7.0 is the Pterodactyl release you claim worked as you expected it to ? If so, I'll spin up an environment running this version in the next couple of days and test it.

But it is absolutely slowly a NoGo, that Matthew here nothing fixed or times writes something to it. likes to take the donations and then sticks his head in the sand. Just my 2 cents I can understand how some might feel that way, and I totally respect that opinion, but leaving it out of the issue tracker may be more productive for us all.

We have a half ready patch that resolves these issues, Once I can get it clean and PR ready Ill post it.

problem is, Matthew is the only one, which can merge sth

gOOvER avatar Jun 24 '23 14:06 gOOvER

Yep, it know that's a pain. I'd love to see my unrar fix merged as well.

I'm happy to provide a build myself that includes my fixes when my PR is ready.

danny6167 avatar Jun 24 '23 14:06 danny6167

Yep, it know that's a pain. I'd love to see my unrar fix merged as well.

I'm happy to provide a build myself that includes my fixes when my PR is ready.

im happy to help you testing it :)

But even if nobody wants to hear it and I make myself unpopular every time; @matthewpi is the wrong man for this project. Any sponsor should immediately stop supporting it (unless there is a secret project where sponsors get fixes, which I don't really believe).

He should think about handing over the project to someone who is willing to keep it going, or the community should fork the whole thing and continue without Matthew. With Matthew the project is doomed to die and this project is too bad for that. Many people have invested time here and just because one person is not interested, it should not die.

gOOvER avatar Jun 24 '23 14:06 gOOvER

It worked with the last version from Dane. After Matthew starts with updates, it stop working. But it could also be a Docker update. So according to my feeling after

Just to clear, you're saying Panel version 1.10.1 with wings 1.7.0 is the Pterodactyl release you claim worked as you expected it to ? If so, I'll spin up an environment running this version in the next couple of days and test it.

sorry; had overlooked that. It could also be a docker update; they changed something with the signals in 22 ? or 23?; but there we come out again on it, that it is a bug with ptero.

gOOvER avatar Jun 24 '23 15:06 gOOvER

It worked with the last version from Dane. After Matthew starts with updates, it stop working. But it could also be a Docker update. So according to my feeling after

Just to clear, you're saying Panel version 1.10.1 with wings 1.7.0 is the Pterodactyl release you claim worked as you expected it to ? If so, I'll spin up an environment running this version in the next couple of days and test it.

sorry; had overlooked that. It could also be a docker update; they changed something with the signals in 22 ? or 23?; but there we come out again on it, that it is a bug with ptero.

Thank you for the additional info, I'll incorporate that version of docker into my test environment when I can, but I'm a little swamped next few days.

danny6167 avatar Jun 25 '23 12:06 danny6167

I have a patch + build here for testing/comments https://github.com/danny6167/wings/releases/tag/signal-test Keep in mind that only resolves the issue of the incorrect signal being sent to the container - Your images must also handle the signal properly by using exec, or an init system, or other valid signal handlers.

@gOOvER tagged because I know you wanted to test

danny6167 avatar Jul 01 '23 10:07 danny6167

Reopening as I haven't seen anything that fixes this. I'll reconfirm and write a PR for it next week.

danny6167 avatar May 05 '24 10:05 danny6167

@danny6167 I notice that this was merged 3 weeks ago and I wanted to ask if it's in the latest release of Wings. When I run docker stop on a container while observing its console on the Panel, it's detected as crashed and I see no output from the game server indicating a graceful stop. I just installed Pterodactyl last night.

If this isn't in the release yet, how would I get support for this? I'm hoping to migrate my Minecraft servers to Pterodactyl but I need the guarantee of graceful shutdowns if I simply run shutdown on my Linux host.

lyra56k avatar Jul 19 '24 01:07 lyra56k

@danny6167 I notice that this was merged 3 weeks ago and I wanted to ask if it's in the latest release of Wings. When I run docker stop on a container while observing its console on the Panel, it's detected as crashed and I see no output from the game server indicating a graceful stop. I just installed Pterodactyl last night.

If this isn't in the release yet, how would I get support for this? I'm hoping to migrate my Minecraft servers to Pterodactyl but I need the guarantee of graceful shutdowns if I simply run shutdown on my Linux host.

Hey @bobbyl140, This hasn't made it to a released version yet (hopefully soon) but this doesn't have anything to do with how docker stops a container when using docker stop or during system shutdown.

I haven't been working on a shutdown hook script that would do what you want, no guarantees, but if it ever reaches a half usable state I'll let you know.

danny6167 avatar Jul 22 '24 13:07 danny6167

Ah, I’m sorry. If it’s okay to ask, is there anything I could programmatically call to safely shut everything down? Or if I was to try making one myself, where could I start?

lyra56k avatar Jul 22 '24 13:07 lyra56k

My solution I'm building is using systemd shutdown triggers, that calls a script that calls the wings API directly (to wings, not the panel) to request a shutdown and poll till they are all stopped.

danny6167 avatar Jul 22 '24 13:07 danny6167

I see, thank you!

lyra56k avatar Jul 22 '24 13:07 lyra56k