Merge pull request #151 from Random-Liu/improve-cpm

Improve cpm

pkg/condition/manager_test.go

@@ -41,7 +41,7 @@ func newTestManager() (*conditionManager, *problemclient.FakeProblemClient, *clo
 func newTestCondition(condition string) types.Condition {
 	return types.Condition{
 		Type:       condition,
-		Status:     true,
+		Status:     types.True,
 		Transition: time.Now(),
 		Reason:     "TestReason",
 		Message:    "test message",

pkg/custompluginmonitor/custom_plugin_monitor.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/node-problem-detector/pkg/custompluginmonitor/plugin"
 	cpmtypes "k8s.io/node-problem-detector/pkg/custompluginmonitor/types"
 	"k8s.io/node-problem-detector/pkg/types"
+	"k8s.io/node-problem-detector/pkg/util"
 	"k8s.io/node-problem-detector/pkg/util/tomb"
 )
 
@@ -124,10 +125,16 @@ func (c *customPluginMonitor) generateStatus(result cpmtypes.Result) *types.Stat
 		for i := range c.conditions {
 			condition := &c.conditions[i]
 			if condition.Type == result.Rule.Condition {
-				status := result.ExitStatus >= cpmtypes.NonOK
+				status := toConditionStatus(result.ExitStatus)
 				if condition.Status != status || condition.Reason != result.Rule.Reason {
 					condition.Transition = timestamp
 					condition.Message = result.Message
+					events = append(events, util.GenerateConditionChangeEvent(
+						condition.Type,
+						status,
+						result.Rule.Reason,
+						timestamp,
+					))
 				}
 				condition.Status = status
 				condition.Reason = result.Rule.Reason
@@ -143,6 +150,17 @@ func (c *customPluginMonitor) generateStatus(result cpmtypes.Result) *types.Stat
 	}
 }
 
+func toConditionStatus(s cpmtypes.Status) types.ConditionStatus {
+	switch s {
+	case cpmtypes.OK:
+		return types.False
+	case cpmtypes.NonOK:
+		return types.True
+	default:
+		return types.Unknown
+	}
+}
+
 // initializeStatus initializes the internal condition and also reports it to the node problem detector.
 func (c *customPluginMonitor) initializeStatus() {
 	// Initialize the default node conditions
@@ -159,8 +177,7 @@ 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].Status = types.False
 		conditions[i].Transition = time.Now()
 	}
 	return conditions

pkg/systemlogmonitor/log_monitor.go

@@ -21,13 +21,14 @@ import (
 	"io/ioutil"
 	"time"
 
+	"github.com/golang/glog"
+
 	"k8s.io/node-problem-detector/pkg/systemlogmonitor/logwatchers"
 	watchertypes "k8s.io/node-problem-detector/pkg/systemlogmonitor/logwatchers/types"
 	logtypes "k8s.io/node-problem-detector/pkg/systemlogmonitor/types"
 	systemlogtypes "k8s.io/node-problem-detector/pkg/systemlogmonitor/types"
 	"k8s.io/node-problem-detector/pkg/types"
-
-	"github.com/golang/glog"
+	"k8s.io/node-problem-detector/pkg/util"
 	"k8s.io/node-problem-detector/pkg/util/tomb"
 )
 
@@ -138,11 +139,17 @@ func (l *logMonitor) generateStatus(logs []*logtypes.Log, rule systemlogtypes.Ru
 				// Update transition timestamp and message when the condition
 				// changes. Condition is considered to be changed only when
 				// status or reason changes.
-				if !condition.Status || condition.Reason != rule.Reason {
+				if condition.Status == types.False || condition.Reason != rule.Reason {
 					condition.Transition = timestamp
 					condition.Message = message
+					events = append(events, util.GenerateConditionChangeEvent(
+						condition.Type,
+						types.True,
+						rule.Reason,
+						timestamp,
+					))
 				}
-				condition.Status = true
+				condition.Status = types.True
 				condition.Reason = rule.Reason
 				break
 			}
@@ -172,8 +179,7 @@ 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].Status = types.False
 		conditions[i].Transition = time.Now()
 	}
 	return conditions

pkg/systemlogmonitor/log_monitor_test.go

@@ -28,6 +28,7 @@ import (
 	watchertest "k8s.io/node-problem-detector/pkg/systemlogmonitor/logwatchers/testing"
 	logtypes "k8s.io/node-problem-detector/pkg/systemlogmonitor/types"
 	"k8s.io/node-problem-detector/pkg/types"
+	"k8s.io/node-problem-detector/pkg/util"
 )
 
 const (
@@ -40,13 +41,13 @@ func TestGenerateStatus(t *testing.T) {
 	initConditions := []types.Condition{
 		{
 			Type:       testConditionA,
-			Status:     true,
+			Status:     types.True,
 			Transition: time.Unix(500, 500),
 			Reason:     "initial reason",
 		},
 		{
 			Type:       testConditionB,
-			Status:     false,
+			Status:     types.False,
 			Transition: time.Unix(500, 500),
 		},
 	}
@@ -73,10 +74,16 @@ func TestGenerateStatus(t *testing.T) {
 			},
 			expected: types.Status{
 				Source: testSource,
+				Events: []types.Event{util.GenerateConditionChangeEvent(
+					testConditionA,
+					types.True,
+					"test reason",
+					time.Unix(1000, 1000),
+				)},
 				Conditions: []types.Condition{
 					{
 						Type:       testConditionA,
-						Status:     true,
+						Status:     types.True,
 						Transition: time.Unix(1000, 1000),
 						Reason:     "test reason",
 						Message:    "test message 1\ntest message 2",
@@ -97,7 +104,7 @@ func TestGenerateStatus(t *testing.T) {
 				Conditions: []types.Condition{
 					{
 						Type:       testConditionA,
-						Status:     true,
+						Status:     types.True,
 						Transition: time.Unix(500, 500),
 						Reason:     "initial reason",
 					},

pkg/types/types.go

@@ -38,13 +38,25 @@ const (
 	Warn Severity = "warn"
 )
 
+// ConditionStatus is the status of the condition.
+type ConditionStatus string
+
+const (
+	// True means the condition status is true.
+	True ConditionStatus = "True"
+	// False means the condition status is false.
+	False ConditionStatus = "False"
+	// Unknown means the condition status is unknown.
+	Unknown ConditionStatus = "Unknown"
+)
+
 // Condition is the node condition used internally by problem detector.
 type Condition struct {
 	// Type is the condition type. It should describe the condition of node in problem. For example
 	// KernelDeadlock, OutOfResource etc.
 	Type string `json:"type"`
 	// Status indicates whether the node is in the condition or not.
-	Status bool `json:"status"`
+	Status ConditionStatus `json:"status"`
 	// Transition is the time when the node transits to this condition.
 	Transition time.Time `json:"transition"`
 	// Reason is a short reason of why node goes into this condition.

pkg/util/convert.go

@@ -37,11 +37,17 @@ func ConvertToAPICondition(condition types.Condition) api.NodeCondition {
 }
 
 // ConvertToAPIConditionStatus converts the internal node condition status to api.ConditionStatus.
-func ConvertToAPIConditionStatus(status bool) api.ConditionStatus {
-	if status {
+func ConvertToAPIConditionStatus(status types.ConditionStatus) api.ConditionStatus {
+	switch status {
+	case types.True:
 		return api.ConditionTrue
+	case types.False:
+		return api.ConditionFalse
+	case types.Unknown:
+		return api.ConditionUnknown
+	default:
+		panic("unknown condition status")
 	}
-	return api.ConditionFalse
 }
 
 // ConvertToAPIEventType converts the internal severity to event type.

pkg/util/convert_test.go

@@ -30,7 +30,7 @@ func TestConvertToAPICondition(t *testing.T) {
 	now := time.Now()
 	condition := types.Condition{
 		Type:       "TestCondition",
-		Status:     true,
+		Status:     types.True,
 		Transition: now,
 		Reason:     "test reason",
 		Message:    "test message",

pkg/util/helpers.go

@@ -0,0 +1,33 @@
+/*
+Copyright 2017 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 util
+
+import (
+	"fmt"
+	"time"
+
+	"k8s.io/node-problem-detector/pkg/types"
+)
+
+// GenerateConditionChangeEvent generates an event for condition change.
+func GenerateConditionChangeEvent(t string, status types.ConditionStatus, reason string, timestamp time.Time) types.Event {
+	return types.Event{
+		Severity:  types.Info,
+		Timestamp: timestamp,
+		Reason:    reason,
+		Message:   fmt.Sprintf("Node condition %s is now: %s, reason: %s", t, status, reason),
+	}
+}