Skip to content

Exit when stopped leading #8326

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

Closed
wants to merge 2 commits into from
Closed

Exit when stopped leading #8326

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
639e3ed
Exit when stopped leading
mu-soliman Jul 16, 2025
e965990
Use exit instead.
mu-soliman Jul 28, 2025
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
3 changes: 2 additions & 1 deletion cluster-autoscaler/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,8 @@ func main() {
run(healthCheck, debuggingSnapshotter)
},
OnStoppedLeading: func() {
klog.Fatalf("lost master")
klog.Error("lost master. Shutting down.")
klog.FlushAndExit(klog.ExitFlushTimeout, 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look to me like this is meant to be used in addition to klog.Fatalf:

// FlushAndExit flushes log data for a certain amount of time and then calls
// os.Exit. Combined with some logging call it provides a replacement for
// traditional calls like Fatal or Exit.

In fact this is unreachable code as klog.Fatalf has the outcome of calling OsExit(255).

// Fatalf logs to the FATAL, ERROR, WARNING, and INFO logs,
// including a stack trace of all running goroutines, then calls OsExit(255).
// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing.
func Fatalf(format string, args ...interface{}) {
	logging.printf(severity.FatalLog, logging.logger, logging.filter, format, args...)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not the current behaviour of klog.fatalf(), even if the docs say so. If you follow the code, logging.printf() has the following implementation:
func (l *loggingT) printf(s severity.Severity, logger *logWriter, filter LogFilter, format string, args ...interface{}) { l.printfDepth(s, logger, filter, 1, format, args...) }

and printfDepth has the following implementation:

`func (l *loggingT) printfDepth(s severity.Severity, logger *logWriter, filter LogFilter, depth int, format string, args ...interface{}) {
if false {
_ = fmt.Sprintf(format, args...) // cause vet to treat this function like fmt.Printf
}

buf, file, line := l.header(s, depth)
// If a logger is set and doesn't support writing a formatted buffer,
// we clear the generated header as we rely on the backing
// logger implementation to print headers.
if logger != nil && logger.writeKlogBuffer == nil {
	buffer.PutBuffer(buf)
	buf = buffer.GetBuffer()
}
if filter != nil {
	format, args = filter.FilterF(format, args)
}
fmt.Fprintf(buf, format, args...)
if buf.Bytes()[buf.Len()-1] != '\n' {
	buf.WriteByte('\n')
}
l.output(s, logger, buf, depth, file, line, false)

}`

This behavior implements the sig-instrumentation guidelines of not using fatal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to using klog.Error() for better clarity and conformity with the guidelines.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know if it matters, but klog.Fatal does exit, see: https://github.com/kubernetes/klog/blob/e7125f792ea66a85818cfb45261c9e1acc585344/klog.go#L936-L964

Here's a test:

package main

import (
	"flag"

	"k8s.io/klog/v2"
)

func main() {
	klog.InitFlags(nil)
	flag.Parse()

	defer klog.Flush()

	klog.Info("About to encounter a fatal error...")
	klog.Fatalf("This is a fatal error - the application will exit here!")

	// This line will never be reached
	klog.Info("This message will never be printed")
}

$ go run main.go
I0728 16:20:33.182323   99101 main.go:15] About to encounter a fatal error...
F0728 16:20:33.182456   99101 main.go:16] This is a fatal error - the application will exit here!
exit status 255

Copy link
Contributor

Choose a reason for hiding this comment

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

@mu-soliman As explained above, klog.Fatalf does exit: https://github.com/kubernetes/klog/blob/main/klog.go#L936

},
},
})
Expand Down
Loading