Clients must wait for completion of actions #943
Conversation
smarterclayton
changed the title
| @@ -152,13 +152,11 @@ func runReplicationControllerTest(kubeClient *client.Client) { | |||
| glog.Infof("Done creating replication controllers") | |||
|
|
|||
| // Give the controllers some time to actually create the pods | |||
| time.Sleep(time.Second * 10) | |||
| if err := client.NewPoller(time.Second, 10).Until(kubeClient.ReplicationControllerAtDesiredState(controllerRequest)); err != nil { | |||
There was a problem hiding this comment.
nit: a bit much for a single line...
There was a problem hiding this comment.
Agreed - debating how complex this should be. Initial thought was to define the minimal likely methods that clients use, but it's still a bit verbose. Will iterate more
|
Integration test now waits for conditions, servers return immediately and clients return as well. |
smarterclayton
changed the title
|
@brendandburns as per the comments in the other issue - I believe this is consistent with the discussion in #353 but now's the time to disagree |
| @@ -135,7 +136,28 @@ func startComponents(manifestURL string) (apiServerURL string) { | |||
| return apiServer.URL | |||
| } | |||
|
|
|||
| func runReplicationControllerTest(kubeClient *client.Client) { | |||
| // podsOnMinions returns true when all of the selected pods exist on a minion. | |||
There was a problem hiding this comment.
Seems like this may not be useful without an expected number of minions?
There was a problem hiding this comment.
Good question - if the query changes to return zero it would exit. Could just pass the list of expected pods.
smarterclayton
force-pushed
the
only_wait_for_acceptance
branch
from
cc3da9b to
fe7ef0c
Compare
|
comments addressed, PTAL (left WaitFor for testability and added Poll for brevity) |
smarterclayton
force-pushed
the
only_wait_for_acceptance
branch
from
fe7ef0c to
681cbef
Compare
| @@ -128,6 +129,9 @@ func responseToPods(response *etcd.Response) ([]kubelet.Pod, error) { | |||
| // stopChannel creates a channel that is closed after a duration for use with etcd client API | |||
| func stopChannel(until time.Duration) chan bool { | |||
| stop := make(chan bool) | |||
| if until == 0 { | |||
There was a problem hiding this comment.
document this behavior in the comment?
|
LGTM, one small comment. |
Listen to etcd longer, and wait a shorter time before reconnecting. No longer an argument to the source.
Add client waiting conditions.
smarterclayton
force-pushed
the
only_wait_for_acceptance
branch
from
681cbef to
baaabca
Compare
|
addressed |
|
LGTM |
Clients must wait for completion of actions
Create some structure on sig cluster lifecycle KEPs
Makes the server return for pods and replication controllers when the object is accepted by the apiserver and persisted, rather than when all the consequences of its creation are relevant as per #353. This adds a very simple core "condition / wait" logic to the client that can be used to wait for things to happen. It does not change kubecfg to wait (and so the README was altered to communicate that waits might be necessary), and the replication controller now returns much more quickly in its loop (since it's not waiting for pods to go to running status).