From 5b07afd32586f7ec36743e32715d1d55998ed423 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Tue, 31 May 2016 18:39:14 -0700 Subject: [PATCH] 1. Make source and conditions configurable. 2. Add multiple events and conditions support in problem interface. --- config/kernel-monitor.json | 13 +++- pkg/kernelmonitor/kernel_monitor.go | 87 +++++++++++++----------- pkg/kernelmonitor/kernel_monitor_test.go | 56 ++++++++++----- pkg/kernelmonitor/types/types.go | 4 ++ pkg/problemclient/problem_client.go | 1 + pkg/problemdetector/problem_detector.go | 8 ++- pkg/types/types.go | 12 ++-- 7 files changed, 114 insertions(+), 67 deletions(-) diff --git a/config/kernel-monitor.json b/config/kernel-monitor.json index 3d69ae20..c038c4b3 100644 --- a/config/kernel-monitor.json +++ b/config/kernel-monitor.json @@ -1,6 +1,14 @@ { "logPath": "/log/kern.log", "bufferSize": 10, + "source": "kernel-monitor", + "conditions": [ + { + "type": "KernelDeadlock", + "reason": "KernelHasNoDeadlock", + "message": "kernel has no deadlock" + } + ], "rules": [ { "type": "temporary", @@ -14,17 +22,20 @@ }, { "type": "permanent", + "condition": "KernelDeadlock", "reason": "AUFSUmountHung", "pattern": "task umount\\.aufs:\\w+ blocked for more than \\w+ seconds\\." }, { "type": "permanent", + "condition": "KernelDeadlock", "reason": "DockerHung", "pattern": "task docker:\\w+ blocked for more than \\w+ seconds\\." }, { "type": "permanent", - "reason": "KernelBug", + "condition": "KernelDeadlock", + "reason": "UnregisterNetDeviceIssue", "pattern": "unregister_netdevice: waiting for \\w+ to become free. Usage count = \\d+" } ] diff --git a/pkg/kernelmonitor/kernel_monitor.go b/pkg/kernelmonitor/kernel_monitor.go index c749ba7e..cd7293e4 100644 --- a/pkg/kernelmonitor/kernel_monitor.go +++ b/pkg/kernelmonitor/kernel_monitor.go @@ -30,20 +30,16 @@ import ( "github.com/golang/glog" ) -// May want to add more conditions if we need finer grained node conditions. -// TODO(random-liu): Make the kernel condition to be a predefined list, and make it configurable -// in rule. -const ( - KernelDeadlockCondition = "KernelDeadlock" - KernelMonitorSource = "kernel-monitor" -) - // MonitorConfig is the configuration of kernel monitor. type MonitorConfig struct { // WatcherConfig is the configuration of kernel log watcher. 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"` } @@ -58,21 +54,20 @@ type KernelMonitor interface { } type kernelMonitor struct { - watcher KernelLogWatcher - buffer LogBuffer - config MonitorConfig - condition types.Condition - uptime time.Time - logCh <-chan *kerntypes.KernelLog - output chan *types.Status - tomb *util.Tomb + watcher KernelLogWatcher + buffer LogBuffer + config MonitorConfig + conditions []types.Condition + uptime time.Time + logCh <-chan *kerntypes.KernelLog + output chan *types.Status + tomb *util.Tomb } // NewKernelMonitorOrDie create a new KernelMonitor, panic if error occurs. func NewKernelMonitorOrDie(configPath string) KernelMonitor { k := &kernelMonitor{ - condition: defaultCondition(), - tomb: util.NewTomb(), + tomb: util.NewTomb(), } f, err := ioutil.ReadFile(configPath) if err != nil { @@ -82,6 +77,8 @@ func NewKernelMonitorOrDie(configPath string) KernelMonitor { if err != nil { panic(err) } + // Initialize the default node conditions + k.conditions = initialConditions(k.config.DefaultConditions) err = validateRules(k.config.Rules) if err != nil { panic(err) @@ -120,7 +117,7 @@ func (k *kernelMonitor) Stop() { // monitorLoop is the main loop of kernel monitor. func (k *kernelMonitor) monitorLoop() { defer k.tomb.Done() - k.output <- defaultStatus() // Update the default status + k.output <- k.initialStatus() // Update the initial status for { select { case log := <-k.logCh: @@ -153,27 +150,34 @@ func (k *kernelMonitor) generateStatus(logs []*kerntypes.KernelLog, rule kerntyp messages = append(messages, log.Message) } message := concatLogs(messages) - var event *types.Event + var events []types.Event if rule.Type == kerntypes.Temp { // For temporary error only generate event - event = &types.Event{ + events = append(events, types.Event{ Severity: types.Warn, Timestamp: timestamp, Reason: rule.Reason, Message: message, - } + }) } else { // For permanent error changes the condition - k.condition.Type = KernelDeadlockCondition - k.condition.Status = true - k.condition.Transition = timestamp - k.condition.Reason = rule.Reason - k.condition.Message = message + for i := range k.conditions { + condition := &k.conditions[i] + if condition.Type == rule.Condition { + condition.Type = rule.Condition + condition.Status = true + condition.Transition = timestamp + condition.Reason = rule.Reason + condition.Message = message + break + } + } } return &types.Status{ - Source: KernelMonitorSource, - Event: event, - Condition: k.condition, + Source: k.config.Source, + // TODO(random-liu): Aggregate events and conditions and then do periodically report. + Events: events, + Conditions: k.conditions, } } @@ -182,22 +186,23 @@ func (k *kernelMonitor) generateTimestamp(timestamp int64) time.Time { return k.uptime.Add(time.Duration(timestamp * int64(time.Microsecond))) } -// defaultStatus returns the default status with default condition. -func defaultStatus() *types.Status { +// initialStatus returns the initial status with initial condition. +func (k *kernelMonitor) initialStatus() *types.Status { return &types.Status{ - Source: KernelMonitorSource, - Condition: defaultCondition(), + Source: k.config.Source, + Conditions: k.conditions, } } -func defaultCondition() types.Condition { - return types.Condition{ - Type: KernelDeadlockCondition, - Status: false, - Transition: time.Now(), - Reason: "KernelHasNoDeadlock", - Message: "kernel has no deadlock", +func initialConditions(defaults []types.Condition) []types.Condition { + conditions := make([]types.Condition, len(defaults)) + copy(conditions, defaults) + for i := range conditions { + // TODO(random-liu): Validate default conditions + conditions[i].Status = false + conditions[i].Transition = time.Now() } + return conditions } // validateRules verifies whether the regular expressions in the rules are valid. diff --git a/pkg/kernelmonitor/kernel_monitor_test.go b/pkg/kernelmonitor/kernel_monitor_test.go index 1f297e19..1dd51a2f 100644 --- a/pkg/kernelmonitor/kernel_monitor_test.go +++ b/pkg/kernelmonitor/kernel_monitor_test.go @@ -25,9 +25,26 @@ import ( "k8s.io/node-problem-detector/pkg/types" ) +const ( + testSource = "TestSource" + testConditionA = "TestConditionA" + testConditionB = "TestConditionB" +) + func TestGenerateStatus(t *testing.T) { uptime := time.Unix(1000, 0) - initCondition := defaultCondition() + initConditions := []types.Condition{ + { + Type: testConditionA, + Status: true, + Transition: time.Now(), + }, + { + Type: testConditionB, + Status: false, + Transition: time.Now(), + }, + } logs := []*kerntypes.KernelLog{ { Timestamp: 100000, @@ -45,17 +62,21 @@ func TestGenerateStatus(t *testing.T) { // Do not need Pattern because we don't do pattern match in this test { rule: kerntypes.Rule{ - Type: kerntypes.Perm, - Reason: "test reason", + Type: kerntypes.Perm, + Condition: testConditionA, + Reason: "test reason", }, expected: types.Status{ - Source: KernelMonitorSource, - Condition: types.Condition{ - Type: KernelDeadlockCondition, - Status: true, - Transition: time.Unix(1000, 100000*1000), - Reason: "test reason", - Message: "test message 1\ntest message 2", + Source: testSource, + Conditions: []types.Condition{ + { + Type: testConditionA, + Status: true, + Transition: time.Unix(1000, 100000*1000), + Reason: "test reason", + Message: "test message 1\ntest message 2", + }, + initConditions[1], }, }, }, @@ -65,20 +86,23 @@ func TestGenerateStatus(t *testing.T) { Reason: "test reason", }, expected: types.Status{ - Source: KernelMonitorSource, - Event: &types.Event{ + Source: testSource, + Events: []types.Event{{ Severity: types.Warn, Timestamp: time.Unix(1000, 100000*1000), Reason: "test reason", Message: "test message 1\ntest message 2", - }, - Condition: initCondition, + }}, + Conditions: initConditions, }, }, } { k := &kernelMonitor{ - condition: initCondition, - uptime: uptime, + config: MonitorConfig{ + Source: testSource, + }, + conditions: initConditions, + uptime: uptime, } got := k.generateStatus(logs, test.rule) if !reflect.DeepEqual(&test.expected, got) { diff --git a/pkg/kernelmonitor/types/types.go b/pkg/kernelmonitor/types/types.go index d38438ba..5545ffaf 100644 --- a/pkg/kernelmonitor/types/types.go +++ b/pkg/kernelmonitor/types/types.go @@ -37,6 +37,10 @@ const ( type Rule struct { // Type is the type of matched kernel problem. Type Type `json:"type"` + // Condition is the type of the condition the kernel problem triggered. Notice that + // the Condition field should be set only when the problem is permanent, or else the + // field will be ignored. + Condition string `json:"condition"` // Reason is the short reason of the kernel problem. Reason string `json:"reason"` // Pattern is the regular expression to match the kernel problem in kernel log. diff --git a/pkg/problemclient/problem_client.go b/pkg/problemclient/problem_client.go index 943b1baf..7c97ba32 100644 --- a/pkg/problemclient/problem_client.go +++ b/pkg/problemclient/problem_client.go @@ -123,6 +123,7 @@ func getEventRecorder(c *client.Client, nodeName, source string) record.EventRec } func getNodeRef(nodeName string) *api.ObjectReference { + // TODO(random-liu): Get node to initalize the node reference return &api.ObjectReference{ Kind: "Node", Name: nodeName, diff --git a/pkg/problemdetector/problem_detector.go b/pkg/problemdetector/problem_detector.go index 961efed8..04ff6e60 100644 --- a/pkg/problemdetector/problem_detector.go +++ b/pkg/problemdetector/problem_detector.go @@ -65,10 +65,12 @@ func (p *problemDetector) Run() error { glog.Errorf("Monitor stopped unexpectedly") break } - if status.Event != nil { - p.client.Eventf(util.ConvertToAPIEventType(status.Event.Severity), status.Source, status.Event.Reason, status.Event.Message) + for _, event := range status.Events { + p.client.Eventf(util.ConvertToAPIEventType(event.Severity), status.Source, event.Reason, event.Message) + } + for _, condition := range status.Conditions { + p.conditionManager.UpdateCondition(condition) } - p.conditionManager.UpdateCondition(status.Condition) } } } diff --git a/pkg/types/types.go b/pkg/types/types.go index c968f07d..d1e64ab3 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -69,10 +69,10 @@ type Event struct { type Status struct { // Source is the name of the problem daemon. Source string `json:"source"` - // Event is the temporary node problem event. If the status is only a condition update, - // this field could be nil. - Event *Event `json:"event"` - // Condition is the permanent node condition. The problem daemon should always report the - // newest node condition in this field. - Condition Condition `json:"condition"` + // Events are temporary node problem events. If the status is only a condition update, + // this field could be nil. Notice that the events should be sorted from oldest to newest. + Events []Event `json:"events"` + // Conditions are the permanent node conditions. The problem daemon should always report the + // newest node conditions in this field. + Conditions []Condition `json:"conditions"` }