[WIP] e2e: wait for cluster to be ready #1546
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
karlkfi
requested review from
sdowell
and removed request for
tiffanny29631 and
Camila-B
| @@ -139,6 +144,9 @@ func upsertCluster(t testing.NTB, cluster Cluster, opts *ntopts.New) (bool, erro | |||
| if err := cluster.Create(); err != nil { | |||
| return true, err | |||
| } | |||
| if err := cluster.WaitForReady(); err != nil { | |||
There was a problem hiding this comment.
Is there evidence that suggests this is needed? We currently wait for all operations on the cluster to complete during the Create call above
There was a problem hiding this comment.
Just ran the tests locally on GKE and ran into the same issue where this doesn't wait for the cluster to be ready. This wasn't an issue before, perhaps there was a change to the GKE operations during cluster creation.
| @@ -260,6 +265,33 @@ func createGKECluster(t testing.NTB, name string) error { | |||
| return listAndWaitForOperations(context.Background(), t, name) | |||
There was a problem hiding this comment.
The listAndWaitForOperations call at the end of createGKECluster and getGKECredentials is currently intended to serve as a readiness check. I'd suggest either:
- Instead of surfacing a new
WaitForReadyinterface method, replace thelistAndWaitForOperationscalls withwaitForReadyGKECluster, assuminglistAndWaitForOperationsis insufficient to determine readiness. - Keep the new
WaitForReadyinterface method but remove the existing readiness checks built in tocreateGKECluster/getGKECredentials
No description provided.