-
Notifications
You must be signed in to change notification settings - Fork 202
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.
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
Switch default logdriver and eventslogger to journald, if root #621
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
// +build systemd | ||
// +build systemd,cgo | ||
|
||
package config | ||
|
||
|
@@ -9,11 +9,19 @@ import ( | |
|
||
"github.com/containers/common/pkg/cgroupv2" | ||
"github.com/containers/storage/pkg/unshare" | ||
"github.com/coreos/go-systemd/v22/sdjournal" | ||
) | ||
|
||
var ( | ||
systemdOnce sync.Once | ||
usesSystemd bool | ||
systemdOnce sync.Once | ||
usesSystemd bool | ||
journaldOnce sync.Once | ||
usesJournald bool | ||
) | ||
|
||
const ( | ||
// DefaultLogDriver is the default type of log files | ||
DefaultLogDriver = "journald" | ||
) | ||
|
||
func defaultCgroupManager() string { | ||
|
@@ -29,20 +37,17 @@ func defaultCgroupManager() string { | |
} | ||
|
||
func defaultEventsLogger() string { | ||
if useSystemd() { | ||
if useJournald() { | ||
return "journald" | ||
} | ||
return "file" | ||
} | ||
|
||
func defaultLogDriver() string { | ||
// If we decide to change the default for logdriver, it should be done here. | ||
if useSystemd() { | ||
return DefaultLogDriver | ||
if useJournald() { | ||
return "journald" | ||
} | ||
|
||
return DefaultLogDriver | ||
|
||
return "k8s-file" | ||
} | ||
|
||
func useSystemd() bool { | ||
|
@@ -56,3 +61,19 @@ func useSystemd() bool { | |
}) | ||
return usesSystemd | ||
} | ||
|
||
func useJournald() bool { | ||
journaldOnce.Do(func() { | ||
if !useSystemd() { | ||
return | ||
} | ||
journal, err := sdjournal.NewJournal() | ||
if err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @giuseppe @vrothberg @nalind Do you know if this is enough to check if we can use the journal? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like the implementation will either manage to open the journal for reading and return a handle for it, or return an error. The APIs for sending data to the journal don't go through the handle, so I think we have to take it on faith that messages we send to the journal will be retrievable through the handle. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM |
||
return | ||
} | ||
journal.Close() | ||
usesJournald = true | ||
return | ||
}) | ||
return usesJournald | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you still set DefaultLogDriver to k8s-file, return it here, and then just system it to systems in the appropriate places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No because we want DefaultLogDriver to be indicated as journald now.