migrate github.com/docker/docker/pkg/signal #69

thaJeztah · 2021-07-09T21:14:25Z

Migrating this package from commit:
moby/moby@c81abef

Strategy taken:

# install filter-repo (https://github.com/newren/git-filter-repo/blob/main/INSTALL.md)
brew install git-filter-repo

cd ~/projects

# create a temporary clone of docker
git clone https://github.com/docker/docker.git moby_signal
cd moby_signal

# remove all code, except for pkg/signal, and rename to /signal
git filter-repo  --path pkg/signal --path-rename pkg/signal:signal

# go to the target github.com/moby/sys repository
cd ~/projects/moby-sys

# create a branch to work with
git checkout -b integrate_moby_signal

# add the temporary repository as an upstream and make sure it's up-to-date
git remote add moby_signal ~/projects/moby_signal
git fetch moby_signal

# merge the upstream code
git merge --allow-unrelated-histories --signoff -S moby_signal/master

After this:

signal: update import-paths
signal: add go.mod
Makefile: enable tests for signals
signal: remove logrus dependency
signal: remove gotest.tools dependency
signal: remove github.com/pkg/errors dependency

Docker-DCO-1.1-Signed-off-by: Guillaume J. Charmes <guillaume@charmes.net> (github: creack)

Docker-DCO-1.1-Signed-off-by: Kato Kazuyoshi <kato.kazuyoshi@gmail.com> (github: kzys)

Docker-DCO-1.1-Signed-off-by: Guillaume J. Charmes <guillaume@charmes.net> (github: creack)

Docker-DCO-1.1-Signed-off-by: Andrew Page <admwiggin@gmail.com> (github: tianon)

Fix various MAINTAINERS format inconsistencies

Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)

Bump version to v0.11.0

Merge release to master v0.11.0

Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)

Bump to version v0.11.1

Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)

Merge release v0.11.1

Bump version to v0.12.0

Merge release v0.12.0

Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)

Bump version to v1.0.0

Merge release v1.0.0

as a maintainer. Best of luck on your e-commerce business Guillaume, and thanks for all the great contributions! Docker-DCO-1.1-Signed-off-by: Solomon Hykes <solomon@docker.com> (github: shykes)

…me_on_his_new_business_and_no_longer_available_as_a_maintainer

Bump v1.0.1

Merge release v1.0.1

Bump v1.1.0

Merge release v1.1.0

Signed-off-by: Solomon Hykes <solomon@docker.com>

Cleanup: refactor shutdown and signal handling facility

Signed-off-by: Michael Crosby <michael@docker.com>

Exit after receiving SIGTERM

Bump v1.2.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

full diff: gotestyourself/gotest.tools@v2.3.0...v3.0.1 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

…64el error log : signal_test.go:20: assertion failed: error is not nil: Invalid signal: SIGEMT signal_test.go:22: assertion failed: When "ParseSignal" function parse sigStr from SignalMap, it find the signal object with key ("SIG"+sigStr). But EMT signal named "SIGEMT" in SignalMap structrue, so the real key is "SIGSIGEMT" , and cannot find the target signal. modify "SIGEMT" to "EMT" in SignalMap structrue. Signed-off-by: liuxiaodong <liuxiaodong@loongson.cn>

TestCatchAll, TestStopCatch: remove unneeded goroutine

Do not handle SIGURG on Linux, as in go1.14+, the go runtime issues SIGURG as an interrupt to support preemptable system calls on Linux. This issue was caught in TestCatchAll, which could fail when updating to Go 1.14 or above; === Failed === FAIL: pkg/signal TestCatchAll (0.01s) signal_linux_test.go:32: assertion failed: urgent I/O condition (string) != continued (string) signal_linux_test.go:32: assertion failed: continued (string) != hangup (string) signal_linux_test.go:32: assertion failed: hangup (string) != child exited (string) signal_linux_test.go:32: assertion failed: child exited (string) != illegal instruction (string) signal_linux_test.go:32: assertion failed: illegal instruction (string) != floating point exception (string) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Other Unix platforms (e.g. Darwin) are also affected by the Go runtime sending SIGURG. This patch changes how we match the signal by just looking for the "URG" name, which should handle any platform that has this signal defined in the SignalMap. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Migrating this package from commit: moby/moby@c81abef Strategy taken: # install filter-repo (https://github.com/newren/git-filter-repo/blob/main/INSTALL.md) brew install git-filter-repo cd ~/projects # create a temporary clone of docker git clone https://github.com/docker/docker.git moby_signal cd moby_signal # remove all code, except for pkg/signal, and rename to /signal git filter-repo --path pkg/signal --path-rename pkg/signal:signal # go to the target github.com/moby/sys repository cd ~/projects/moby-sys # create a branch to work with git checkout -b integrate_moby_signal # add the temporary repository as an upstream and make sure it's up-to-date git remote add moby_signal ~/projects/moby_signal git fetch moby_signal # merge the upstream code git merge --allow-unrelated-histories --signoff -S moby_signal/master Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

signal_linux_test.go:28:16: Error return value of `syscall.Kill` is not checked (errcheck) syscall.Kill(syscall.Getpid(), signal) ^ signal_linux_test.go:59:14: Error return value of `syscall.Kill` is not checked (errcheck) syscall.Kill(syscall.Getpid(), signal) ^ trap.go:59:16: Error return value of `DumpStacks` is not checked (errcheck) DumpStacks("") ^ trap.go:94:15: Error return value of `f.Sync` is not checked (errcheck) defer f.Sync() ^ testfiles/main.go:42:14: Error return value of `p.Signal` is not checked (errcheck) p.Signal(sigmap[s]) ^ testfiles/main.go:45:13: Error return value of `p.Signal` is not checked (errcheck) p.Signal(sigmap[s]) ^ testfiles/main.go:48:12: Error return value of `p.Signal` is not checked (errcheck) p.Signal(sigmap[s]) ^ Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2021-07-09T21:33:29Z

signal/trap.go

+						}
+					} else {
+						// 3 SIGTERM/INT signals received; force exit without cleanup
+						logger.Info("Forcing docker daemon shutdown without cleanup; 3 interrupts received")


This should probably be made more generic

Suggested change

logger.Info("Forcing docker daemon shutdown without cleanup; 3 interrupts received")

logger.Info("Forcing shutdown; 3 interrupts received")

thaJeztah · 2021-07-09T21:33:54Z

signal/trap.go

+					}
+				case syscall.SIGQUIT:
+					_, _ = DumpStacks("")
+					logger.Info("Forcing docker daemon shutdown without cleanup on SIGQUIT")


Same for this:

Suggested change

logger.Info("Forcing docker daemon shutdown without cleanup on SIGQUIT")

logger.Info("Forcing shutdown on SIGQUIT")

Or perhaps this whole file should be kept in the moby repository (as it may be quite specific to moby)

thaJeztah · 2021-07-09T21:38:34Z

Some old commits were missing a proper DCO, so I manually marked it to "pass"

Commit sha: 6057481, Author: Guillaume J. Charmes, Committer: Guillaume J. Charmes; The sign-off is missing.
Commit sha: 108a12b, Author: Guillaume J. Charmes, Committer: Guillaume J. Charmes; The sign-off is missing.
Commit sha: aa57f84, Author: Guillaume J. Charmes, Committer: Guillaume J. Charmes; The sign-off is missing.
Commit sha: 5fcf5fa, Author: Kato Kazuyoshi, Committer: Kato Kazuyoshi; The sign-off is missing.
Commit sha: 530bcc0, Author: Guillaume J. Charmes, Committer: Guillaume J. Charmes; The sign-off is missing.
Commit sha: a3e2247, Author: Tianon Gravi, Committer: Tianon Gravi; The sign-off is missing.
Commit sha: 2f0e890, Author: Solomon Hykes, Committer: Solomon Hykes; The sign-off is missing.
Commit sha: 57a9d66, Author: Phil Estes, Committer: Phil Estes; The sign-off is missing.
Commit sha: 860f076, Author: Amit Krishnan, Committer: Amit Krishnan; Expected "Amit Krishnan amit.krishnan@oracle.com", but got "Amit Krishnan krish.amit@gmail.com".

thaJeztah · 2021-07-09T21:39:37Z

I guess this is one to decide (can be done later if we want)

Or perhaps this whole file should be kept in the moby repository (as it may be quite specific to moby)

thaJeztah · 2021-07-15T18:53:42Z

I'm updating the moby package in moby/moby#42641 to get rid of the things we don't need

thaJeztah · 2021-07-16T08:16:26Z

closing in favour of (WIP) #70

creack and others added 30 commits March 10, 2014 17:36

Move signal to pkg

6057481

Docker-DCO-1.1-Signed-off-by: Guillaume J. Charmes <guillaume@charmes.net> (github: creack)

Create portable signalMap

108a12b

Docker-DCO-1.1-Signed-off-by: Guillaume J. Charmes <guillaume@charmes.net> (github: creack)

Make docker use the signal pkg with strings

aa57f84

Docker-DCO-1.1-Signed-off-by: Guillaume J. Charmes <guillaume@charmes.net> (github: creack)

Like signal_linux.go, we don't have import os and os/signal

5fcf5fa

Docker-DCO-1.1-Signed-off-by: Kato Kazuyoshi <kato.kazuyoshi@gmail.com> (github: kzys)

Update email + add self to pkg/signal

530bcc0

Docker-DCO-1.1-Signed-off-by: Guillaume J. Charmes <guillaume@charmes.net> (github: creack)

Fix various MAINTAINERS format inconsistencies

a3e2247

Docker-DCO-1.1-Signed-off-by: Andrew Page <admwiggin@gmail.com> (github: tianon)

Merge pull request #5500 from tianon/fix-maintainers-format

f93da8c

Fix various MAINTAINERS format inconsistencies

Merge branch 'master' into bump_v0.11.0

699bc87

Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)

Merge pull request #5640 from crosbymichael/bump_v0.11.0

144c482

Bump version to v0.11.0

Merge pull request #5660 from crosbymichael/merge_release_v0.11.0

97a15c1

Merge release to master v0.11.0

Merge branch 'master' into bump_v0.11.1

555b503

Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)

Merge pull request #5668 from crosbymichael/bump_v0.11.1

5fad90f

Bump to version v0.11.1

Merge branch 'release' into merge_release_v0.11.1

1dc5839

Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)

Merge pull request #5671 from crosbymichael/merge_release_v0.11.1

27ce6a3

Merge release v0.11.1

Merge pull request #6230 from tiborvass/bump_v0.12.0

0393c32

Bump version to v0.12.0

Merge pull request #6238 from tiborvass/merge_release_v0.12.0

399a4a6

Merge release v0.12.0

Merge branch 'master' into bump_v1.0.0

6c00d86

Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)

Merge pull request #6303 from crosbymichael/bump_v1.0.0

a35ebee

Bump version to v1.0.0

Merge pull request #6305 from crosbymichael/merge_release_v1.0.0

214f4c8

Merge release v1.0.0

Guillaume is busy full-time on his new business, and no longer available

2f0e890

as a maintainer. Best of luck on your e-commerce business Guillaume, and thanks for all the great contributions! Docker-DCO-1.1-Signed-off-by: Solomon Hykes <solomon@docker.com> (github: shykes)

Merge pull request #6448 from shykes/pr_out_guillaume_is_busy_full_ti…

5e16354

…me_on_his_new_business_and_no_longer_available_as_a_maintainer

Merge pull request #6559 from vieux/bump_v1.0.1

35e3739

Bump v1.0.1

Merge pull request #6563 from vieux/merge_release_v1.0.1

dcf7652

Merge release v1.0.1

Merge pull request #6818 from vieux/bump_v1.1.0

c502063

Bump v1.1.0

Merge pull request #6838 from vieux/merge_release_v1.1.0

7ddadb8

Merge release v1.1.0

Move signal handling code to pkg/signal.Trap

28ce221

Signed-off-by: Solomon Hykes <solomon@docker.com>

Merge pull request #7449 from shykes/cleanup-shutdown

e39ec53

Cleanup: refactor shutdown and signal handling facility

Exit after receiving SIGTERM

8c8d0fd

Signed-off-by: Michael Crosby <michael@docker.com>

Merge pull request #7568 from crosbymichael/replace-7240

0cde53c

Exit after receiving SIGTERM

Merge pull request #7644 from vieux/bump_v1.2.0

f0d6747

Bump v1.2.0

thaJeztah and others added 11 commits November 27, 2019 15:41

pkg/signal: normalize comment formatting

5662150

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

bump gotest.tools v3.0.1 for compatibility with Go 1.14

749f8fc

full diff: gotestyourself/gotest.tools@v2.3.0...v3.0.1 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

TestCatchAll, TestStopCatch: remove unneeded goroutine

9aca473

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Merge pull request #40496 from thaJeztah/locally_scope_variable

102c202

TestCatchAll, TestStopCatch: remove unneeded goroutine

signal: update import-paths

4949301

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

signal: add go.mod

a92799b

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Makefile: enable tests for signals

25f5f69

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah marked this pull request as draft July 9, 2021 21:16

thaJeztah added 3 commits July 9, 2021 23:20

signal: remove logrus dependency

dde2a2a

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

signal: remove gotest.tools dependency

f4c3850

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

signal: remove github.com/pkg/errors dependency

609d332

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the integrate_moby_signal branch from 6550a6e to 609d332 Compare July 9, 2021 21:21

thaJeztah commented Jul 9, 2021

View reviewed changes

thaJeztah marked this pull request as ready for review July 9, 2021 21:38

This was referenced Jul 9, 2021

Request - containerd to support SIGRTMIN+3 stopsignal containerd/containerd#5402

Closed

replace pkg/signal with moby/sys/signal v0.5.0 moby/moby#42616

Merged

kzys mentioned this pull request Jul 12, 2021

Support SIGRTMIN+n signals containerd/containerd#5693

Merged

thaJeztah mentioned this pull request Jul 15, 2021

pkg/signal: make package self-contained (in preparation of moving to moby/sys) moby/moby#42641

Merged

thaJeztah mentioned this pull request Jul 15, 2021

migrate github.com/docker/docker/pkg/signal (take 2) #70

Merged

thaJeztah closed this Jul 16, 2021

thaJeztah deleted the integrate_moby_signal branch July 17, 2023 09:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate github.com/docker/docker/pkg/signal #69

migrate github.com/docker/docker/pkg/signal #69

thaJeztah commented Jul 9, 2021

thaJeztah Jul 9, 2021

AkihiroSuda Jul 15, 2021

thaJeztah Jul 9, 2021

thaJeztah Jul 9, 2021

thaJeztah commented Jul 9, 2021

thaJeztah commented Jul 9, 2021

thaJeztah commented Jul 15, 2021

thaJeztah commented Jul 16, 2021

	logger.Info("Forcing docker daemon shutdown without cleanup; 3 interrupts received")
	logger.Info("Forcing shutdown; 3 interrupts received")

	logger.Info("Forcing docker daemon shutdown without cleanup on SIGQUIT")
	logger.Info("Forcing shutdown on SIGQUIT")

migrate github.com/docker/docker/pkg/signal #69

migrate github.com/docker/docker/pkg/signal #69

Conversation

thaJeztah commented Jul 9, 2021

thaJeztah Jul 9, 2021

Choose a reason for hiding this comment

AkihiroSuda Jul 15, 2021

Choose a reason for hiding this comment

thaJeztah Jul 9, 2021

Choose a reason for hiding this comment

thaJeztah Jul 9, 2021

Choose a reason for hiding this comment

thaJeztah commented Jul 9, 2021

thaJeztah commented Jul 9, 2021

thaJeztah commented Jul 15, 2021

thaJeztah commented Jul 16, 2021