-
Notifications
You must be signed in to change notification settings - Fork 663
Journald support #39
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
Journald support #39
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 +1,2 @@ | ||
/bin/node-problem-detector | ||
/Dockerfile |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,73 @@ | ||
.PHONY: all build-container build-tar build push-container push-tar push clean vet fmt version | ||
# Copyright 2017 The Kubernetes Authors. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
# Build the node-problem-detector image. | ||
|
||
.PHONY: all build-container build-tar build push-container push-tar push clean vet fmt version Dockerfile | ||
|
||
all: build | ||
|
||
VERSION := $(shell git describe --tags --dirty) | ||
# VERSION is the version of the binary. | ||
VERSION:=$(shell git describe --tags --dirty) | ||
|
||
TAG ?= $(VERSION) | ||
# TAG is the tag of the container image, default to binary version. | ||
TAG?=$(VERSION) | ||
|
||
UPLOAD_PATH ?= gs://kubernetes-release | ||
# PROJ is the image project. | ||
PROJ?=gcr.io/google_containers | ||
|
||
# UPLOAD_PATH is the cloud storage path to upload release tar. | ||
UPLOAD_PATH?=gs://kubernetes-release | ||
# Trim the trailing '/' in the path | ||
UPLOAD_PATH := $(shell echo $(UPLOAD_PATH) | sed '$$s/\/*$$//') | ||
UPLOAD_PATH:=$(shell echo $(UPLOAD_PATH) | sed '$$s/\/*$$//') | ||
|
||
# PKG is the package name of node problem detector repo. | ||
PKG:=k8s.io/node-problem-detector | ||
|
||
# PKG_SOURCES are all the go source code. | ||
PKG_SOURCES:=$(shell find pkg cmd -name '*.go') | ||
|
||
PROJ ?= google_containers | ||
# TARBALL is the name of release tar. Include binary version by default. | ||
TARBALL:=node-problem-detector-$(VERSION).tar.gz | ||
|
||
PKG := k8s.io/node-problem-detector | ||
# IMAGE is the image name of the node problem detector container image. | ||
IMAGE:=$(PROJ)/node-problem-detector:$(TAG) | ||
|
||
PKG_SOURCES := $(shell find pkg cmd -name '*.go') | ||
# ENABLE_JOURNALD enables build journald support or not. Building journald support needs libsystemd-dev | ||
# or libsystemd-journal-dev. | ||
# TODO(random-liu): Build NPD inside container. | ||
ENABLE_JOURNALD?=1 | ||
|
||
TARBALL := node-problem-detector-$(VERSION).tar.gz | ||
# TODO(random-liu): Support different architectures. | ||
BASEIMAGE:=alpine:3.4 | ||
|
||
IMAGE := gcr.io/$(PROJ)/node-problem-detector:$(TAG) | ||
# Disable cgo by default to make the binary statically linked. | ||
CGO_ENABLED:=0 | ||
|
||
# NOTE that enable journald will increase the image size. | ||
ifeq ($(ENABLE_JOURNALD), 1) | ||
# Enable journald build tag. | ||
BUILD_TAGS:=-tags journald | ||
# Use fedora because it has newer systemd version (229) and support +LZ4. +LZ4 is needed | ||
# on some os distros such as GCI. | ||
BASEIMAGE:=fedora | ||
# Enable cgo because sdjournal needs cgo to compile. The binary will be dynamically | ||
# linked if CGO_ENABLED is enabled. This is fine because fedora already has necessary | ||
# dynamic library. We can not use `-extldflags "-static"` here, because go-systemd uses | ||
# dlopen, and dlopen will not work properly in a statically linked application. | ||
CGO_ENABLED:=1 | ||
endif | ||
|
||
vet: | ||
go list ./... | grep -v "./vendor/*" | xargs go vet | ||
|
@@ -30,14 +79,17 @@ version: | |
@echo $(VERSION) | ||
|
||
./bin/node-problem-detector: $(PKG_SOURCES) | ||
GOOS=linux go build -o bin/node-problem-detector \ | ||
-ldflags '-w -extldflags "-static" -X $(PKG)/pkg/version.version=$(VERSION)' \ | ||
cmd/node_problem_detector.go | ||
CGO_ENABLED=$(CGO_ENABLED) GOOS=linux go build -o bin/node-problem-detector \ | ||
-ldflags '-w -X $(PKG)/pkg/version.version=$(VERSION)' \ | ||
$(BUILD_TAGS) cmd/node_problem_detector.go | ||
|
||
Dockerfile: Dockerfile.in | ||
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. Should there be a 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. Will do. 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. If you don't add Dockerfile to PHONY targets, users will be confused. Consider this scenario: make
# The Dockerfile file didn't exist and make created it.
# @BASEIMAGE@ is Fedora and systemd-journal is enabled.
make ENABLE_JOURNALD=0
# The Dockerfile file already exists and make skipped the Dockerfile target.
# However the user believes that @BASEIMAGE@ is set to "alpine:3.4" and
# systemd-journal is disabled. 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. @jfilak I see. You are right. Thanks for explaining. :) 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. Done 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. According to #39(discussion), we should also make |
||
sed -e 's|@BASEIMAGE@|$(BASEIMAGE)|g' $< >$@ | ||
|
||
test: vet fmt | ||
go test -timeout=1m -v -race ./pkg/... | ||
go test -timeout=1m -v -race ./pkg/... $(BUILD_TAGS) | ||
|
||
build-container: ./bin/node-problem-detector | ||
build-container: ./bin/node-problem-detector Dockerfile | ||
docker build -t $(IMAGE) . | ||
|
||
build-tar: ./bin/node-problem-detector | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ spec: | |
- name: node-problem-detector | ||
command: | ||
- /node-problem-detector | ||
- --logtostderr | ||
- --kernel-monitor=/config/kernel-monitor.json | ||
image: gcr.io/google_containers/node-problem-detector:v0.2 | ||
imagePullPolicy: Always | ||
|
@@ -24,7 +25,12 @@ spec: | |
fieldPath: spec.nodeName | ||
volumeMounts: | ||
- name: log | ||
mountPath: /log | ||
mountPath: /var/log | ||
readOnly: true | ||
# Make sure node problem detector is in the same timezone | ||
# with the host. | ||
- name: localtime | ||
mountPath: /etc/localtime | ||
readOnly: true | ||
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. FYI yesterday, Dan Walsh posted this on Atomic devel list (Atomic is basically Fedora): In short, if the However, I don't think you can do something to avoid this issue. 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. That's wired. When I initially decided to go with this approach, I remembered I've tried the symlink situation, it worked. I'll try again then. 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. I read your link and understand what you mean. Thanks for sharing! IIUC, it means that if This should not cause trouble to golang, but it's worth fixing to avoid confusion. 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. Done. |
||
- name: config | ||
mountPath: /config | ||
|
@@ -34,6 +40,9 @@ spec: | |
# Config `log` to your system log directory | ||
hostPath: | ||
path: /var/log/ | ||
- name: localtime | ||
hostPath: | ||
path: /etc/localtime | ||
- name: config | ||
configMap: | ||
name: node-problem-detector-config |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/* | ||
Copyright 2016 The Kubernetes Authors All rights reserved. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package kernelmonitor | ||
|
||
import ( | ||
watchertypes "k8s.io/node-problem-detector/pkg/kernelmonitor/logwatchers/types" | ||
kerntypes "k8s.io/node-problem-detector/pkg/kernelmonitor/types" | ||
"k8s.io/node-problem-detector/pkg/types" | ||
) | ||
|
||
// MonitorConfig is the configuration of kernel monitor. | ||
type MonitorConfig struct { | ||
// WatcherConfig is the configuration of kernel log watcher. | ||
watchertypes.WatcherConfig | ||
// BufferSize is the size (in lines) of the log buffer. | ||
BufferSize int `json:"bufferSize"` | ||
// Source is the source name of the kernel monitor | ||
Source string `json:"source"` | ||
// DefaultConditions are the default states of all the conditions kernel monitor should handle. | ||
DefaultConditions []types.Condition `json:"conditions"` | ||
// Rules are the rules kernel monitor will follow to parse the log file. | ||
Rules []kerntypes.Rule `json:"rules"` | ||
// StartPattern is the pattern of the start line | ||
StartPattern string `json:"startPattern, omitempty"` | ||
} | ||
|
||
// applyDefaultConfiguration applies default configurations. | ||
func applyDefaultConfiguration(cfg *MonitorConfig) { | ||
if cfg.BufferSize == 0 { | ||
cfg.BufferSize = 10 | ||
} | ||
if cfg.WatcherConfig.Lookback == "" { | ||
cfg.WatcherConfig.Lookback = "0" | ||
} | ||
} |
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.
Nit: spaces around the assignment operators missing to keep this in-sync with the other variables here.
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.
ACK. Will fix. Thanks.
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.
Done.