-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
print master config error if it exists #12832
print master config error if it exists #12832
Conversation
[test] |
e1971c5
to
7fa03f5
Compare
pkg/diagnostics/host/util.go
Outdated
} else { | ||
r.Debug("DH0003", fmt.Sprintf("Found a master config file: %[1]s", masterConfigFile)) | ||
} | ||
return masterConfig, masterConfigLoadError | ||
} | ||
|
||
func printMasterConfigLoadError(r types.DiagnosticResult, masterConfigFile string) { | ||
if masterConfigLoadError != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not opposed to having a helper method if there's good reason. Though I'd recommend just printing the error and assume the caller has handled the error checking.
@jcantrill, Would you mind taking a look at this PR? |
LGTM. You may wish to alter your commit message per the guidelines here: https://mojo.redhat.com/docs/DOC-1093603 |
print master config error if it exists
7fa03f5
to
72c6f26
Compare
@brenton @jcantrill |
Evaluated for origin test up to 72c6f26 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13659/) (Base Commit: 861c5fb) |
@jcantrill ok to merge this now? |
LGTM [merge] |
Evaluated for origin merge up to 72c6f26 |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13659/) (Base Commit: 863f435) (Image: devenv-rhel7_5882) |
Related bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1419472
Although a MasterConfig load error is stored globally, it is only printed the first time that it is encountered during a diagnostics check.
This patch ensures that, even if the error has already been encountered once, its message gets printed in subsequent diagnostic checks.
@openshift/cli-review @sosiouxme