Commit b906e345 authored by Yu-Ju Hong's avatar Yu-Ju Hong

kubelet: trigger pod workers independently

Currently, whenever there is any update, kubelet would force all pod workers to sync again, causing resource contention and hence performance degradation. This commit flips kubelet to use incremental updates (as opposed to snapshots). This allows us to know what pods have changed and send updates to those pod workers only. The `SyncPods` function has been replaced with individual handlers, each handling an operation (ADD, REMOVE, UPDATE). Pod workers are still triggered periodically, and kubelet performs periodic cleanup as well. This commit also spawns a new goroutine solely responsible for killing pods. This is necessary because pod killing could hold up the sync loop for indefinitely long amount of time now user can define the graceful termination period in the container spec.
parent 5dfc904c
......@@ -688,7 +688,7 @@ func startKubelet(k KubeletBootstrap, podCfg *config.PodConfig, kc *KubeletConfi
func makePodSourceConfig(kc *KubeletConfig) *config.PodConfig {
// source of all configuration
cfg := config.NewPodConfig(config.PodConfigNotificationSnapshotAndUpdates, kc.Recorder)
cfg := config.NewPodConfig(config.PodConfigNotificationIncremental, kc.Recorder)
// define file config source
if kc.ConfigFile != "" {
......
......@@ -504,8 +504,6 @@ func (kl *kubeletExecutor) Run(updates <-chan kubelet.PodUpdate) {
util.Until(func() { kl.Kubelet.Run(pipe) }, 0, kl.executorDone)
//TODO(jdef) revisit this if/when executor failover lands
err := kl.SyncPods([]*api.Pod{}, nil, nil, time.Now())
if err != nil {
log.Errorf("failed to cleanly remove all pods and associated state: %v", err)
}
// Force kubelet to delete all pods.
kl.HandlePodDeletions(kl.GetPods())
}
......@@ -249,7 +249,7 @@ func (s *podStorage) merge(source string, change interface{}) (adds, updates, de
}
case kubelet.SET:
glog.V(4).Infof("Setting pods for source %s : %v", source, update)
glog.V(4).Infof("Setting pods for source %s", source)
s.markSourceSet(source)
// Clear the old map entries by just creating a new map
oldPods := pods
......
......@@ -42,6 +42,8 @@ func (f *fakePodWorkers) UpdatePod(pod *api.Pod, mirrorPod *api.Pod, updateCompl
func (f *fakePodWorkers) ForgetNonExistingPodWorkers(desiredPods map[types.UID]empty) {}
func (f *fakePodWorkers) ForgetWorker(uid types.UID) {}
type TestingInterface interface {
Errorf(format string, args ...interface{})
}
......@@ -132,6 +132,7 @@ func newTestKubelet(t *testing.T) *TestKubelet {
fakeClock := &util.FakeClock{Time: time.Now()}
kubelet.backOff = util.NewBackOff(time.Second, time.Minute)
kubelet.backOff.Clock = fakeClock
kubelet.podKillingCh = make(chan *kubecontainer.Pod, 20)
return &TestKubelet{kubelet, fakeRuntime, mockCadvisor, fakeKubeClient, fakeMirrorClient}
}
......@@ -348,10 +349,7 @@ func TestSyncPodsStartPod(t *testing.T) {
},
}
kubelet.podManager.SetPods(pods)
err := kubelet.SyncPods(pods, emptyPodUIDs, map[string]*api.Pod{}, time.Now())
if err != nil {
t.Errorf("unexpected error: %v", err)
}
kubelet.HandlePodSyncs(pods)
fakeRuntime.AssertStartedPods([]string{string(pods[0].UID)})
}
......@@ -375,16 +373,12 @@ func TestSyncPodsDeletesWhenSourcesAreReady(t *testing.T) {
},
},
}
if err := kubelet.SyncPods([]*api.Pod{}, emptyPodUIDs, map[string]*api.Pod{}, time.Now()); err != nil {
t.Errorf("unexpected error: %v", err)
}
kubelet.HandlePodCleanups()
// Sources are not ready yet. Don't remove any pods.
fakeRuntime.AssertKilledPods([]string{})
ready = true
if err := kubelet.SyncPods([]*api.Pod{}, emptyPodUIDs, map[string]*api.Pod{}, time.Now()); err != nil {
t.Errorf("unexpected error: %v", err)
}
kubelet.HandlePodCleanups()
// Sources are ready. Remove unwanted pods.
fakeRuntime.AssertKilledPods([]string{"12345678"})
......@@ -2004,18 +1998,17 @@ func TestGetHostPortConflicts(t *testing.T) {
{Spec: api.PodSpec{Containers: []api.Container{{Ports: []api.ContainerPort{{HostPort: 83}}}}}},
}
// Pods should not cause any conflict.
_, conflicts := checkHostPortConflicts(pods)
if len(conflicts) != 0 {
t.Errorf("expected no conflicts, Got %#v", conflicts)
if hasHostPortConflicts(pods) {
t.Errorf("expected no conflicts, Got conflicts")
}
// The new pod should cause conflict and be reported.
expected := &api.Pod{
Spec: api.PodSpec{Containers: []api.Container{{Ports: []api.ContainerPort{{HostPort: 81}}}}},
}
// The new pod should cause conflict and be reported.
pods = append(pods, expected)
if _, actual := checkHostPortConflicts(pods); !reflect.DeepEqual(actual, []*api.Pod{expected}) {
t.Errorf("expected %#v, Got %#v", expected, actual)
if !hasHostPortConflicts(pods) {
t.Errorf("expected no conflict, Got no conflicts")
}
}
......@@ -2052,7 +2045,7 @@ func TestHandlePortConflicts(t *testing.T) {
// The newer pod should be rejected.
conflictedPod := pods[0]
kl.handleNotFittingPods(pods)
kl.HandlePodAdditions(pods)
// Check pod status stored in the status map.
status, found := kl.statusManager.GetPodStatus(conflictedPod.UID)
if !found {
......@@ -2094,7 +2087,7 @@ func TestHandleNodeSelector(t *testing.T) {
// The first pod should be rejected.
notfittingPod := pods[0]
kl.handleNotFittingPods(pods)
kl.HandlePodAdditions(pods)
// Check pod status stored in the status map.
status, found := kl.statusManager.GetPodStatus(notfittingPod.UID)
if !found {
......@@ -2142,7 +2135,7 @@ func TestHandleMemExceeded(t *testing.T) {
// The newer pod should be rejected.
notfittingPod := pods[0]
kl.handleNotFittingPods(pods)
kl.HandlePodAdditions(pods)
// Check pod status stored in the status map.
status, found := kl.statusManager.GetPodStatus(notfittingPod.UID)
if !found {
......@@ -2167,12 +2160,13 @@ func TestPurgingObsoleteStatusMapEntries(t *testing.T) {
}
podToTest := pods[1]
// Run once to populate the status map.
kl.handleNotFittingPods(pods)
kl.HandlePodAdditions(pods)
if _, found := kl.statusManager.GetPodStatus(podToTest.UID); !found {
t.Fatalf("expected to have status cached for pod2")
}
// Sync with empty pods so that the entry in status map will be removed.
kl.SyncPods([]*api.Pod{}, emptyPodUIDs, map[string]*api.Pod{}, time.Now())
kl.podManager.SetPods([]*api.Pod{})
kl.HandlePodCleanups()
if _, found := kl.statusManager.GetPodStatus(podToTest.UID); found {
t.Fatalf("expected to not have status cached for pod2")
}
......@@ -2695,12 +2689,8 @@ func TestDeleteOrphanedMirrorPods(t *testing.T) {
}
kl.podManager.SetPods(orphanPods)
pods, mirrorMap := kl.podManager.GetPodsAndMirrorMap()
// Sync with an empty pod list to delete all mirror pods.
err := kl.SyncPods(pods, emptyPodUIDs, mirrorMap, time.Now())
if err != nil {
t.Errorf("unexpected error: %v", err)
}
kl.HandlePodCleanups()
if manager.NumOfPods() != 0 {
t.Errorf("expected zero mirror pods, got %v", manager.GetPods())
}
......@@ -2802,7 +2792,7 @@ func TestDoNotCacheStatusForStaticPods(t *testing.T) {
{
ObjectMeta: api.ObjectMeta{
UID: "12345678",
Name: "foo",
Name: "staticFoo",
Namespace: "new",
Annotations: map[string]string{
ConfigSourceAnnotationKey: "file",
......@@ -2815,11 +2805,9 @@ func TestDoNotCacheStatusForStaticPods(t *testing.T) {
},
},
}
kubelet.podManager.SetPods(pods)
err := kubelet.SyncPods(pods, emptyPodUIDs, map[string]*api.Pod{}, time.Now())
if err != nil {
t.Errorf("unexpected error: %v", err)
}
kubelet.HandlePodSyncs(kubelet.podManager.GetPods())
status, ok := kubelet.statusManager.GetPodStatus(pods[0].UID)
if ok {
t.Errorf("unexpected status %#v found for static pod %q", status, pods[0].UID)
......@@ -3147,10 +3135,7 @@ func TestSyncPodsSetStatusToFailedForPodsThatRunTooLong(t *testing.T) {
}
// Let the pod worker sets the status to fail after this sync.
err := kubelet.SyncPods(pods, emptyPodUIDs, map[string]*api.Pod{}, time.Now())
if err != nil {
t.Errorf("unexpected error: %v", err)
}
kubelet.HandlePodUpdates(pods)
status, found := kubelet.statusManager.GetPodStatus(pods[0].UID)
if !found {
t.Errorf("expected to found status for pod %q", pods[0].UID)
......@@ -3201,10 +3186,7 @@ func TestSyncPodsDoesNotSetPodsThatDidNotRunTooLongToFailed(t *testing.T) {
}
kubelet.podManager.SetPods(pods)
err := kubelet.SyncPods(pods, emptyPodUIDs, map[string]*api.Pod{}, time.Now())
if err != nil {
t.Errorf("unexpected error: %v", err)
}
kubelet.HandlePodUpdates(pods)
status, found := kubelet.statusManager.GetPodStatus(pods[0].UID)
if !found {
t.Errorf("expected to found status for pod %q", pods[0].UID)
......@@ -3239,10 +3221,7 @@ func TestDeletePodDirsForDeletedPods(t *testing.T) {
kl.podManager.SetPods(pods)
// Sync to create pod directories.
err := kl.SyncPods(pods, emptyPodUIDs, map[string]*api.Pod{}, time.Now())
if err != nil {
t.Errorf("unexpected error: %v", err)
}
kl.HandlePodSyncs(kl.podManager.GetPods())
for i := range pods {
if !dirExists(kl.getPodDir(pods[i].UID)) {
t.Errorf("expected directory to exist for pod %d", i)
......@@ -3250,10 +3229,8 @@ func TestDeletePodDirsForDeletedPods(t *testing.T) {
}
// Pod 1 has been deleted and no longer exists.
err = kl.SyncPods([]*api.Pod{pods[0]}, emptyPodUIDs, map[string]*api.Pod{}, time.Now())
if err != nil {
t.Errorf("unexpected error: %v", err)
}
kl.podManager.SetPods([]*api.Pod{pods[0]})
kl.HandlePodCleanups()
if !dirExists(kl.getPodDir(pods[0].UID)) {
t.Errorf("expected directory to exist for pod 0")
}
......@@ -3294,10 +3271,7 @@ func TestDoesNotDeletePodDirsForTerminatedPods(t *testing.T) {
kl.podManager.SetPods(pods)
// Sync to create pod directories.
err := kl.SyncPods(pods, emptyPodUIDs, map[string]*api.Pod{}, time.Now())
if err != nil {
t.Errorf("unexpected error: %v", err)
}
kl.HandlePodSyncs(pods)
for i := range pods {
if !dirExists(kl.getPodDir(pods[i].UID)) {
t.Errorf("expected directory to exist for pod %d", i)
......@@ -3307,7 +3281,7 @@ func TestDoesNotDeletePodDirsForTerminatedPods(t *testing.T) {
// deleted.
kl.statusManager.SetPodStatus(pods[1], api.PodStatus{Phase: api.PodFailed})
kl.statusManager.SetPodStatus(pods[2], api.PodStatus{Phase: api.PodSucceeded})
err = kl.SyncPods(pods, emptyPodUIDs, map[string]*api.Pod{}, time.Now())
kl.HandlePodCleanups()
for i := range pods {
if !dirExists(kl.getPodDir(pods[i].UID)) {
t.Errorf("expected directory to exist for pod %d", i)
......
......@@ -19,7 +19,6 @@ package kubelet
import (
"sync"
"github.com/golang/glog"
"k8s.io/kubernetes/pkg/api"
client "k8s.io/kubernetes/pkg/client/unversioned"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
......@@ -45,9 +44,19 @@ type podManager interface {
GetPods() []*api.Pod
GetPodByFullName(podFullName string) (*api.Pod, bool)
GetPodByName(namespace, name string) (*api.Pod, bool)
GetPodsAndMirrorMap() ([]*api.Pod, map[string]*api.Pod)
GetPodByMirrorPod(*api.Pod) (*api.Pod, bool)
GetMirrorPodByPod(*api.Pod) (*api.Pod, bool)
GetPodsAndMirrorPods() ([]*api.Pod, []*api.Pod)
// SetPods replaces the internal pods with the new pods.
// It is currently only used for testing.
SetPods(pods []*api.Pod)
UpdatePods(u PodUpdate, podSyncTypes map[types.UID]SyncPodType)
// Methods that modify a single pod.
AddPod(pod *api.Pod)
UpdatePod(pod *api.Pod)
DeletePod(pod *api.Pod)
DeleteOrphanedMirrorPods()
TranslatePodUID(uid types.UID) types.UID
IsMirrorPodOf(mirrorPod, pod *api.Pod) bool
......@@ -103,50 +112,6 @@ func newBasicPodManager(apiserverClient client.Interface) *basicPodManager {
return pm
}
// Update the internal pods with those provided by the update.
func (pm *basicPodManager) UpdatePods(u PodUpdate, podSyncTypes map[types.UID]SyncPodType) {
pm.lock.Lock()
defer pm.lock.Unlock()
switch u.Op {
case SET:
glog.V(3).Infof("SET: Containers changed")
// Store the new pods. Don't worry about filtering host ports since those
// pods will never be looked up.
existingPods := make(map[types.UID]struct{})
for uid := range pm.podByUID {
existingPods[uid] = struct{}{}
}
// Update the internal pods.
pm.setPods(u.Pods)
for uid := range pm.podByUID {
if _, ok := existingPods[uid]; !ok {
podSyncTypes[uid] = SyncPodCreate
}
}
case UPDATE:
glog.V(3).Infof("Update: Containers changed")
// Store the updated pods. Don't worry about filtering host ports since those
// pods will never be looked up.
for i := range u.Pods {
podSyncTypes[u.Pods[i].UID] = SyncPodUpdate
}
allPods := applyUpdates(u.Pods, pm.getAllPods())
pm.setPods(allPods)
default:
panic("syncLoop does not support incremental changes")
}
// Mark all remaining pods as sync.
for uid := range pm.podByUID {
if _, ok := podSyncTypes[uid]; !ok {
podSyncTypes[uid] = SyncPodSync
}
}
}
// Set the internal pods based on the new pods.
func (pm *basicPodManager) SetPods(newPods []*api.Pod) {
pm.lock.Lock()
......@@ -177,24 +142,34 @@ func (pm *basicPodManager) setPods(newPods []*api.Pod) {
pm.mirrorPodByFullName = mirrorPodByFullName
}
func applyUpdates(changed []*api.Pod, current []*api.Pod) []*api.Pod {
updated := []*api.Pod{}
m := map[types.UID]*api.Pod{}
for _, pod := range changed {
m[pod.UID] = pod
}
func (pm *basicPodManager) AddPod(pod *api.Pod) {
pm.UpdatePod(pod)
}
for _, pod := range current {
if m[pod.UID] != nil {
updated = append(updated, m[pod.UID])
glog.V(4).Infof("pod with UID: %q has a new spec %+v", pod.UID, *m[pod.UID])
} else {
updated = append(updated, pod)
glog.V(4).Infof("pod with UID: %q stay with the same spec %+v", pod.UID, *pod)
}
func (pm *basicPodManager) UpdatePod(pod *api.Pod) {
pm.lock.Lock()
defer pm.lock.Unlock()
podFullName := kubecontainer.GetPodFullName(pod)
if isMirrorPod(pod) {
pm.mirrorPodByUID[pod.UID] = pod
pm.mirrorPodByFullName[podFullName] = pod
} else {
pm.podByUID[pod.UID] = pod
pm.podByFullName[podFullName] = pod
}
}
return updated
func (pm *basicPodManager) DeletePod(pod *api.Pod) {
pm.lock.Lock()
defer pm.lock.Unlock()
podFullName := kubecontainer.GetPodFullName(pod)
if isMirrorPod(pod) {
delete(pm.mirrorPodByUID, pod.UID)
delete(pm.mirrorPodByFullName, podFullName)
} else {
delete(pm.podByUID, pod.UID)
delete(pm.podByFullName, podFullName)
}
}
// GetPods returns the regular pods bound to the kubelet and their spec.
......@@ -204,23 +179,20 @@ func (pm *basicPodManager) GetPods() []*api.Pod {
return podsMapToPods(pm.podByUID)
}
// GetPodsAndMirrorPods returns the both regular and mirror pods.
func (pm *basicPodManager) GetPodsAndMirrorPods() ([]*api.Pod, []*api.Pod) {
pm.lock.RLock()
defer pm.lock.RUnlock()
pods := podsMapToPods(pm.podByUID)
mirrorPods := podsMapToPods(pm.mirrorPodByUID)
return pods, mirrorPods
}
// Returns all pods (including mirror pods).
func (pm *basicPodManager) getAllPods() []*api.Pod {
return append(podsMapToPods(pm.podByUID), podsMapToPods(pm.mirrorPodByUID)...)
}
// GetPodsAndMirrorMap returns the a copy of the regular pods and the mirror
// pods indexed by full name.
func (pm *basicPodManager) GetPodsAndMirrorMap() ([]*api.Pod, map[string]*api.Pod) {
pm.lock.RLock()
defer pm.lock.RUnlock()
mirrorPods := make(map[string]*api.Pod)
for key, pod := range pm.mirrorPodByFullName {
mirrorPods[key] = pod
}
return podsMapToPods(pm.podByUID), mirrorPods
}
// GetPodByName provides the (non-mirror) pod that matches namespace and name,
// as well as whether the pod was found.
func (pm *basicPodManager) GetPodByName(namespace, name string) (*api.Pod, bool) {
......@@ -295,3 +267,17 @@ func podsMapToPods(UIDMap map[types.UID]*api.Pod) []*api.Pod {
}
return pods
}
func (pm *basicPodManager) GetMirrorPodByPod(pod *api.Pod) (*api.Pod, bool) {
pm.lock.RLock()
defer pm.lock.RUnlock()
mirrorPod, ok := pm.mirrorPodByFullName[kubecontainer.GetPodFullName(pod)]
return mirrorPod, ok
}
func (pm *basicPodManager) GetPodByMirrorPod(mirrorPod *api.Pod) (*api.Pod, bool) {
pm.lock.RLock()
defer pm.lock.RUnlock()
pod, ok := pm.podByFullName[kubecontainer.GetPodFullName(mirrorPod)]
return pod, ok
}
......@@ -32,6 +32,7 @@ import (
type PodWorkers interface {
UpdatePod(pod *api.Pod, mirrorPod *api.Pod, updateComplete func())
ForgetNonExistingPodWorkers(desiredPods map[types.UID]empty)
ForgetWorker(uid types.UID)
}
type syncPodFnType func(*api.Pod, *api.Pod, kubecontainer.Pod, SyncPodType) error
......@@ -171,19 +172,30 @@ func (p *podWorkers) UpdatePod(pod *api.Pod, mirrorPod *api.Pod, updateComplete
}
}
func (p *podWorkers) removeWorker(uid types.UID) {
if ch, ok := p.podUpdates[uid]; ok {
close(ch)
delete(p.podUpdates, uid)
// If there is an undelivered work update for this pod we need to remove it
// since per-pod goroutine won't be able to put it to the already closed
// channel when it finish processing the current work update.
if _, cached := p.lastUndeliveredWorkUpdate[uid]; cached {
delete(p.lastUndeliveredWorkUpdate, uid)
}
}
}
func (p *podWorkers) ForgetWorker(uid types.UID) {
p.podLock.Lock()
defer p.podLock.Unlock()
p.removeWorker(uid)
}
func (p *podWorkers) ForgetNonExistingPodWorkers(desiredPods map[types.UID]empty) {
p.podLock.Lock()
defer p.podLock.Unlock()
for key, channel := range p.podUpdates {
for key := range p.podUpdates {
if _, exists := desiredPods[key]; !exists {
close(channel)
delete(p.podUpdates, key)
// If there is an undelivered work update for this pod we need to remove it
// since per-pod goroutine won't be able to put it to the already closed
// channel when it finish processing the current work update.
if _, cached := p.lastUndeliveredWorkUpdate[key]; cached {
delete(p.lastUndeliveredWorkUpdate, key)
}
p.removeWorker(key)
}
}
}
......
......@@ -53,10 +53,15 @@ func (kl *Kubelet) RunOnce(updates <-chan PodUpdate) ([]RunPodResult, error) {
// runOnce runs a given set of pods and returns their status.
func (kl *Kubelet) runOnce(pods []*api.Pod, retryDelay time.Duration) (results []RunPodResult, err error) {
kl.handleNotFittingPods(pods)
ch := make(chan RunPodResult)
admitted := []*api.Pod{}
for _, pod := range pods {
// Check if we can admit the pod.
if ok, reason, message := kl.canAdmitPod(append(admitted, pod), pod); !ok {
kl.rejectPod(pod, reason, message)
} else {
admitted = append(admitted, pod)
}
go func(pod *api.Pod) {
err := kl.runPod(pod, retryDelay)
ch <- RunPodResult{pod, err}
......
......@@ -76,6 +76,7 @@ func TestRunOnce(t *testing.T) {
cadvisor.On("MachineInfo").Return(&cadvisorApi.MachineInfo{}, nil)
podManager, _ := newFakePodManager()
diskSpaceManager, _ := newDiskSpaceManager(cadvisor, DiskSpacePolicy{})
kb := &Kubelet{
rootDirectory: "/tmp/kubelet",
......@@ -88,6 +89,7 @@ func TestRunOnce(t *testing.T) {
podManager: podManager,
os: kubecontainer.FakeOS{},
volumeManager: newVolumeManager(),
diskSpaceManager: diskSpaceManager,
}
kb.containerManager, _ = newContainerManager(cadvisor, "", "", "")
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment