Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 6519/don t increase serial when adding nodes #850

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,10 @@ class DetectChangeInNodeConfiguration extends Loggable {
logger.trace(s"Checking changes in node '${targetConfig.nodeInfo.id.value}'")
val changedRuleIds = currentOpt match {
case None =>
//what do we do if we don't have a cache for the node ? All the target rules are "changes" ?
//what do we do if we don't have a cache for the node ? All the target rules are "changes" ? No, we skip,
// as all changes will be caught later. Otherwise, it will increase the serial of all rules applied to this node
logger.trace("`-> No node configuration cache availabe for that node")
targetConfig.policyDrafts.map( _.ruleId ).toSet
Set[RuleId]()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, in case of clear cache, this doesn't do as expected

case Some(current) =>

val target = NodeConfigurationCache(targetConfig)
Expand Down Expand Up @@ -128,8 +129,9 @@ class DetectChangeInNodeConfiguration extends Loggable {
logger.trace(s"`-> rule with ID '${ruleId.value}' was deleted")
Set(ruleId)
case (None, Some(PolicyCache(ruleId, _, _))) =>
logger.trace(s"`-> rule with ID '${ruleId.value}' was added")
Set(ruleId)
logger.trace(s"`-> rule with ID '${ruleId.value}' was added, skipping it")
// Ignoring nodes addition to Rules. The node will be updated, thanks to the check on change of hash of node config
Set[RuleId]()
case (Some(PolicyCache(r0, d0, c0)), Some(PolicyCache(r1, d1, c1))) =>
//d0 and d1 are equals by construction, but keep them for future-proofing
if(d0 == d1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,12 @@ class NodeConfigurationChangeDetectServiceTest extends Specification {
) === Set(new RuleId("ruleId1"))
}

"have a change if nothing is different, but previous CR is not existant" in {
"have no change if nothing is different, but previous CR is not existant (extending a Rule should be free)" in {
service.detectChangeInNode(
Some(NodeConfigurationCache(emptyNodeConfig))
, complexeNodeConfig
, directiveLib
) === Set(new RuleId("ruleId1"))
) must beTheSameAs(Set())
}

"have a change if nothing is different, but previous CR is existant and current is non existant" in {
Expand Down