Bug 1005951 - ScriptServer resource availability regular expression matching fails due to complete string matching being applied to multiline output
ScriptServer resource availability regular expression matching fails due to c...
Status: CLOSED CURRENTRELEASE
Product: JBoss Operations Network
Classification: JBoss
Component: Plugin -- Other (Show other bugs)
JON 3.1.2
Unspecified Unspecified
unspecified Severity high
: DR02
: JON 3.2.1
Assigned To: Libor Zoubek
Mike Foley
: NeedsTestCase
Depends On: 848258
Blocks:
  Show dependency treegraph
 
Reported: 2013-09-09 14:14 EDT by Larry O'Leary
Modified: 2015-11-01 19:43 EST (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-05-08 13:44:19 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
screen snapshot (114.86 KB, image/jpeg)
2013-12-12 14:59 EST, John Mazzitelli
no flags Details


External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Knowledge Base (Solution) 470093 None None None Never

  None (edit)
Description Larry O'Leary 2013-09-09 14:14:04 EDT
Description of problem:
The script server resource is supposed to support the use of regular expressions for script output to match on availability checks. However, the regular expression does not work unless it matches the entire output of the script or unless the script's output is single-line.

Version-Release number of selected component (if applicable):
4.4.0.JON312GA

How reproducible:
Always

Steps to Reproduce:
1.  Start JBoss ON system.
2.  Import Linux platform resource into inventory.
3.  Create the version.conf file which will be used by the test script server:

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

4.  Create the new _script server_ resource:

    From the platform resource's _child resources inventory page_ select _Import > Script Server_
    
        *   _Executable_:   `/bin/cat`
        *   _Availability Exit Code Regex_: `0`
        *   _Availability Output Regex_: `^JBoss Admin Tool : Version 0\.6$`
        *   _Availability Arguments_: `/tmp/version.conf`

5.  Wait for an availability scan to occur.

Actual results:
Script is reported as DOWN.

Expected results:
Script should be reported as UP.

Additional info:
This is because the availability check is using the String / Matcher.matches method to perform the regular expression matching. The matches method performs the match on the entire string and it must therefore match the entire string. IN this case, that would be the entire output of the script rather then a sub-string of the output.

Instead, this method should use Matcher.find as described in the Java API and is already used for version and description matching in org.rhq.plugins.script.ScriptDiscoveryComponent.
Comment 1 Heiko W. Rupp 2013-09-17 04:28:13 EDT
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
Comment 2 Larry O'Leary 2013-09-17 09:42:48 EDT
.* 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?
Comment 3 Heiko W. Rupp 2013-09-17 10:55:09 EDT
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.
Comment 4 Heiko W. Rupp 2013-09-17 15:23:06 EDT
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.
Comment 5 Larry O'Leary 2013-09-17 16:01:13 EDT
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.
Comment 7 John Mazzitelli 2013-12-12 14:08:42 EST
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.
Comment 8 John Mazzitelli 2013-12-12 14:59:12 EST
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.
Comment 9 Larry O'Leary 2013-12-13 10:50:14 EST
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
Comment 10 Libor Zoubek 2014-02-11 08:08:56 EST
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.
Comment 11 Larry O'Leary 2014-02-11 14:27:06 EST
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?
Comment 12 Libor Zoubek 2014-02-11 18:21:42 EST
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.
Comment 13 Larry O'Leary 2014-02-11 19:22:23 EST
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.
Comment 14 Libor Zoubek 2014-02-12 08:38:00 EST
in master

commit f5644d991c63931ad16e45515c4861114f0f814f
Author: Libor Zoubek <lzoubek@redhat.com>
Date:   Wed Feb 12 14:15:14 2014 +0100
Comment 15 Libor Zoubek 2014-02-19 06:44:53 EST
in master

commit 62cc988c8980a15e00b61fc45370f76c10a3608f
Author: Libor Zoubek <lzoubek@redhat.com>
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.
Comment 16 Jay Shaughnessy 2014-02-27 09:27:01 EST
release/jon3.2.x commit 26b3ff9c1d544267405804bc4cd72d762a8c0586
Author: Jay Shaughnessy <jshaughn@redhat.com>
Date:   Thu Feb 27 09:21:04 2014 -0500

    Cherry-Pick master f5644d991c63931ad16e45515c4861114f0f814f
    Signed-off-by: Jay Shaughnessy <jshaughn@redhat.com>


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

    Cherry-Pick master 62cc988c8980a15e00b61fc45370f76c10a3608f
    Signed-off-by: Jay Shaughnessy <jshaughn@redhat.com>
Comment 17 Simeon Pinder 2014-03-05 17:21:41 EST
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.
Comment 18 Jan Bednarik 2014-03-10 08:56:44 EDT
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)
Comment 19 Mike Foley 2014-05-08 13:44:19 EDT
JON 3.2.1 released week of 5/5/2014

Note You need to log in before you can comment on or make changes to this bug.