Revert "cmd/openshift-install/create: Allow hung watch"

[wking]

This reverts commit 6dc1bf6, #615.

109531f (#606) made the watch re-connects reliable, so make watch timeouts fatal again. This avoids confusing users by showing "Install complete!" messages when they may actually have a hung bootstrap process.

[abhinavdahiya]

to compare it to how the error would have been structured without wrap
fmt.Errorf("failed waiting for bootstrap-complete: %v", err)

I would suggest we keep it errors.Wrap(err, "failed waiting for bootstrap-complete")

[wking]

$ git describe --dirty
v0.4.0-25-gb13b7fa-dirty
$ git diff
diff --git a/cmd/openshift-install/create.go b/cmd/openshift-install/create.go
index 5ba6517..8c95b8c 100644
--- a/cmd/openshift-install/create.go
+++ b/cmd/openshift-install/create.go
@@ -203,8 +203,8 @@ func destroyBootstrap(ctx context.Context, directory string) (err error) {
 				watcher, err := events.Watch(metav1.ListOptions{
 					ResourceVersion: sinceResourceVersion,
 				})
-				if err == nil {
-					return watcher, nil
+				if err == nil || true {
+					return watcher, err
 				}
 				select {
 				case <-eventContext.Done():
$ TAGS=libvirt_destroy hack/build.sh
$ openshift-install --dir=wking create cluster
INFO Fetching OS image...
INFO Using Terraform to create cluster...
INFO Waiting for bootstrap completion...
INFO API v1.11.0+d4cacc0 up
WARNING RetryWatcher - getting event failed! Re-creating the watcher. Last RV: 2214 
FATAL Error executing openshift-install: waiting for bootstrap-complete: watch closed before UntilWithoutRetry timeout 

That line includes both "FATAL" and "Error" already. Personally I think that's enough without also including an additional "failed", but I can add it if you like.

[abhinavdahiya]

as error returns from this function first one is more clear when looking at the function return (not few levels up).

[wking]

VPC exhaustion:

level=error msg="Error: Error applying plan:\n\n1 error(s) occurred:\n\n* module.vpc.aws_vpc.new_vpc: 1 error(s) occurred:\n\n* aws_vpc.new_vpc: Error creating VPC: VpcLimitExceeded: The maximum number of VPCs has been reached.\n\tstatus code: 400, request id: d1fea395

I've since cleaned up the leaked CI VPCs, and #740 is in flight to address the current leak.

/retest

[wking]

e2e-aws:

level=error msg="Error: Error applying plan:\n\n1 error(s) occurred:\n\n* aws_route53_record.etcd_a_nodes[2]: 1 error(s) occurred:\n\n* aws_route53_record.etcd_a_nodes.2: timeout while waiting for state to become 'INSYNC' (timeout: 30m0s)\n\nTerraform does not automatically rollback in the face of errors.\nInstead, your Terraform state file has been partially updated with\nany resources that successfully completed. Please address the error\nabove and apply again to incrementally change your infrastructure."
level=fatal msg="Error executing openshift-install: failed to fetch Cluster: failed to generate asset \"Cluster\": failed to run terraform: failed to execute Terraform: exit status 1"

I'll wait until CI settles down and kick this again tonight.

[abhinavdahiya]

don't care enough about #741 (comment) (just preferred the recommendation)

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [abhinavdahiya,wking]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.