Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubelet: send all recevied pods in one update #23141

Merged
merged 1 commit into from
Mar 18, 2016
Merged

kubelet: send all recevied pods in one update #23141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions pkg/kubelet/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,16 +238,7 @@ func (s *podStorage) merge(source string, change interface{}) (adds, updates, de
}
recordFirstSeenTime(ref)
pods[name] = ref
// If a pod is not found in the cache, and it's also not in the
// pending phase, it implies that kubelet may have restarted.
// Treat this pod as update so that kubelet wouldn't reject the
// pod in the admission process.
if ref.Status.Phase != api.PodPending {
updatePods = append(updatePods, ref)
} else {
// this is an add
addPods = append(addPods, ref)
}
addPods = append(addPods, ref)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yujuhong nit: maybe add a TODO or file an issue about the original issue of pod rejection?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on your old comment, with this change, some existing pods might be rejected by admission process if there is over-commitment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I saw your comment below. But I am not 100% convinced the change is safe here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dchen1107 FYI, This part of code is introduced in #18546

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dchen1107 In fact, the original pod rejection bug was there for a while. Yuju tried to fix it with #18546, but introduced one of the issues in #23104. This PR just recover the old behavior.

If you think we should still fix the old bug, maybe we could try some other ways like: #18546 (comment)

}
}

Expand Down
3 changes: 0 additions & 3 deletions pkg/kubelet/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ func CreateValidPod(name, namespace string) *api.Pod {
},
},
},
Status: api.PodStatus{
Phase: api.PodPending,
},
}
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/kubelet/kubelet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2386,6 +2386,10 @@ func (kl *Kubelet) syncLoopIteration(updates <-chan kubetypes.PodUpdate, handler
switch u.Op {
case kubetypes.ADD:
glog.V(2).Infof("SyncLoop (ADD, %q): %q", u.Source, format.Pods(u.Pods))
// After restarting, kubelet will get all existing pods through
// ADD as if they are new pods. These pods will then go through the
// admission process and *may* be rejcted. This can be resolved
// once we have checkpointing.
handler.HandlePodAdditions(u.Pods)
case kubetypes.UPDATE:
glog.V(2).Infof("SyncLoop (UPDATE, %q): %q", u.Source, format.Pods(u.Pods))
Expand Down