Bug #3341
closed
Bug #3244: Detection of last promise update seems to be broken
Correct a broken time range definition in the system promises that breaks the broken BDB detection
Added by Matthieu CERDA about 11 years ago. Updated about 9 years ago.
Description
Correct a broken time range definition in the system promises that breaks the broken BDB detection
"mtime => irange(2147483647, ago(0,0,0,1,0,0));" is a broken definition, that should be "mtime => irange(ago(0,0,0,1,0,0), now);"
Pull request: https://github.com/Normation/rudder-techniques/pull/60
Updated by Jonathan CLARKE about 11 years ago
redmine@rudder-project.org a écrit :
Issue #3341 has been reported by Matthieu CERDA.
----------------------------------------
Implementation (development) #3341: Correct a broken time range
definition in the system promises that breaks the broken BDB detection
http://www.rudder-project.org/redmine/issues/3341Author: Matthieu CERDA
Status: In progress
Priority: 1
Assignee: Matthieu CERDA
Category: Techniques
Target version: 2.4.4
Technical review done: NoCorrect a broken time range definition in the system promises that
breaks the broken BDB detection"mtime => irange(2147483647, ago(0,0,0,1,0,0));" is a broken
definition, that should be "mtime => irange(ago(0,0,0,1,0,0),
now);"
Updated by Matthieu CERDA about 11 years ago
- Description updated (diff)
- Status changed from In progress to 10
- Assignee changed from Matthieu CERDA to Jonathan CLARKE
- % Done changed from 0 to 100
Pull request: https://github.com/Normation/rudder-techniques/pull/60
Updated by François ARMAND about 11 years ago
- Tracker changed from Enhancement to Bug
- Assignee changed from Jonathan CLARKE to Nicolas CHARLES
Updated by François ARMAND about 11 years ago
- Tracker changed from Bug to Enhancement
Updated by François ARMAND about 11 years ago
- Status changed from 10 to Discussion
NCH, please it's a Technical Review, but redmine does not want me to set that status.
Updated by Nicolas CHARLES about 11 years ago
Matthieu, I've made some remarks on the PR, could ou correct them ?
Updated by Matthieu CERDA about 11 years ago
- Status changed from Discussion to Pending release
Applied in changeset commit:0930271a399b59639dd95932ee15a79670ef5f34.
Updated by Matthieu CERDA about 11 years ago
Applied in changeset commit:4eaf4aefd6db32d179fe6cbca9282dd86c083713.
Updated by Nicolas CHARLES about 11 years ago
Applied in changeset commit:4ceb2af43abec26d034911d4b17ef721d0001c5a.
Updated by Jonathan CLARKE about 11 years ago
- Status changed from Pending release to Pending technical review
As I tried to say in my comment #1 above (but it got cut off somehow):
I'm pretty sure there is a reason why it was done that way around, to avoid an edge case where "now" is actually the start time of cf-agent, and the file is modified after that.
In a specific example: if I start cf-agent at 01:00:00, we have now=09:00:00 and "1 hour ago"=08:00:00. The logic of "!mtime" is then "any file created before 08:00:00 OR AFTER 09:00:00". And if cf-agent spends more than 1 second before testing this, the time is then 09:00:30 for example, it will match files created since cf-agent started.
So, unless we can prove that bug has been fixed in CFEngine, I don't think we should be reversing the logic.
Can you please explain what problem are you encountering with this, and why you want to change it? Or, if you reason to believe this bug is no longer present, please explain why.
Updated by Nicolas CHARLES about 11 years ago
As tested on the labo systems, the result was that the agent considered the file to be out of range, and thus deleting the cfengine database, at each run (so every 5 minutes) (as explained in the parent ticket)
Something must have been broken in the logic or somewhere
With the implementation being "cannot find file not between now and 1 hour ago", the result is as expected :
- if the file is older than one hour, the database get erased
- if the promises are updated, the database is not erased
Updated by Nicolas PERRON about 11 years ago
- Status changed from Pending technical review to Pending release
Applied in changeset commit:f3a39c887ff6a77f392a5496e7b775a8e36e87e5.
Updated by Nicolas PERRON about 11 years ago
Applied in changeset commit:f3a39c887ff6a77f392a5496e7b775a8e36e87e5.
Updated by Nicolas PERRON about 11 years ago
Applied in changeset commit:ff7ca303b395881d708f545475c0539af46a6091.
Updated by Nicolas PERRON about 11 years ago
Applied in changeset commit:ff7ca303b395881d708f545475c0539af46a6091.
Updated by Jonathan CLARKE about 11 years ago
Nicolas CHARLES wrote:
As tested on the labo systems, the result was that the agent considered the file to be out of range, and thus deleting the cfengine database, at each run (so every 5 minutes) (as explained in the parent ticket)
Something must have been broken in the logic or somewhere
Oh, OK. I didn't know we did parent/sub tickets for bugs, my bad.
With the implementation being "cannot find file not between now and 1 hour ago", the result is as expected :
- if the file is older than one hour, the database get erased
- if the promises are updated, the database is not erased
OK, that all sounds good then.
Updated by Nicolas CHARLES about 11 years ago
Jonathan CLARKE wrote:
Nicolas CHARLES wrote:
As tested on the labo systems, the result was that the agent considered the file to be out of range, and thus deleting the cfengine database, at each run (so every 5 minutes) (as explained in the parent ticket)
Something must have been broken in the logic or somewhereOh, OK. I didn't know we did parent/sub tickets for bugs, my bad.
Actually, we don't :)
Users have been a bit overzealous in the opening tickets
With the implementation being "cannot find file not between now and 1 hour ago", the result is as expected :
- if the file is older than one hour, the database get erased
- if the promises are updated, the database is not erasedOK, that all sounds good then.
It's been deployed on orch-3 for test, and it seems ok
Updated by Nicolas PERRON about 11 years ago
- Status changed from Pending release to Released
Updated by Benoît PECCATTE about 9 years ago
- Tracker changed from Enhancement to Bug