Bug 1077664: oo-diagnostics: Fix test_node_mco_log when no log #4983
Conversation
Make test_node_mco_log fail gracefully when the node mcollective log file is absent. Before this commit, the test would hang because it would run a sed command with no input file provided. This commit fixes bug 1077664.
|
[test] |
|
Online Test Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/7527/) |
| logfile = "#{@scl_prefix}mcollective.log" | ||
| logfile = [ "/var/log/openshift/node/#{logfile}", "/var/log/#{logfile}" ].find {|f| File.exist? f} | ||
| if logfile.nil? || logfile.empty? | ||
| do_fail <<-"FAIL" |
There was a problem hiding this comment.
FAIL seems a little strong if it's just that the log file isn't where expected. And with the logging changes coming, this will probably be a common occurrence.
Is the non-existence of the log file actually indicative of a problem? Otherwise this would be a WARN at best, maybe just a "verbose".
|
Origin Test Results: FAILURE (https://originci-openshift.rhcloud.com/job/test_pull_requests/2280/) |
Place comments with relevant code, consistently use proper case and punctuation, and use whitespace for better logical grouping.
|
Updated to use [test] |
|
@sosiouxme, does it look good? |
|
@Miciah yes, looks good. Just one thought. Would it make more sense to FAIL when this file is missing? I can't think of a case when a working node would be missing this file. |
|
@sosiouxme, you mentioned before that the file might not exist if logs are going to syslog (and syslog is not configured to put them in that file). |
|
You are right. I am scatterbrained. I see no reason not to [merge] then. |
|
Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/merge_pull_requests/5423/) (Image: devenv_4787) |
|
Evaluated for online up to 6a917ca |
|
Evaluated for origin up to 6a917ca |
…node_mco_log-fixes Merged by openshift-bot
|
Spiffin'. Thanks! Looks like Origin tests failed because of ssh timeouts setting up the environment, unrelated to this PR, so I'm hoping merges are not gated by Origin tests at present. |
|
And right as I say that, it gets merged. All right then. |
oo-diagnostics: Fix test_node_mco_log when no log
Make test_node_mco_log fail gracefully when the node mcollective log file is absent. Before this commit, the test would hang because it would run a sed command with no input file provided.
This commit fixes bug 1077664.
oo-diagnostics: Fix comments for test_node_mco_log
Place comments with relevant code, consistently use proper case and punctuation, and use whitespace for better logical grouping.