Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support to manage logs using lumber jack rolling logger #3566

Closed
wants to merge 1 commit into from
Closed

add support to manage logs using lumber jack rolling logger #3566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions images/build/go-runner/go-runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,19 @@ import (
"os/signal"
"strings"
"syscall"

"gopkg.in/natefinch/lumberjack.v2"
)

var (
logFilePath = flag.String("log-file", "", "If non-empty, save stdout to this file")
alsoToStdOut = flag.Bool("also-stdout", false, "useful with log-file, log to standard output as well as the log file")
redirectStderr = flag.Bool("redirect-stderr", true, "treat stderr same as stdout")
logFilePath = flag.String("log-file", "", "If non-empty, save stdout to this file")
alsoToStdOut = flag.Bool("also-stdout", false, "useful with log-file, log to standard output as well as the log file")
redirectStderr = flag.Bool("redirect-stderr", true, "treat stderr same as stdout")
logRotate = flag.Bool("log-rotate", false, "allow go-runner to handle log rotation")
logMaxSize = flag.Int("log-maxsize", 100, "The maximum size in megabytes of the log file before it gets rotated")
logMaxAge = flag.Int("log-maxage", 0, "The maximum number of days to retain old log files based on the timestamp encoded in their filename")
logMaxBackups = flag.Int("log-maxbackup", 1, "The maximum number of old log files to retain. Setting a value of 0 will mean there's no restriction on the number of files")
logBackupCompress = flag.Bool("log-compress", false, "If set, the rotated log files will be compressed using gzip")
)

func main() {
Expand All @@ -46,6 +53,7 @@ func configureAndRun() error {
var (
outputStream io.Writer = os.Stdout
errStream io.Writer = os.Stderr
logFile io.Writer
)

args := flag.Args()
Expand All @@ -54,9 +62,23 @@ func configureAndRun() error {
}

if logFilePath != nil && *logFilePath != "" {
logFile, err := os.OpenFile(*logFilePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644)
if err != nil {
return fmt.Errorf("failed to create log file %v: %w", *logFilePath, err)

if *logRotate {
logger := &lumberjack.Logger{
Filename: *logFilePath,
MaxSize: *logMaxSize, // megabytes
MaxBackups: *logMaxBackups, // log file retain count
MaxAge: *logMaxAge, // days
Compress: *logBackupCompress, // disabled by default
}
defer logger.Close()
logFile = logger
} else {
file, err := os.OpenFile(*logFilePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644)
if err != nil {
return fmt.Errorf("failed to create log file %v: %w", *logFilePath, err)
}
logFile = file
}
if *alsoToStdOut {
outputStream = io.MultiWriter(os.Stdout, logFile)
Expand Down
2 changes: 2 additions & 0 deletions images/build/go-runner/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
module k8s.io/release/images/build/go-runner

go 1.21

require gopkg.in/natefinch/lumberjack.v2 v2.2.1
Copy link
Member

Choose a reason for hiding this comment

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

We need to reach consensus if we want to introduce a dependency to go-runner. I'm not in favor of that.

/hold

Copy link
Member

Choose a reason for hiding this comment

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

cc @kubernetes/release-managers @kubernetes/sig-architecture

Copy link
Member

Choose a reason for hiding this comment

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

Since go-runner is used for various core Kubernetes components, maybe we should consider requiring a KEP for such a change.

Copy link
Member

Choose a reason for hiding this comment

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

I really do not want to see us growing dependencies outside the stdlib for this

Copy link
Contributor

Choose a reason for hiding this comment

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

@xmudrii is correct this is used for basically all the components so at a minimum we'd need to think about this beyond the scope of just this PR.

Overall -1, in agreement with @liggitt and @saschagrunert

Copy link
Member

Choose a reason for hiding this comment

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

+1 with sascha and liggitt comments

this might be achieve using a sidecar if need to do that

Copy link
Member

Choose a reason for hiding this comment

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

thanks for all the feedback folks!

Copy link

Choose a reason for hiding this comment

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

I really do not want to see us growing dependencies outside the stdlib for this

Looking at the Lumberjack go.mod file there are no third party dependencies. At this point its only using all stdlib code and has been pretty stable for last few years (used in APIserver audit logs as well).

2 changes: 2 additions & 0 deletions images/build/go-runner/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
gopkg.in/natefinch/lumberjack.v2 v2.2.1 h1:bBRl1b0OH9s/DuPhuXpNl+VtCaJXFZ5/uEFST95x9zc=
gopkg.in/natefinch/lumberjack.v2 v2.2.1/go.mod h1:YD8tP3GAjkrDg1eZH7EGmyESg/lsYskCTPBJVb9jqSc=
Loading