pelican-dev / wings

MIT License
107 stars 13 forks source link

Wings: Fix stoplogic #31

Open QuintenQVD0 opened 2 weeks ago

QuintenQVD0 commented 2 weeks ago

Changes

The signal must be a string so "SIGTERM" but we used to pass to syscall.SIGTERM and trim some parts but that does not work. so in the function, all incoming signals are converted to signals in string format. They also get matched with their corresponding timeout. What means for any signal the default timeout before it gets killed is 10 seconds. So:

  1. The container gets the signal
  2. it sends the signal to the container
  3. if it exitis in those 10 seconds then it will just continue
  4. if not after 10 seconds, SIGKILL is sent

The timeout is 0 for os.kill as that is what the kill button sends and then it should stop immediately .

this is combined with my panel PR so ^C send SIGTERM.

This also makes wine eggs not need anymore for ^^C but wine can not wait it will just direct exit (how it also does it with ^^C)

you can also now do as stop ^SIGINT, ^SIGTERM, ^SIGABRT what will be converted in to a signal.

This would stop the need for tini but it still can be usefull.

Example

mordhau:

afbeelding Tested with the panel patch with ^C and ^SIGTERM

QuintenQVD0 commented 2 weeks ago

the docker default is SIGTERM and a timeout of 10seconds? and sigkill must happen inimitably ?

and that setting of sig is needed as you must pass a string and not a syscall signal directly?

RMartinOscar commented 2 weeks ago

There's no use to match case for os.Kill when the consequence is the same as the default case

QuintenQVD0 commented 2 weeks ago

There's no use to match case for os.Kill when the consequence is the same as the default case

I changed it.

QuintenQVD0 commented 2 weeks ago

Converted to a draft as their is something wrong with the wings or panel logic itself

QuintenQVD0 commented 1 day ago

IF you revert the this commit: https://github.com/pelican-dev/wings/commit/7bf9f6a9d0f8061bcdbcceeb34372d0f32ad25e6

then stop works but kill is broken

RMartinOscar commented 1 day ago

This may resolve it

QuintenQVD0 commented 1 day ago

This may resolve it

I worked with him and no. Their is something weong with our panel reporting container statuses. I can't realy point to what exactly but this pr does the same just a diffrend way.