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

Jump to bottom

asset/cluster: clean up logging messages #840

Merged
merged 1 commit into from
Dec 11, 2018
Merged

asset/cluster: clean up logging messages #840

merged 1 commit into from
Dec 11, 2018

Conversation

crawford
Copy link
Contributor

@crawford crawford commented Dec 7, 2018

This removes explicit mention of Terraform in the log messages. We
received feedback that this was confusing users and product requested
that we remove it.

@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 7, 2018
@abhinavdahiya
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2018
@abhinavdahiya
Copy link
Contributor

/retest

@wking
Copy link
Member

wking commented Dec 7, 2018 •
edited
Loading

e2e-aws:

level=fatal msg="Error executing openshift-install: failed to fetch Terraform Variables: failed to fetch dependency of \"Terraform Variables\": failed to fetch dependency of \"Bootstrap Ignition Config\": failed to fetch dependency of \"Openshift Manifests\": failed to generate asset \"Master Machines\": failed to fetch availability zones: cannot fetch availability zones: RequestLimitExceeded: Request limit exceeded.\n\tstatus code: 503, request id: 94133334-f70a-428b-b93b-39c3942424b9"

I guess I won't retest...

@wking
Copy link
Member

wking commented Dec 8, 2018

We've got past that rate limit a few times now, maybe whatever load has blown over.

/retest

@wking
Copy link
Member

wking commented Dec 8, 2018

Router never came up.

/retest

@crawford
Copy link
Contributor Author

crawford commented Dec 8, 2018

/retest

@crawford crawford mentioned this pull request Dec 8, 2018
Merged
@crawford
Copy link
Contributor Author

crawford commented Dec 8, 2018

Looks like the storage operator is crashlooping.

@crawford
Copy link
Contributor Author

crawford commented Dec 8, 2018

/retest

@crawford
Copy link
Contributor Author

crawford commented Dec 9, 2018

Router didn't come up.

/retest

@crawford
Copy link
Contributor Author

crawford commented Dec 9, 2018

Still no router......

/retest

@crawford
Copy link
Contributor Author

crawford commented Dec 9, 2018

Router came up. Bunch of network failures though.

/retest

@crawford
Copy link
Contributor Author

crawford commented Dec 9, 2018

Storage operator failing again. I give up for now.

@wking
Copy link
Member

wking commented Dec 10, 2018

/retest

@wking wking mentioned this pull request Dec 10, 2018
Merged
@crawford
Copy link
Contributor Author

/retest

@wking wking mentioned this pull request Dec 10, 2018
Merged
@crawford
Copy link
Contributor Author

I see a whole bunch of "no pods to fetch metrics for" lines (which I wish would read "no pods for which to fetch metrics").

/retest

@abhinavdahiya
Copy link
Contributor

/retest

@crawford
asset/cluster: clean up logging messages
be211a4 
This removes explicit mention of Terraform in the log messages. We
received feedback that this was confusing users and product requested
that we remove it.
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 10, 2018
@abhinavdahiya
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 10, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [abhinavdahiya,crawford]

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

wking added a commit to wking/openshift-installer that referenced this pull request Dec 10, 2018
@wking
hack/get-terraform: Remove obsolete script
aff2e98 
Now that we're vendoring Terraform, we no longer need a script to pull
down pre-compiled binaries.

I've also reworded the 'create cluster' description in overview.md to
be more generic.  See 0c3d8658 (asset/cluster: clean up logging
messages, 2018-12-07, openshift#840) for similar changes to another location.
@crawford
Copy link
Contributor Author

This has been a wild ride.

/retest

@openshift-merge-robot openshift-merge-robot merged commit 01fbeae into openshift:master Dec 11, 2018
@crawford crawford deleted the logs branch December 13, 2018 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@staebler staebler Awaiting requested review from staebler

@tomassedovic tomassedovic Awaiting requested review from tomassedovic

Assignees

@abhinavdahiya abhinavdahiya

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@crawford @abhinavdahiya @wking @openshift-ci-robot @openshift-merge-robot