Fix logging, collect status of forked processes #1790
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
klizhentas
force-pushed
the
sasha/patch-fixes
branch
from
5b148bd to
b20e603
Compare
r0mant
approved these changes
Mar 19, 2018
russjones
approved these changes
Mar 19, 2018
lib/service/service.go
Outdated
| @@ -728,6 +733,8 @@ func (process *TeleportProcess) initAuthService() error { | |||
| }) | |||
|
|
|||
| process.RegisterFunc("auth.tls", func() error { | |||
| utils.Consolef(cfg.Console, "[AUTH] Auth service is starting on %v.", cfg.Auth.SSHAddr.Addr) | |||
There was a problem hiding this comment.
These log lines will be misaligned because they don't use the new component logger we have.
Should be easy to fix because utils.Consolf just writes to a writer and then to the logs: https://github.com/gravitational/teleport/blob/v2.5.2/lib/utils/cli.go#L140-L148
| var wait syscall.WaitStatus | ||
| rpid, err := syscall.Wait4(pid, &wait, syscall.WNOHANG, nil) | ||
| if err != nil { | ||
| log.Errorf("Wait call failed: %v.", err) |
There was a problem hiding this comment.
continue after this line.
lib/service/signals.go
Outdated
| // have tried to collect the status of this process. | ||
| // Instead this logic tries to collect statuses of the processes | ||
| // forked during restart procedure. | ||
| func (process *TeleportProcess) collectStatuses() { |
There was a problem hiding this comment.
Comment should start with collectStatuses.
fixes #1785, fixes #1776 This commit fixes several issues with output: First teleport start now prints output matching quickstart guide and sets default console logging to ERROR. SIGCHLD handler now only collects processes PID forked during live restart to avoid confusing other wait calls that have no process status to collect any more.
klizhentas
force-pushed
the
sasha/patch-fixes
branch
from
b20e603 to
7d05c05
Compare
russjones
approved these changes
Mar 19, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #1785, fixes #1776
This commit fixes several issues with output:
First teleport start now prints output
matching quickstart guide and sets default
console logging to ERROR.
SIGCHLD handler now only collects
processes PID forked during live restart
to avoid confusing other wait calls that
have no process status to collect any more.