Can you add a one-sentence explanation of how this addresses #1954 for people who don't want to read the whole PR? (Including what a "source" means in this context.) Thanks!

Sure:

The kubelet receives configuration information from a variety of sources:

• static file
• http server
• etcd/boundPods

There are three types of updates:

• ADD Incremental update adding pod(s) to the complete set of pods for this source
• SET complete overwrite of pods for that source
• DELETE Incremental delete of pod(s) from the complete set of pods for this source

These are all mixed together into a single update stream.

The kubelet itself does the following loop:

for {
  select {
     // normal watch/update case
     case update := <- updates:
        handleUpdate(update)
     // periodic resync, in case updates are missed
     case <- time.Timeout(syncTimeout)
        handleResync()
}

The trouble occurs when the resync timeout occurs before complete information from all sources have been obtained. The set of active pods on the kubelet is incomplete, and the resync uses this incomplete information, and thus kills pods that are correctly scheduled on the host, but the kubelet is not currently aware of.

This PR fixes this, by refusing to delete pods until all sources have sent at least one SET message.

Hope that helps!

Ok, unit test added. Empirically verified also.
This is ready to merge.
ptal.

Thanks!
--brendan

config is just one of sources through which kubelet receives information. We shouldn't have this SeenAllSources() implemented in config.go to decided if all sources are received for kubelet. This is part of kubelet control loop, why not implement it in Kubelet itself? I understand that you want to have the dependency injection here, but we still can have that at kubelet.go since it already have the factory-like methods for prod and test anyway.

Actually, the trouble is that the config object abstracts across all of the
different sources, so the kubelet has no visibility about the various
sources.

Also, the config object is actually the only source of bound pod
information.

We could break the config abstraction and pull the sources all the way up
into the kubelet, but that's a large scale re-factor.

--brendan

On Wed, Dec 17, 2014 at 3:25 PM, Dawn Chen notifications@github.com wrote:

config is just one of sources through which kubelet receives information.
We shouldn't have this SeenAllSources() implemented in config.go to decided
if all sources are received for kubelet. This is part of kubelet control
loop, why not implement it in Kubelet itself? I understand that you want to
have the dependency injection here, but we still can have that at
kubelet.go since it already have the factory-like methods for prod and test
anyway.

—
Reply to this email directly or view it on GitHub
#2994 (comment)
.

ACK and LGTM.