Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

main: stacktrace/coredump on error #61

Merged
merged 1 commit into from
Mar 28, 2018

Conversation

jodh-intel
Copy link
Contributor

If the proxy fails due to an internal error or if it receives
a fatal signal, write a stack trace to the log and exit.

If it was started with --debug also dump core.

Fixes #59.

Signed-off-by: James O. D. Hunt [email protected]

@jodh-intel
Copy link
Contributor Author

Replaces #60.

See also: kata-containers/shim#59.

@grahamwhaley
Copy link
Contributor

static check fail on Travis

  vet
fatal.go:64:11:warning: should omit 2nd value from range; this loop is equivalent to `for sig := range ...` (golint)
proxy.go:250::warning: cyclomatic complexity 16 of function realMain() is high (> 15) (gocyclo)
fatal.go:1::warning: file is not gofmted with -s (gofmt)

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shame the go generics could not quite do this for us.
lgtm

@jodh-intel
Copy link
Contributor Author

Branch updated.

If the proxy fails due to an internal error or if it receives
a fatal signal, write a stack trace to the log and exit.

If it was started with `--debug` also dump core.

Note: `handleVersion()` introduced to keep `gocyclo` happy.

Fixes kata-containers#59.

Signed-off-by: James O. D. Hunt <[email protected]>
@jodh-intel
Copy link
Contributor Author

Travis is now happy.

@jodh-intel
Copy link
Contributor Author

ping @kata-containers/proxy.

@devimc
Copy link

devimc commented Mar 22, 2018

LGTM

@jodh-intel
Copy link
Contributor Author

Wow, I'd forgotten about this one. CAn we get one more ack before we consider force-merging? :)

@jodh-intel
Copy link
Contributor Author

2 weeks is a long time on this project ;)

@jodh-intel
Copy link
Contributor Author

Please can we get some eyeballs on this? This PR has been open for 18 days now.

It's non-contentious and is very similar in content to the following already-merged PRs:

@jshachm
Copy link
Member

jshachm commented Mar 28, 2018

LGTM ‘casue it's a similar PR. @WeiZhang555 Can you help @jodh-intel get review +1

@caoruidong
Copy link
Member

I think you can reping members...

@jodh-intel
Copy link
Contributor Author

ping @sboeuf, @bergwolf.

@WeiZhang555
Copy link
Member

WeiZhang555 commented Mar 28, 2018

LGTM
Sorry for the late

Approved with PullApprove

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants