github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #6214 from gmarek/move_consts_to_flags

Browse Source 
Move NodeController constants to flags
This commit is contained in:
David Oppenheimer
2015-04-07 11:31:11 -07:00
parent ef3cdb2f18 321a81047c
commit 402bf60366
7 changed files with 118 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
pkg/cloudprovider/controller/nodecontroller.go
View File

@@ -34,40 +34,17 @@ import (
"github.com/golang/glog"
)
const (
// The constant is used if sync_nodes_status=False. NodeController will not proactively
// sync node status in this case, but will monitor node status updated from kubelet. If
// it doesn't receive update for this amount of time, it will start posting "NodeReady==
// ConditionUnknown". The amount of time before which NodeController start evicting pods
// is controlled via flag 'pod_eviction_timeout'.
// Note: be cautious when changing the constant, it must work with nodeStatusUpdateFrequency
// in kubelet. There are several constraints:
// 1. nodeMonitorGracePeriod must be N times more than nodeStatusUpdateFrequency, where
// N means number of retries allowed for kubelet to post node status. It is pointless
// to make nodeMonitorGracePeriod be less than nodeStatusUpdateFrequency, since there
// will only be fresh values from Kubelet at an interval of nodeStatusUpdateFrequency.
// The constant must be less than podEvictionTimeout.
// 2. nodeMonitorGracePeriod can't be too large for user experience - larger value takes
// longer for user to see up-to-date node status.
nodeMonitorGracePeriod = 8 * time.Second
// The constant is used if sync_nodes_status=False, only for node startup. When node
// is just created, e.g. cluster bootstrap or node creation, we give a longer grace period.
nodeStartupGracePeriod = 30 * time.Second
// The constant is used if sync_nodes_status=False. It controls NodeController monitoring
// period, i.e. how often does NodeController check node status posted from kubelet.
// Theoretically, this value should be lower than nodeMonitorGracePeriod.
// TODO: Change node status monitor to watch based.
nodeMonitorPeriod = 5 * time.Second
// Constant controlling number of retries of writing NodeStatus update.
nodeStatusUpdateRetry = 5
)
var (
ErrRegistration = errors.New("unable to register all nodes.")
ErrQueryIPAddress = errors.New("unable to query IP address.")
ErrCloudInstance = errors.New("cloud provider doesn't support instances.")
)
const (
// Constant controlling number of retries of writing NodeStatus update.
nodeStatusUpdateRetry = 5
)
type NodeStatusData struct {
probeTimestamp util.Time
readyTransitionTimestamp util.Time
@@ -88,6 +65,28 @@ type NodeController struct {
// This timestamp is to be used instead of LastProbeTime stored in Condition. We do this
// to aviod the problem with time skew across the cluster.
nodeStatusMap map[string]NodeStatusData
// Value used if sync_nodes_status=False. NodeController will not proactively
// sync node status in this case, but will monitor node status updated from kubelet. If
// it doesn't receive update for this amount of time, it will start posting "NodeReady==
// ConditionUnknown". The amount of time before which NodeController start evicting pods
// is controlled via flag 'pod_eviction_timeout'.
// Note: be cautious when changing the constant, it must work with nodeStatusUpdateFrequency
// in kubelet. There are several constraints:
// 1. nodeMonitorGracePeriod must be N times more than nodeStatusUpdateFrequency, where
// N means number of retries allowed for kubelet to post node status. It is pointless
// to make nodeMonitorGracePeriod be less than nodeStatusUpdateFrequency, since there
// will only be fresh values from Kubelet at an interval of nodeStatusUpdateFrequency.
// The constant must be less than podEvictionTimeout.
// 2. nodeMonitorGracePeriod can't be too large for user experience - larger value takes
// longer for user to see up-to-date node status.
nodeMonitorGracePeriod time.Duration
// Value used if sync_nodes_status=False, only for node startup. When node
// is just created, e.g. cluster bootstrap or node creation, we give a longer grace period.
nodeStartupGracePeriod time.Duration
// Value controlling NodeController monitoring period, i.e. how often does NodeController
// check node status posted from kubelet. Theoretically, this value should be lower than nodeMonitorGracePeriod.
// TODO: Change node status monitor to watch based.
nodeMonitorPeriod time.Duration
// Method for easy mocking in unittest.
lookupIP func(host string) ([]net.IP, error)
now func() util.Time
@@ -103,7 +102,10 @@ func NewNodeController(
kubeletClient client.KubeletClient,
registerRetryCount int,
podEvictionTimeout time.Duration,
deletingPodsRateLimiter util.RateLimiter) *NodeController {
deletingPodsRateLimiter util.RateLimiter,
nodeMonitorGracePeriod time.Duration,
nodeStartupGracePeriod time.Duration,
nodeMonitorPeriod time.Duration) *NodeController {
return &NodeController{
cloud: cloud,
matchRE: matchRE,
@@ -115,6 +117,9 @@ func NewNodeController(
podEvictionTimeout: podEvictionTimeout,
deletingPodsRateLimiter: deletingPodsRateLimiter,
nodeStatusMap: make(map[string]NodeStatusData),
nodeMonitorGracePeriod: nodeMonitorGracePeriod,
nodeMonitorPeriod: nodeMonitorPeriod,
nodeStartupGracePeriod: nodeStartupGracePeriod,
lookupIP: net.LookupIP,
now: util.Now,
}
@@ -177,7 +182,7 @@ func (nc *NodeController) Run(period time.Duration, syncNodeList, syncNodeStatus
if err := nc.MonitorNodeStatus(); err != nil {
glog.Errorf("Error monitoring node status: %v", err)
}
}, nodeMonitorPeriod)
}, nc.nodeMonitorPeriod)
}
}
@@ -438,7 +443,7 @@ func (nc *NodeController) tryUpdateNodeStatus(node *api.Node) (time.Duration, ap
LastProbeTime: node.CreationTimestamp,
LastTransitionTime: node.CreationTimestamp,
}
gracePeriod = nodeStartupGracePeriod
gracePeriod = nc.nodeStartupGracePeriod
nc.nodeStatusMap[node.Name] = NodeStatusData{
status: node.Status,
probeTimestamp: node.CreationTimestamp,
@@ -447,7 +452,7 @@ func (nc *NodeController) tryUpdateNodeStatus(node *api.Node) (time.Duration, ap
} else {
// If ready condition is not nil, make a copy of it, since we may modify it in place later.
lastReadyCondition = *readyCondition
gracePeriod = nodeMonitorGracePeriod
gracePeriod = nc.nodeMonitorGracePeriod
}
savedNodeStatus, found := nc.nodeStatusMap[node.Name]
@@ -590,6 +595,7 @@ func (nc *NodeController) MonitorNodeStatus() error {
nc.now().After(nc.nodeStatusMap[node.Name].readyTransitionTimestamp.Add(nc.podEvictionTimeout)) {
// Node stays in not ready for at least 'podEvictionTimeout' - evict all pods on the unhealthy node.
// Makes sure we are not removing pods from to many nodes in the same time.
glog.Infof("Evicting pods: %v is later than %v + %v", nc.now(), nc.nodeStatusMap[node.Name].readyTransitionTimestamp, nc.podEvictionTimeout)
if nc.deletingPodsRateLimiter.CanAccept() {
nc.deletePods(node.Name)
}
@@ -598,6 +604,7 @@ func (nc *NodeController) MonitorNodeStatus() error {
nc.now().After(nc.nodeStatusMap[node.Name].probeTimestamp.Add(nc.podEvictionTimeout-gracePeriod)) {
// Same as above. Note however, since condition unknown is posted by node controller, which means we
// need to substract monitoring grace period in order to get the real 'podEvictionTimeout'.
glog.Infof("Evicting pods2: %v is later than %v + %v", nc.now(), nc.nodeStatusMap[node.Name].readyTransitionTimestamp, nc.podEvictionTimeout-gracePeriod)
if nc.deletingPodsRateLimiter.CanAccept() {
nc.deletePods(node.Name)
}

58
pkg/cloudprovider/controller/nodecontroller_test.go
View File

@@ -38,6 +38,12 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
)
const (
testNodeMonitorGracePeriod = 8 * time.Second
testNodeStartupGracePeriod = 30 * time.Second
testNodeMonitorPeriod = 5 * time.Second
)
// FakeNodeHandler is a fake implementation of NodesInterface and NodeInterface. It
// allows test cases to have fine-grained control over mock behaviors. We also need
// PodsInterface and PodInterface to test list & delet pods, which is implemented in
@@ -247,7 +253,8 @@ func TestRegisterNodes(t *testing.T) {
for _, machine := range item.machines {
nodes.Items = append(nodes.Items, *newNode(machine))
}
nodeController := NewNodeController(nil, "", item.machines, &api.NodeResources{}, item.fakeNodeHandler, nil, 10, time.Minute, util.NewFakeRateLimiter())
nodeController := NewNodeController(nil, "", item.machines, &api.NodeResources{}, item.fakeNodeHandler, nil, 10, time.Minute,
util.NewFakeRateLimiter(), testNodeMonitorGracePeriod, testNodeStartupGracePeriod, testNodeMonitorPeriod)
err := nodeController.RegisterNodes(&nodes, item.retryCount, time.Millisecond)
if !item.expectedFail && err != nil {
t.Errorf("unexpected error: %v", err)
@@ -332,7 +339,8 @@ func TestCreateGetStaticNodesWithSpec(t *testing.T) {
},
}
for _, item := range table {
nodeController := NewNodeController(nil, "", item.machines, &resources, nil, nil, 10, time.Minute, util.NewFakeRateLimiter())
nodeController := NewNodeController(nil, "", item.machines, &resources, nil, nil, 10, time.Minute,
util.NewFakeRateLimiter(), testNodeMonitorGracePeriod, testNodeStartupGracePeriod, testNodeMonitorPeriod)
nodes, err := nodeController.GetStaticNodesWithSpec()
if err != nil {
t.Errorf("unexpected error: %v", err)
@@ -393,7 +401,8 @@ func TestCreateGetCloudNodesWithSpec(t *testing.T) {
}
for _, item := range table {
nodeController := NewNodeController(item.fakeCloud, ".*", nil, &api.NodeResources{}, nil, nil, 10, time.Minute, util.NewFakeRateLimiter())
nodeController := NewNodeController(item.fakeCloud, ".*", nil, &api.NodeResources{}, nil, nil, 10, time.Minute,
util.NewFakeRateLimiter(), testNodeMonitorGracePeriod, testNodeStartupGracePeriod, testNodeMonitorPeriod)
nodes, err := nodeController.GetCloudNodesWithSpec()
if err != nil {
t.Errorf("unexpected error: %v", err)
@@ -499,7 +508,8 @@ func TestSyncCloudNodes(t *testing.T) {
}
for _, item := range table {
nodeController := NewNodeController(item.fakeCloud, item.matchRE, nil, &api.NodeResources{}, item.fakeNodeHandler, nil, 10, time.Minute, util.NewFakeRateLimiter())
nodeController := NewNodeController(item.fakeCloud, item.matchRE, nil, &api.NodeResources{}, item.fakeNodeHandler, nil, 10, time.Minute,
util.NewFakeRateLimiter(), testNodeMonitorGracePeriod, testNodeStartupGracePeriod, testNodeMonitorPeriod)
if err := nodeController.SyncCloudNodes(); err != nil {
t.Errorf("unexpected error: %v", err)
}
@@ -585,7 +595,8 @@ func TestSyncCloudNodesEvictPods(t *testing.T) {
}
for _, item := range table {
nodeController := NewNodeController(item.fakeCloud, item.matchRE, nil, &api.NodeResources{}, item.fakeNodeHandler, nil, 10, time.Minute, util.NewFakeRateLimiter())
nodeController := NewNodeController(item.fakeCloud, item.matchRE, nil, &api.NodeResources{}, item.fakeNodeHandler, nil, 10, time.Minute,
util.NewFakeRateLimiter(), testNodeMonitorGracePeriod, testNodeStartupGracePeriod, testNodeMonitorPeriod)
if err := nodeController.SyncCloudNodes(); err != nil {
t.Errorf("unexpected error: %v", err)
}
@@ -665,7 +676,8 @@ func TestNodeConditionsCheck(t *testing.T) {
}
for _, item := range table {
nodeController := NewNodeController(nil, "", nil, nil, nil, item.fakeKubeletClient, 10, time.Minute, util.NewFakeRateLimiter())
nodeController := NewNodeController(nil, "", nil, nil, nil, item.fakeKubeletClient, 10, time.Minute,
util.NewFakeRateLimiter(), testNodeMonitorGracePeriod, testNodeStartupGracePeriod, testNodeMonitorPeriod)
nodeController.now = func() util.Time { return fakeNow }
conditions := nodeController.DoCheck(item.node)
if !reflect.DeepEqual(item.expectedConditions, conditions) {
@@ -696,7 +708,8 @@ func TestPopulateNodeAddresses(t *testing.T) {
}
for _, item := range table {
nodeController := NewNodeController(item.fakeCloud, ".*", nil, nil, nil, nil, 10, time.Minute, util.NewFakeRateLimiter())
nodeController := NewNodeController(item.fakeCloud, ".*", nil, nil, nil, nil, 10, time.Minute,
util.NewFakeRateLimiter(), testNodeMonitorGracePeriod, testNodeStartupGracePeriod, testNodeMonitorPeriod)
result, err := nodeController.PopulateAddresses(item.nodes)
// In case of IP querying error, we should continue.
if err != nil {
@@ -785,7 +798,8 @@ func TestSyncProbedNodeStatus(t *testing.T) {
}
for _, item := range table {
nodeController := NewNodeController(item.fakeCloud, ".*", nil, nil, item.fakeNodeHandler, item.fakeKubeletClient, 10, time.Minute, util.NewFakeRateLimiter())
nodeController := NewNodeController(item.fakeCloud, ".*", nil, nil, item.fakeNodeHandler, item.fakeKubeletClient, 10, time.Minute,
util.NewFakeRateLimiter(), testNodeMonitorGracePeriod, testNodeStartupGracePeriod, testNodeMonitorPeriod)
nodeController.now = func() util.Time { return fakeNow }
if err := nodeController.SyncProbedNodeStatus(); err != nil {
t.Errorf("unexpected error: %v", err)
@@ -876,7 +890,8 @@ func TestSyncProbedNodeStatusTransitionTime(t *testing.T) {
}
for _, item := range table {
nodeController := NewNodeController(nil, "", []string{"node0"}, nil, item.fakeNodeHandler, item.fakeKubeletClient, 10, time.Minute, util.NewFakeRateLimiter())
nodeController := NewNodeController(nil, "", []string{"node0"}, nil, item.fakeNodeHandler, item.fakeKubeletClient, 10, time.Minute,
util.NewFakeRateLimiter(), testNodeMonitorGracePeriod, testNodeStartupGracePeriod, testNodeMonitorPeriod)
nodeController.lookupIP = func(host string) ([]net.IP, error) { return nil, fmt.Errorf("lookup %v: no such host", host) }
nodeController.now = func() util.Time { return fakeNow }
if err := nodeController.SyncProbedNodeStatus(); err != nil {
@@ -1029,7 +1044,8 @@ func TestSyncProbedNodeStatusEvictPods(t *testing.T) {
}
for _, item := range table {
nodeController := NewNodeController(nil, "", []string{"node0"}, nil, item.fakeNodeHandler, item.fakeKubeletClient, 10, 5*time.Minute, util.NewFakeRateLimiter())
nodeController := NewNodeController(nil, "", []string{"node0"}, nil, item.fakeNodeHandler, item.fakeKubeletClient, 10, 5*time.Minute,
util.NewFakeRateLimiter(), testNodeMonitorGracePeriod, testNodeStartupGracePeriod, testNodeMonitorPeriod)
nodeController.lookupIP = func(host string) ([]net.IP, error) { return nil, fmt.Errorf("lookup %v: no such host", host) }
if err := nodeController.SyncProbedNodeStatus(); err != nil {
t.Errorf("unexpected error: %v", err)
@@ -1045,13 +1061,14 @@ func TestSyncProbedNodeStatusEvictPods(t *testing.T) {
func TestMonitorNodeStatusEvictPods(t *testing.T) {
fakeNow := util.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC)
evictionTimeout := 30 * time.Minute
evictionTimeout := 10 * time.Minute
table := []struct {
fakeNodeHandler *FakeNodeHandler
timeToPass time.Duration
newNodeStatus api.NodeStatus
expectedEvictPods bool
description string
}{
// Node created recently, with no status (happens only at cluster startup).
{
@@ -1071,6 +1088,7 @@ func TestMonitorNodeStatusEvictPods(t *testing.T) {
timeToPass: 0,
newNodeStatus: api.NodeStatus{},
expectedEvictPods: false,
description: "Node created recently, with no status.",
},
// Node created long time ago, and kubelet posted NotReady for a short period of time.
{
@@ -1097,7 +1115,7 @@ func TestMonitorNodeStatusEvictPods(t *testing.T) {
PodsList: api.PodList{Items: []api.Pod{*newPod("pod0", "node0")}},
},
},
timeToPass: 10 * time.Minute,
timeToPass: evictionTimeout,
newNodeStatus: api.NodeStatus{
Conditions: []api.NodeCondition{
{
@@ -1110,6 +1128,7 @@ func TestMonitorNodeStatusEvictPods(t *testing.T) {
},
},
expectedEvictPods: false,
description: "Node created long time ago, and kubelet posted NotReady for a short period of time.",
},
// Node created long time ago, and kubelet posted NotReady for a long period of time.
{
@@ -1149,6 +1168,7 @@ func TestMonitorNodeStatusEvictPods(t *testing.T) {
},
},
expectedEvictPods: true,
description: "Node created long time ago, and kubelet posted NotReady for a long period of time.",
},
// Node created long time ago, node controller posted Unknown for a short period of time.
{
@@ -1175,7 +1195,7 @@ func TestMonitorNodeStatusEvictPods(t *testing.T) {
PodsList: api.PodList{Items: []api.Pod{*newPod("pod0", "node0")}},
},
},
timeToPass: 10 * time.Minute,
timeToPass: evictionTimeout - testNodeMonitorGracePeriod,
newNodeStatus: api.NodeStatus{
Conditions: []api.NodeCondition{
{
@@ -1188,6 +1208,7 @@ func TestMonitorNodeStatusEvictPods(t *testing.T) {
},
},
expectedEvictPods: false,
description: "Node created long time ago, node controller posted Unknown for a short period of time.",
},
// Node created long time ago, node controller posted Unknown for a long period of time.
{
@@ -1227,11 +1248,14 @@ func TestMonitorNodeStatusEvictPods(t *testing.T) {
},
},
expectedEvictPods: true,
description: "Node created long time ago, node controller posted Unknown for a long period of time.",
},
}
for _, item := range table {
nodeController := NewNodeController(nil, "", []string{"node0"}, nil, item.fakeNodeHandler, nil, 10, evictionTimeout, util.NewFakeRateLimiter())
nodeController := NewNodeController(nil, "", []string{"node0"}, nil, item.fakeNodeHandler, nil, 10,
evictionTimeout, util.NewFakeRateLimiter(), testNodeMonitorGracePeriod,
testNodeStartupGracePeriod, testNodeMonitorPeriod)
nodeController.now = func() util.Time { return fakeNow }
if err := nodeController.MonitorNodeStatus(); err != nil {
t.Errorf("unexpected error: %v", err)
@@ -1250,7 +1274,8 @@ func TestMonitorNodeStatusEvictPods(t *testing.T) {
}
}
if item.expectedEvictPods != podEvicted {
t.Errorf("expected pod eviction: %+v, got %+v", item.expectedEvictPods, podEvicted)
t.Errorf("expected pod eviction: %+v, got %+v for %+v", item.expectedEvictPods,
podEvicted, item.description)
}
}
}
@@ -1439,7 +1464,8 @@ func TestMonitorNodeStatusUpdateStatus(t *testing.T) {
}
for _, item := range table {
nodeController := NewNodeController(nil, "", []string{"node0"}, nil, item.fakeNodeHandler, nil, 10, 5*time.Minute, util.NewFakeRateLimiter())
nodeController := NewNodeController(nil, "", []string{"node0"}, nil, item.fakeNodeHandler, nil, 10, 5*time.Minute, util.NewFakeRateLimiter(),
testNodeMonitorGracePeriod, testNodeStartupGracePeriod, testNodeMonitorPeriod)
nodeController.now = func() util.Time { return fakeNow }
if err := nodeController.MonitorNodeStatus(); err != nil {
t.Errorf("unexpected error: %v", err)