Project

General

Profile

Actions

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 over 11 years ago. Updated about 9 years ago.

Status:
Released
Priority:
1
Category:
Techniques
Target version:
Severity:
UX impact:
User visibility:
Effort required:
Priority:
Name check:
Fix check:
Regression:

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

Actions #1

Updated by Jonathan CLARKE over 11 years ago

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/3341

Author: Matthieu CERDA
Status: In progress
Priority: 1
Assignee: Matthieu CERDA
Category: Techniques
Target version: 2.4.4
Technical review done: No

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);"

Actions #2

Updated by Matthieu CERDA over 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
Actions #3

Updated by François ARMAND over 11 years ago

  • Tracker changed from Enhancement to Bug
  • Assignee changed from Jonathan CLARKE to Nicolas CHARLES
Actions #4

Updated by François ARMAND over 11 years ago

  • Tracker changed from Bug to Enhancement
Actions #5

Updated by François ARMAND over 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.

Actions #6

Updated by Nicolas CHARLES over 11 years ago

Matthieu, I've made some remarks on the PR, could ou correct them ?

Actions #7

Updated by Matthieu CERDA over 11 years ago

  • Status changed from Discussion to Pending release

Applied in changeset commit:0930271a399b59639dd95932ee15a79670ef5f34.

Actions #8

Updated by Matthieu CERDA over 11 years ago

Applied in changeset commit:4eaf4aefd6db32d179fe6cbca9282dd86c083713.

Actions #9

Updated by Nicolas CHARLES over 11 years ago

Applied in changeset commit:4ceb2af43abec26d034911d4b17ef721d0001c5a.

Actions #10

Updated by Jonathan CLARKE over 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.

Actions #11

Updated by Nicolas CHARLES over 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

Actions #12

Updated by Nicolas PERRON over 11 years ago

  • Status changed from Pending technical review to Pending release

Applied in changeset commit:f3a39c887ff6a77f392a5496e7b775a8e36e87e5.

Actions #13

Updated by Nicolas PERRON over 11 years ago

Applied in changeset commit:f3a39c887ff6a77f392a5496e7b775a8e36e87e5.

Actions #14

Updated by Nicolas PERRON over 11 years ago

Applied in changeset commit:ff7ca303b395881d708f545475c0539af46a6091.

Actions #15

Updated by Nicolas PERRON over 11 years ago

Applied in changeset commit:ff7ca303b395881d708f545475c0539af46a6091.

Actions #16

Updated by Jonathan CLARKE over 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.

Actions #17

Updated by Nicolas CHARLES over 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 somewhere

Oh, 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 erased

OK, that all sounds good then.

It's been deployed on orch-3 for test, and it seems ok

Actions #18

Updated by Nicolas PERRON about 11 years ago

  • Status changed from Pending release to Released
Actions #19

Updated by Benoît PECCATTE about 9 years ago

  • Tracker changed from Enhancement to Bug
Actions

Also available in: Atom PDF