Change the eviction metric type and fix rate-limited-timed-queue

pkg/controller/node/metrics.go

@@ -18,11 +18,7 @@ package node
 
 import (
 	"sync"
-	"time"
 
-	"k8s.io/kubernetes/pkg/api/unversioned"
-
-	"github.com/golang/glog"
 	"github.com/prometheus/client_golang/prometheus"
 )
 
@@ -31,8 +27,7 @@ const (
 	ZoneHealthStatisticKey  = "zone_health"
 	ZoneSizeKey             = "zone_size"
 	ZoneNoUnhealthyNodesKey = "unhealty_nodes_in_zone"
-	EvictionsIn10MinutesKey = "10_minute_evictions"
-	EvictionsIn1HourKey     = "1_hour_evictions"
+	EvictionsNumberKey      = "evictions_number"
 )
 
 var (
@@ -60,19 +55,11 @@ var (
 		},
 		[]string{"zone"},
 	)
-	Evictions10Minutes = prometheus.NewGaugeVec(
-		prometheus.GaugeOpts{
-			Subsystem: NodeControllerSubsystem,
-			Name:      EvictionsIn10MinutesKey,
-			Help:      "Gauge measuring number of Node evictions that happened in previous 10 minutes per zone.",
-		},
-		[]string{"zone"},
-	)
-	Evictions1Hour = prometheus.NewGaugeVec(
-		prometheus.GaugeOpts{
+	EvictionsNumber = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
 			Subsystem: NodeControllerSubsystem,
-			Name:      EvictionsIn1HourKey,
-			Help:      "Gauge measuring number of Node evictions that happened in previous hour per zone.",
+			Name:      EvictionsNumberKey,
+			Help:      "Number of Node evictions that happened since current instance of NodeController started.",
 		},
 		[]string{"zone"},
 	)
@@ -85,117 +72,6 @@ func Register() {
 		prometheus.MustRegister(ZoneHealth)
 		prometheus.MustRegister(ZoneSize)
 		prometheus.MustRegister(UnhealthyNodes)
-		prometheus.MustRegister(Evictions10Minutes)
-		prometheus.MustRegister(Evictions1Hour)
+		prometheus.MustRegister(EvictionsNumber)
 	})
 }
-
-type eviction struct {
-	node string
-	time unversioned.Time
-}
-
-type evictionData struct {
-	sync.Mutex
-	nodeEvictionCount map[string]map[string]int
-	nodeEvictionList  []eviction
-	now               func() unversioned.Time
-	windowSize        time.Duration
-}
-
-func newEvictionData(windowSize time.Duration) *evictionData {
-	return &evictionData{
-		nodeEvictionCount: make(map[string]map[string]int),
-		nodeEvictionList:  make([]eviction, 0),
-		now:               unversioned.Now,
-		windowSize:        windowSize,
-	}
-}
-
-func (e *evictionData) slideWindow() {
-	e.Lock()
-	defer e.Unlock()
-	now := e.now()
-	firstInside := 0
-	for _, v := range e.nodeEvictionList {
-		if v.time.Add(e.windowSize).Before(now.Time) {
-			firstInside++
-			zone := ""
-			for z := range e.nodeEvictionCount {
-				if _, ok := e.nodeEvictionCount[z][v.node]; ok {
-					zone = z
-					break
-				}
-			}
-			if zone == "" {
-				glog.Warningf("EvictionData corruption - unknown zone for node %v", v.node)
-				continue
-			}
-			if e.nodeEvictionCount[zone][v.node] > 1 {
-				e.nodeEvictionCount[zone][v.node] = e.nodeEvictionCount[zone][v.node] - 1
-			} else {
-				delete(e.nodeEvictionCount[zone], v.node)
-			}
-		} else {
-			break
-		}
-	}
-	e.nodeEvictionList = e.nodeEvictionList[firstInside:]
-}
-
-func (e *evictionData) registerEviction(node, zone string) {
-	e.Lock()
-	defer e.Unlock()
-
-	e.nodeEvictionList = append(e.nodeEvictionList, eviction{node: node, time: e.now()})
-	if _, ok := e.nodeEvictionCount[zone]; !ok {
-		e.nodeEvictionCount[zone] = make(map[string]int)
-	}
-	if _, ok := e.nodeEvictionCount[zone][node]; !ok {
-		e.nodeEvictionCount[zone][node] = 1
-	} else {
-		e.nodeEvictionCount[zone][node] = e.nodeEvictionCount[zone][node] + 1
-	}
-}
-
-func (e *evictionData) removeEviction(node, zone string) {
-	e.Lock()
-	defer e.Unlock()
-
-	// TODO: This may be inefficient, but hopefully will be rarely called. Verify that this is true.
-	for i := len(e.nodeEvictionList) - 1; i >= 0; i-- {
-		if e.nodeEvictionList[i].node == node {
-			e.nodeEvictionList = append(e.nodeEvictionList[:i], e.nodeEvictionList[i+1:]...)
-			break
-		}
-	}
-	if e.nodeEvictionCount[zone][node] > 1 {
-		e.nodeEvictionCount[zone][node] = e.nodeEvictionCount[zone][node] - 1
-	} else {
-		delete(e.nodeEvictionCount[zone], node)
-	}
-}
-
-func (e *evictionData) countEvictions(zone string) int {
-	e.Lock()
-	defer e.Unlock()
-	return len(e.nodeEvictionCount[zone])
-}
-
-func (e *evictionData) getZones() []string {
-	e.Lock()
-	defer e.Unlock()
-
-	zones := make([]string, 0, len(e.nodeEvictionCount))
-	for k := range e.nodeEvictionCount {
-		zones = append(zones, k)
-	}
-	return zones
-}
-
-func (e *evictionData) initZone(zone string) {
-	e.Lock()
-	defer e.Unlock()
-
-	e.nodeEvictionCount[zone] = make(map[string]int)
-}

pkg/controller/node/nodecontroller.go

@@ -46,8 +46,6 @@ import (
 	"k8s.io/kubernetes/pkg/util/wait"
 	"k8s.io/kubernetes/pkg/version"
 	"k8s.io/kubernetes/pkg/watch"
-
-	"github.com/prometheus/client_golang/prometheus"
 )
 
 func init() {
@@ -167,9 +165,6 @@ type NodeController struct {
 	// the controller using NewDaemonSetsController(passing SharedInformer), this
 	// will be null
 	internalPodInformer framework.SharedIndexInformer
-
-	evictions10Minutes *evictionData
-	evictions1Hour     *evictionData
 }
 
 // NewNodeController returns a new node controller to sync instances from cloudprovider.
@@ -241,8 +236,6 @@ func NewNodeController(
 		largeClusterThreshold:       largeClusterThreshold,
 		unhealthyZoneThreshold:      unhealthyZoneThreshold,
 		zoneStates:                  make(map[string]zoneState),
-		evictions10Minutes:          newEvictionData(10 * time.Minute),
-		evictions1Hour:              newEvictionData(time.Hour),
 	}
 	nc.enterPartialDisruptionFunc = nc.ReducedQPSFunc
 	nc.enterFullDisruptionFunc = nc.HealthyQPSFunc
@@ -417,16 +410,15 @@ func (nc *NodeController) Run() {
 		defer nc.evictorLock.Unlock()
 		for k := range nc.zonePodEvictor {
 			nc.zonePodEvictor[k].Try(func(value TimedValue) (bool, time.Duration) {
-				obj, exists, err := nc.nodeStore.Get(value.Value)
+				obj, exists, err := nc.nodeStore.GetByKey(value.Value)
 				if err != nil {
 					glog.Warningf("Failed to get Node %v from the nodeStore: %v", value.Value, err)
 				} else if !exists {
 					glog.Warningf("Node %v no longer present in nodeStore!", value.Value)
 				} else {
 					node, _ := obj.(*api.Node)
 					zone := utilnode.GetZoneKey(node)
-					nc.evictions10Minutes.registerEviction(zone, value.Value)
-					nc.evictions1Hour.registerEviction(zone, value.Value)
+					EvictionsNumber.WithLabelValues(zone).Inc()
 				}
 
 				nodeUid, _ := value.UID.(string)
@@ -506,8 +498,9 @@ func (nc *NodeController) monitorNodeStatus() error {
 			nc.zonePodEvictor[zone] =
 				NewRateLimitedTimedQueue(
 					flowcontrol.NewTokenBucketRateLimiter(nc.evictionLimiterQPS, evictionRateLimiterBurst))
-			nc.evictions10Minutes.initZone(zone)
-			nc.evictions1Hour.initZone(zone)
+			// Init the metric for the new zone.
+			glog.Infof("Initilizing eviction metric for zone: %v", zone)
+			EvictionsNumber.WithLabelValues(zone).Add(0)
 		}
 		if _, found := nc.zoneTerminationEvictor[zone]; !found {
 			nc.zoneTerminationEvictor[zone] = NewRateLimitedTimedQueue(
@@ -607,20 +600,10 @@ func (nc *NodeController) monitorNodeStatus() error {
 		}
 	}
 	nc.handleDisruption(zoneToNodeConditions, nodes)
-	nc.updateEvictionMetric(Evictions10Minutes, nc.evictions10Minutes)
-	nc.updateEvictionMetric(Evictions1Hour, nc.evictions1Hour)
 
 	return nil
 }
 
-func (nc *NodeController) updateEvictionMetric(metric *prometheus.GaugeVec, data *evictionData) {
-	data.slideWindow()
-	zones := data.getZones()
-	for _, z := range zones {
-		metric.WithLabelValues(z).Set(float64(data.countEvictions(z)))
-	}
-}
-
 func (nc *NodeController) handleDisruption(zoneToNodeConditions map[string][]*api.NodeCondition, nodes *api.NodeList) {
 	newZoneStates := map[string]zoneState{}
 	allAreFullyDisrupted := true
@@ -924,8 +907,6 @@ func (nc *NodeController) cancelPodEviction(node *api.Node) bool {
 	wasTerminating := nc.zoneTerminationEvictor[zone].Remove(node.Name)
 	if wasDeleting || wasTerminating {
 		glog.V(2).Infof("Cancelling pod Eviction on Node: %v", node.Name)
-		nc.evictions10Minutes.removeEviction(zone, node.Name)
-		nc.evictions1Hour.removeEviction(zone, node.Name)
 		return true
 	}
 	return false