Bug 1005951

I can't reproduce this with current master on OS/X

I think for partial matches you would want to surround the expression by
.* pairs

.* will not match new lines and therefore will fail when using multi-line output. I am not sure why it seems to have worked for you in OS X. Perhaps the new line wasn't added to the output or perhaps the JVM you are using is matching on \r when it shouldn't?

Found out that the code on master changed due to Bug 848258, which removes the newlines, which explains I failed to reproduce the original issue from the description.

I now have my /tmp/version.conf as


snert:~ hrupp$ cat /tmp/version.conf
JBoss Admin Tool : Version 0.6
Slot=bla
snert:~ hrupp$

and an expression of 

^JBoss Admin Tool : Version 0\.6\nSlot=bla$

and this shows green. When I change the 'S' in Slot to lowercase in the expression, the resource immediately goes DOWN. If I change the expression back or adjust the file content accordingly, the resource goes UP immediately (when standing on it and the new availability auto-refresh kicks in.

The expression you are using is bad. You can not put \n in the expression because:
 a. the user specifying a single line boundary using ^ and $
 b. the script output should not be platform dependent

Instead, the correct solution here is to use a real expression evaluation the same as we do for the other components of this plug-in. We should not use the single-line mode provided by "matches" and instead use "find".

However, that being said, the expression you are using fails for me. However, I am using 3.1.2.

Is there a reason we should not be treating the output the same as any search command such as what is used in grep, egrep, awk, sed, find, etc?

If we are not wanting to support regular expressions and do simple text matching then we should probably change the description and name of this property to reflect. 

The different here is that when using .matches the output MUST MATCH the expression. Where as .find means that the expression MUST EXIST in the output.

I followed the replication procedure in the description while running the latest master build and it works OK - resource goes green/UP.

I will try on the latest 3.2 GA build and see what different behavior I see.

Created attachment 836014 [details]
screen snapshot

Unable to reproduce on the 3.2 GA build. See attached screen shot showing the about box for the version info as was as, in the background, my settings used in the Script Server resource.

You were not able to reproduce the original issue due to a change you made for bug 848258 as mentioned in comment 3 and comment 4 above.

After reviewing https://git.fedorahosted.org/cgit/rhq/rhq.git/commit/?id=49fe82e497cd3fb258fa29aca8bfebe7c480b14e, seems like that is a bad thing to do. As white space can have actual meaning in the real world, trimming it could result in failed matches. If white space shouldn't be used in the regex then the regex should specify that. We should not be changing the output. Which is the point of this BZ. As indicated in the upstream bug, option 4 

Due to the (bad) change introduced by bug 848258, to reproduce this issue now, for step 3 above, change:

echo "JBoss Admin Tool : Version 0.6"> /tmp/version.conf

To:

cat >/tmp/version.conf <<EOF
JBoss Admin Tool : Version 0.6
A tool that does tool like stuff.
EOF

I have potential fix for this issue. I changed availabilityRegex behaviour to used Matcher.find instead of matches as suggested.

Regex `^JBoss Admin Tool : Version 0\.6$` does not match 

cat >/tmp/version.conf <<EOF
JBoss Admin Tool : Version 0.6
A tool that does tool like stuff.
EOF

because ^ and $ in this case means start/end of whole string, not just line. If we want to force multi-line we have to 

either change regex `(?m)^JBoss Admin Tool : Version 0\.6$`
or enable MULTILINE flag in pattern handling.

To get closer to grep-like behaviour we should enable MULTILINE flag by default.

From the original use-case MULTILINE is what was expected. However, would there be a way to disable multiline if it was defined on the pattern? Perhaps single-line as the default is acceptable as the user could simply use (?m) to enable multiline. This option was previously available due to the method being used.

Another way to look at this is from a legacy perspective. By switching to the find method with no options, would existing regular expressions continue to work as they had been?

I'd have to add another property to enable/disable mutliline flag - there is no point in it, since it can be expressed within regex. Single-line default is better, because you can still write `JBoss Admin Tool : Version 0\.6`.

Existing expressions could work. Matches is just a special case of find, so it should not happen, that after upgrade script server resources would become DOWN. But there can be difference with detecting DOWN state. 

For example, if a script outputs : "OK" to indicate availability=UP and "ERROR...OK" to indicate DOWN, user could have regex `OK` to detect UP state. After changing to Pattern.find() we'll report resource as UP

To keep compatibility, we can introduce Matches/find control property, unset by default (that would mean Matches). Or (if possible) fix regexes during update (I guess adding ^ before and $ after would do the job).

IMHO we don't need to care, because those cases would be very rare.

Hm. Not sure I follow what you are saying. Let me try though:

> I'd have to add another property to enable/disable mutliline flag - there is
> no point in it, since it can be expressed within regex. Single-line default
> is better, because you can still write `JBoss Admin Tool : Version 0\.6`.

I am not advocating an actual configurable property. My point was that if I could not override the behavior to switch multiline to single-line using regex itself, then perhaps single-line mode makes more sense. This was it is backward compatible and if a user wants to use ^...$ or full-regex for processing script output, then they could just use (?m).

> For example, if a script outputs : "OK" to indicate availability=UP and
> "ERROR...OK" to indicate DOWN, user could have regex `OK` to detect UP
> state. After changing to Pattern.find() we'll report resource as UP

In the existing code, both ERROR...OK and OK would result in UP. That would be bad if the user really wanted OK to mean up. Which is what raised this BZ. I would expect to use ^OK$ (or (?m)^OK$) to mean UP while everything else would mean DOWN.

> To keep compatibility, we can introduce Matches/find control property, unset
> by default (that would mean Matches). Or (if possible) fix regexes during
> update (I guess adding ^ before and $ after would do the job).
> 
> IMHO we don't need to care, because those cases would be very rare.

My point was that if we just leave single-line mode (the default) and switch to using find, we would continue to work with legacy regex, right? But users could also do what they would normally need to do and parse multiline output by using (?m) at the beginning of their regular expression.

in master

commit f5644d991c63931ad16e45515c4861114f0f814f
Author: Libor Zoubek <lzoubek>
Date:   Wed Feb 12 14:15:14 2014 +0100

in master

commit 62cc988c8980a15e00b61fc45370f76c10a3608f
Author: Libor Zoubek <lzoubek>
Date:   Wed Feb 19 12:41:19 2014 +0100

    [BZ 1005951] ScriptServer resource availability regular expression matching
    fails due to complete string matching being applied to multiline output

    Improved description for availabilityOutputRegex config property.

release/jon3.2.x commit 26b3ff9c1d544267405804bc4cd72d762a8c0586
Author: Jay Shaughnessy <jshaughn>
Date:   Thu Feb 27 09:21:04 2014 -0500

    Cherry-Pick master f5644d991c63931ad16e45515c4861114f0f814f
    Signed-off-by: Jay Shaughnessy <jshaughn>


release/jon3.2.x commit 27d664d1c294822a0162711f73decbf03b1ea73a
Author: Jay Shaughnessy <jshaughn>
Date:   Thu Feb 27 09:24:25 2014 -0500

    Cherry-Pick master 62cc988c8980a15e00b61fc45370f76c10a3608f
    Signed-off-by: Jay Shaughnessy <jshaughn>

Moving to ON_QA as available for testing in the following brew build:
https://brewweb.devel.redhat.com//buildinfo?buildID=340294

Note: the installed version is still JON 3.2.0.GA by design and this represents part of the payload for JON 3.2.1 also known as cumulative patch 1 for 3.2.0.GA.  How this will be delivered to customers is still being discussed.

Moving to VERIFIED. I tried both combinations with (?m), (?s) for specifying one-line / multi-line output and all worked as expected.

Tested on JON 3.2.1 DR02 (Build Number: d18651a:f535707)

JON 3.2.1 released week of 5/5/2014