Bug 1136751 - drift detection interval for individual drift definition is *always* ignored and value from agent config is used
Summary: drift detection interval for individual drift definition is *always* ignored ...
Status: CLOSED CURRENTRELEASE
Alias: None
Product: JBoss Operations Network
Classification: JBoss
Component: Drift
Version: JON 3.3.0
Hardware: Unspecified
OS: Unspecified
unspecified
urgent
Target Milestone: ER05
: JON 3.3.0
Assignee: Michael Burman
QA Contact: Armine Hovsepyan
URL:
Whiteboard:
Keywords:
Depends On:
Blocks: 1069646
TreeView+ depends on / blocked
 
Reported: 2014-09-03 08:44 UTC by Armine Hovsepyan
Modified: 2015-09-03 00:03 UTC (History)
6 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2014-12-11 14:03:35 UTC


Attachments (Terms of Use)
drift_detection_interval (588.82 KB, application/octet-stream)
2014-09-03 08:44 UTC, Armine Hovsepyan
no flags Details
drift_detection_interval.log (28.06 KB, text/plain)
2014-09-03 08:45 UTC, Armine Hovsepyan
no flags Details
drift_detection_interval_errorMsg (152.44 KB, image/png)
2014-10-08 18:49 UTC, Armine Hovsepyan
no flags Details
drift_detection.log (69.00 KB, text/plain)
2014-10-08 18:55 UTC, Armine Hovsepyan
no flags Details
3_drift_detections (316.01 KB, image/png)
2014-10-22 10:04 UTC, Armine Hovsepyan
no flags Details

Description Armine Hovsepyan 2014-09-03 08:44:06 UTC
Created attachment 933978 [details]
drift_detection_interval

Description of problem:
drift detection interval for individual drift definition is *always* ignored and value from agent config is used 

Version-Release number of selected component (if applicable):
jon 3.3 er01.1

How reproducible:
always

Steps to Reproduce:
1. create a drift definition with specified detection interval (e.g. 20 secs)
2. update agent config, change drift detection interval from default (60 secs) to 9 secs
3.

Actual results:
After step1. default detection interval from agent config is used (60 secs)
After step2. detection interval from agent config is used (9 secs)


Expected results:
Not sure: Either the smallest interval should be used, or the smallest from individual drift definitions should be used.

Additional info:
screen-shots and logs attached

Comment 1 Armine Hovsepyan 2014-09-03 08:45:40 UTC
Created attachment 933980 [details]
drift_detection_interval.log

Comment 2 Michael Burman 2014-09-04 18:18:46 UTC
The drift interval is actually what's been set on the agent, it's just that any given configuration can only be run as often as the agent's interval. Maybe this is something we should only address in the UI description field instead of fixing anything?

The default is to run every 60 seconds. If we set out drift detection to run every 120 seconds, it will run correctly every other time. And so on, as long as the time is higher than the default 60 seconds it will work (with a delay of up to 59s), however setting lower value than the agent's configuration will not work.

So instead of quite large change to modify the threading in DriftManager (for very small gain, as the user can still do drift detection with lower values, he just needs to lower the polling interval on that agent), I'd go with better documentation for the user.

Comment 6 Michael Burman 2014-09-18 09:51:55 UTC
Fixed in master (we can't validate the input on GUI, instead we send reply back when we have processed it on the agent side):

commit 6f392e47047a13931cf966f857c013b65fba97e4
Author: Michael Burman <miburman@redhat.com>
Date:   Thu Sep 18 12:49:26 2014 +0300

    [BZ 1136751] If drift definition's interval is set to a lower value than agent's interval, show resource error

Comment 8 Thomas Segismont 2014-09-23 14:59:22 UTC
Cherry-picked over to release/jon3.3.x

commit e65ac19da46181aac394a4ce3a9325cd2d776225
Author: Michael Burman <miburman@redhat.com>
Date:   Thu Sep 18 12:49:26 2014 +0300
    
    (cherry picked from commit 6f392e47047a13931cf966f857c013b65fba97e4)
    Signed-off-by: Thomas Segismont <tsegismo@redhat.com>

Comment 9 Simeon Pinder 2014-10-01 21:32:57 UTC
Moving to ON_QA as available for test with build:
https://brewweb.devel.redhat.com/buildinfo?buildID=388959

Comment 10 Armine Hovsepyan 2014-10-08 18:49:58 UTC
Created attachment 945131 [details]
drift_detection_interval_errorMsg

Comment 11 Armine Hovsepyan 2014-10-08 18:54:28 UTC
when detection interval is < agent detection interval, error msg is visible on GUI telling that detection interval is small, but

drift detection runs for *only* one drift at a time.

Please take a look into DriftDetector.run() method (line 91) -- it takes only 'next' detection from scheduled queue.  

This is not a regression, this must have been there for long period.
Log, showing that only 1 drift per run processed  is attached.

Moving back to assigned.

Comment 12 Armine Hovsepyan 2014-10-08 18:55:21 UTC
Created attachment 945133 [details]
drift_detection.log

Comment 14 John Sanda 2014-10-08 21:03:22 UTC
I talked with Armine, and I think a good argument can be made that the implementation should change such that the schedules queue is drained of all schedules that are ready for detection. 

I am not sure what the docs say about this. I think the key was this. If I set the the detection interval to 5 minutes for a drift definition, then that means drift detection will run at an interval that will be 5 minutes or greater but not less. If the agent configuration is much larger than the detection intervals, then certainly I can see where it will look like drift detection is running behind.

Comment 15 Michael Burman 2014-10-09 11:22:17 UTC
Behaviour changed in the master:

commit 6ba1d1b9d8f7a68fe6377dcc7559428e6be15dba
Author: Michael Burman <miburman@redhat.com>
Date:   Thu Oct 9 14:21:23 2014 +0300

    [BZ 1136751] For each iteration of DriftDetector, run all the available drift schedules

Comment 16 Thomas Segismont 2014-10-09 15:54:49 UTC
Cherry-picked over to release/jon3.3.x

commit f3ce9ea888ea73815973d923655760702c77132b
Author: Michael Burman <miburman@redhat.com>
Date:   Thu Oct 9 14:21:23 2014 +0300
    
    (cherry picked from commit 6ba1d1b9d8f7a68fe6377dcc7559428e6be15dba)
    Signed-off-by: Thomas Segismont <tsegismo@redhat.com>

Comment 17 Thomas Segismont 2014-10-10 13:24:22 UTC
Additional commit in master

commit 487cc7c9b3640f4fca960fd4665763052fd5303e
Author: Thomas Segismont <tsegismo@redhat.com>
Date:   Fri Oct 10 15:22:22 2014 +0200
    
    Fix issues with previous commit.
    
    Prevent the DriftDetector from running in infinite loop:
    * when the queue is full of disabled schedules
    * when a previous snapshot file exists

Comment 18 Thomas Segismont 2014-10-10 13:28:36 UTC
Cherry-picked over to release/jon3.3.x

commit 54675532db09ec12e756ae466c448cae1da62af2
Author: Thomas Segismont <tsegismo@redhat.com>
Date:   Fri Oct 10 15:22:22 2014 +0200

    Fix issues with previous commit.
    
    Prevent the DriftDetector from running in infinite loop:
    * when the queue is full of disabled schedules
    * when a previous snapshot file exists
    
    (cherry picked from commit 487cc7c9b3640f4fca960fd4665763052fd5303e)
    Signed-off-by: Thomas Segismont <tsegismo@redhat.com>

Comment 20 Simeon Pinder 2014-10-21 20:24:05 UTC
Moving to ON_QA as available to test with the latest brew build:
https://brewweb.devel.redhat.com//buildinfo?buildID=394734

Comment 21 Armine Hovsepyan 2014-10-22 10:04:48 UTC
Created attachment 949327 [details]
3_drift_detections

Comment 22 Armine Hovsepyan 2014-10-22 10:05:14 UTC
verified in JON 3.3 ER05

screen-shot attached


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