aii-ks: fix logging of pre and post script actions for EL7 #218
Conversation
| @@ -692,13 +692,12 @@ sub log_action { | |||
| } | |||
|
|
|||
| if ($consolelogging) { | |||
| push(@logactions, '# Make sure messages show up on the serial console', | |||
| "tail -f $logfile > /dev/console &"); | |||
| push(@logactions, "tee $logfile", "tee /dev/pts/0"); | |||
There was a problem hiding this comment.
i guess tail -f $logfile > /dev/pts/0 doesn't do the trick?
There was a problem hiding this comment.
Good suggestion! I didn't try, I will do it. Clearly if at the end this is just the matter of changin console to pts/0, it would be an easy change...
| return join("\n", @logactions) | ||
| # push(@logactions,"drainsleep=$drainsleep"); # add trailing newline | ||
| # push(@logactions,''); # add trailing newline | ||
| return join(" | ", @logactions) |
There was a problem hiding this comment.
the remote logging feature generates after exec
(tail -f /tmp/pre-log.log | awk '{print "<190>AII: "$0; fflush(); system("usleep 1000 >& /dev/null");}' | nc -u gastly.gastly.os 515) &
i'm not sure how to replace this more pipes
jouvin
force-pushed
the
pre_post_formatting
branch
from
e1de078
to
b355371
Compare
|
@stdweird you're right! The problem is that |
|
One of the unit test checking for the present of |
jouvin
force-pushed
the
pre_post_formatting
branch
from
b355371
to
af96c01
Compare
jouvin
force-pushed
the
pre_post_formatting
branch
from
af96c01
to
b958ce3
Compare
| # resulting in unreadable messages on the console. /dev/pts/0 must be used instead. | ||
| # But on prior versions, /dev/pts/0 doesn't exist at installation time and cannot be used. | ||
| # The following code allows to use /dev/pts/0 if it exists else to revert to /dev/console. | ||
| push(@logactions, "console='/dev/console'"); |
There was a problem hiding this comment.
there is no harm in always setting these variables early on, even if no consolelogging is used. this will allow a cleaner if/else construct, because this is a bit ugly.
There was a problem hiding this comment.
I am not sure to understand your point. Why it would be clearer to have it earlier? This variable definition in the %pre script is useful only if you use console logging, there is no real point in adding them otherwise. I would find it more confusing to have this defined unconditionally, both in the ks.pm code and the resulting %pre script...
There was a problem hiding this comment.
it's a bit hackish. but i saw that i did similar hack for the wait time, so i'm not going to complain any further 😄
There was a problem hiding this comment.
😄 In fact this is not that much a hack... These pre/post scripts are built using generated code and the resulting code is quite ok...
jouvin
force-pushed
the
pre_post_formatting
branch
from
583e06e
to
0e0c590
Compare
|
I don't really understand why the unit test BTW, all these unit tests seem very fragile... On my SL6 machine, many (16) subtests of |
jouvin
force-pushed
the
pre_post_formatting
branch
from
0e0c590
to
cb31efe
Compare
|
I managed to get the unit test 'kickstart_pre I am not sure what was wrong in it but I am not completely convinced of the value of testing each line this way... |
|
the lines are tested to make sure they are not deleted/modified unintentionally. the test fails because you have |
|
inparticular, use quattor/quattor.github.com#146 (comment) to run the tests. |
jouvin
force-pushed
the
pre_post_formatting
branch
from
cb31efe
to
6f316e3
Compare
|
@stdweird I must really apologize for this |
|
For the record, the problems with |
|
@jouvin if you have tested this, can you remove the |
jouvin
changed the title
|
LGTM |
Fixes #120