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.

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

Bug 1508724 - Improve logging when using an incorrect tag #536

Merged
merged 1 commit into from
Nov 6, 2017

Conversation

rthallisey
Copy link
Contributor

@rthallisey rthallisey commented Nov 6, 2017

Describe what this PR does and why we need it:
If you set an incorrect tag for your images in the broker config, the logging
does not indicate what the error is. Instead, make a suggestion
about the error and print out the request body.

Changes proposed in this pull request

  • Improve logging when an incorrect tag is used

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 6, 2017
if err != nil {
log.Errorf("Image '%s' may not exist in registry: %s", image, debug)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using the registry name here, rather then the response body, might make more sense and then you can remove the copy of the byte array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error check really is catching two error scenarios: A json decoding error and a request error. I'm going to change this to two different checks by looking for a bad http status code.

I thought about that adding in the registry, but it's embedded in the image variable: http://<registry>/<image>/<tag>.

If a registry is missing the image with the specified tag, the logging
does not indicate what the error could be.  Instead, make a suggestion
about the error and print out the request body.
@rthallisey
Copy link
Contributor Author

rthallisey commented Nov 6, 2017

The poor logging was identified in: https://bugzilla.redhat.com/show_bug.cgi?id=1508724
Do we want an addition bz for this?

@shawn-hurley
Copy link
Contributor

I just re-use that BZ, I think this is something worth merging IMO

@rthallisey rthallisey changed the title Improve logging when using an incorrect tag bz 1508724 - Improve logging when using an incorrect tag Nov 6, 2017
@shawn-hurley shawn-hurley changed the title bz 1508724 - Improve logging when using an incorrect tag Bug 1508724 - Improve logging when using an incorrect tag Nov 6, 2017
Copy link
Contributor

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK makes sense.

@rthallisey rthallisey merged commit 82c91df into openshift:master Nov 6, 2017
jianzhangbjz pushed a commit to jianzhangbjz/ansible-service-broker that referenced this pull request May 17, 2018
If a registry is missing the image with the specified tag, the logging
does not indicate what the error could be.  Instead, make a suggestion
about the error and print out the request body.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants