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.

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

Merged
merged 1 commit into from
Aug 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 10 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ GO_BUILD=$(GO) build
ifeq ($(shell go help mod >/dev/null 2>&1 && echo true), true)
GO_BUILD=GO111MODULE=on $(GO) build -mod=vendor
endif
BUILDTAGS := containers_image_openpgp,systemd,no_libsubid
BUILDTAGS := containers_image_openpgp,systemd,no_libsubid,exclude_graphdriver_devicemapper
DESTDIR ?=
PREFIX := /usr/local
CONFIGDIR := ${PREFIX}/share/containers
Expand Down Expand Up @@ -39,8 +39,17 @@ define go-build
GOOS=$(1) GOARCH=$(2) $(GO) build -tags "$(3)" ./...
endef

define go-build-c
CGO_ENABLED=1 \
GOOS=$(1) GOARCH=$(2) $(GO) build -tags "$(3)" ./...
endef

.PHONY:
build-cross:
$(call go-build-c,linux) # attempt to build without tags
$(call go-build-c,linux,,${BUILDTAGS})
$(call go-build-c,linux,386,${BUILDTAGS})
$(call go-build,linux) # attempt to build without tags
$(call go-build,linux,386,${BUILDTAGS})
$(call go-build,linux,arm,${BUILDTAGS})
$(call go-build,linux,arm64,${BUILDTAGS})
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/containers/image/v5 v5.15.0
github.com/containers/ocicrypt v1.1.2
github.com/containers/storage v1.33.2
github.com/coreos/go-systemd/v22 v22.3.2
github.com/disiqueira/gotree/v3 v3.0.2
github.com/docker/distribution v2.7.1+incompatible
github.com/docker/docker v20.10.8+incompatible
Expand Down
2 changes: 0 additions & 2 deletions pkg/config/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,6 @@ const (
DefaultApparmorProfile = apparmor.Profile
// SystemdCgroupsManager represents systemd native cgroup manager
SystemdCgroupsManager = "systemd"
// DefaultLogDriver is the default type of log files
DefaultLogDriver = "k8s-file"
// DefaultLogSizeMax is the default value for the maximum log size
// allowed for a container. Negative values mean that no limit is imposed.
DefaultLogSizeMax = -1
Expand Down
7 changes: 6 additions & 1 deletion pkg/config/nosystemd.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
// +build !systemd
// +build !systemd !cgo

package config

const (
// DefaultLogDriver is the default type of log files
DefaultLogDriver = "k8s-file"
)

func defaultCgroupManager() string {
return CgroupfsCgroupsManager
}
Expand Down
41 changes: 31 additions & 10 deletions pkg/config/systemd.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// +build systemd
// +build systemd,cgo

package config

Expand All @@ -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 {
Expand All @@ -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"
Copy link
Member

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?

Copy link
Member Author

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.

}

func useSystemd() bool {
Expand All @@ -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 {
Copy link
Member Author

Choose a reason for hiding this comment

The 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?

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Loading