Skip to content

Commit

Permalink
Fixes #17237: Do not compute dynamic groups if nothing changed in LDAP
Browse files Browse the repository at this point in the history
  • Loading branch information
fanf committed Apr 26, 2020
1 parent 7bb8cc9 commit 3164d5b
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ sealed trait GroupUpdateMessage
final object GroupUpdateMessage {
final case object StartUpdate extends GroupUpdateMessage
final case object ManualStartUpdate extends GroupUpdateMessage
final case object ForceStartUpdate extends GroupUpdateMessage
final case object DelayedUpdate extends GroupUpdateMessage
final case class DynamicUpdateResult(id:Long, modId:ModificationId, start: DateTime, end:DateTime, results: Map[NodeGroupId, Box[DynGroupDiff]]) extends GroupUpdateMessage
}
Expand Down Expand Up @@ -102,6 +103,10 @@ class UpdateDynamicGroups(
laUpdateDyngroupManager ! GroupUpdateMessage.ManualStartUpdate
}

def forceStartUpdate : Unit = {
laUpdateDyngroupManager ! GroupUpdateMessage.ForceStartUpdate
}

def isIdle() = laUpdateDyngroupManager.isIdle()

////////////////////////////////////////////////////////////////
Expand All @@ -120,6 +125,8 @@ class UpdateDynamicGroups(
val logger = ScheduledJobLogger

private var updateId = 0L
private var lastUpdateTime = new DateTime(0)
private var avoidedUpdate = 0L
private var currentState: DynamicGroupUpdaterStates = IdleGroupUpdater
private var onePending = false
private var needDeployment = false
Expand All @@ -136,7 +143,10 @@ class UpdateDynamicGroups(
def isIdle() = currentState == IdleGroupUpdater

private[this] def processUpdate = {
val need = dynGroupService.changesSince(lastUpdateTime).getOrElse(true)
if(need) {
logger.trace("***** Start a new update")

currentState match {
case IdleGroupUpdater =>
dynGroupService.getAllDynGroups match {
Expand All @@ -152,6 +162,10 @@ class UpdateDynamicGroups(
case _ =>
logger.debug("Ignoring start dynamic group update request because another update is in progress")
}
} else {
avoidedUpdate = avoidedUpdate + 1
logger.debug(s"No changes that can lead to a dynamic group update happened since ${lastUpdateTime.toString(ISODateTimeFormat.dateTime())} (total ${avoidedUpdate} times avoided)")
}
}

private[this] def displayNodechange (nodes : Set[NodeId]) : String = {
Expand All @@ -177,6 +191,10 @@ class UpdateDynamicGroups(
case GroupUpdateMessage.ManualStartUpdate =>
processUpdate

case GroupUpdateMessage.ForceStartUpdate =>
lastUpdateTime = new DateTime(0)
processUpdate

// This case is launched when an update was pending, it only launch the process
// and it does not schedule a new update.
case GroupUpdateMessage.DelayedUpdate =>
Expand All @@ -188,7 +206,7 @@ class UpdateDynamicGroups(
//
case GroupUpdateMessage.DynamicUpdateResult(id, modId, start, end, results) => //TODO: other log ?
logger.trace(s"***** Get result for process: ${id}")

lastUpdateTime = start
currentState = IdleGroupUpdater

// If one update is pending, immediately start a new group update
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import cats.implicits._
import com.normation.box._
import com.normation.inventory.domain.NodeId
import com.normation.inventory.ldap.core.LDAPConstants
import com.normation.ldap.sdk.BuildFilter._
import com.normation.ldap.sdk._
import com.normation.rudder.domain.RudderLDAPConstants._
import com.normation.rudder.domain.logger.NodeLogger
Expand All @@ -53,8 +52,21 @@ import com.normation.rudder.domain.queries.Query
import com.normation.rudder.domain.RudderDit
import com.normation.rudder.repository.ldap.LDAPEntityMapper
import net.liftweb.common._
import zio._
import com.normation.ldap.sdk.syntax._
import com.unboundid.ldap.sdk.DereferencePolicy
import com.unboundid.ldap.sdk.SearchRequest
import com.normation.inventory.ldap.core.LDAPConstants._
import com.normation.ldap.sdk.BuildFilter._
import com.unboundid.ldap.sdk.Filter
import com.unboundid.ldap.sdk.LDAPException
import com.unboundid.ldap.sdk.LDAPSearchException
import com.unboundid.ldap.sdk.ResultCode
import org.joda.time.DateTime
import zio._
import zio.syntax._
import com.normation.errors._
import com.normation.rudder.domain.logger.ScheduledJobLoggerPure
import com.normation.rudder.domain.logger.TimingDebugLoggerPure

/**
* A service used to manage dynamic groups : find
Expand All @@ -67,6 +79,11 @@ trait DynGroupService {
* Retrieve the list of all dynamic groups.
*/
def getAllDynGroups(): Box[Seq[NodeGroup]]

/**
* Find if changes happened since `lastTime`.
*/
def changesSince(lastTime: DateTime): Box[Boolean]
}

class DynGroupServiceImpl(
Expand Down Expand Up @@ -106,6 +123,61 @@ class DynGroupServiceImpl(
dyngroupIds.sortBy(numberOfQuery)
}
}.toBox

override def changesSince(lastTime: DateTime): Box[Boolean] = {
val n0 = System.currentTimeMillis
if(n0 - lastTime.getMillis < 100) {
Full(true)
} else {
/*
* We want to see if an entry in:
* - ou=inventories
* - DN: ou=Accepted Inventories,ou=Inventories,cn=rudder-configuration
* - DN: ou=Removed Inventories,ou=Inventories,cn=rudder-configuration
* - DN: ou=Software,ou=Inventories,cn=rudder-configuration
* - ou=nodes: DN: ou=Nodes,cn=rudder-configuration (it will also trigger if compliance mode etc change, but no way to filter out that)
* - ou=groups: DN: groupCategoryId=GroupRoot,ou=Rudder,cn=rudder-configuration (we can filter out categories here)
*/
val searchRequest = new SearchRequest("cn=rudder-configuration", Sub.toUnboundid, DereferencePolicy.NEVER, 1, 0, false
, AND(
OR(
// ou=Removed Inventories,ou=Inventories,cn=rudder-configuration
Filter.create(s"entryDN:dnSubtreeMatch:=ou=Removed Inventories,ou=Inventories,cn=rudder-configuration")
// ou=Accepted Inventories,ou=Inventories,cn=rudder-configuration
, Filter.create(s"entryDN:dnSubtreeMatch:=ou=Accepted Inventories,ou=Inventories,cn=rudder-configuration")
, AND(IS(OC_SOFTWARE), Filter.create(s"entryDN:dnSubtreeMatch:=ou=Software,ou=Inventories,cn=rudder-configuration"))
// ou=Nodes,cn=rudder-configuration - the objectClass is used only here
, AND(IS(OC_RUDDER_NODE), Filter.create(s"entryDN:dnOneLevelMatch:=ou=Nodes,cn=rudder-configuration"))
, AND(IS(OC_RUDDER_NODE_GROUP), Filter.create(s"entryDN:dnSubtreeMatch:=groupCategoryId=GroupRoot,ou=Rudder,cn=rudder-configuration"))
)
, GTEQ("modifyTimestamp", GeneralizedTime(lastTime).toString)
)
, "1.1"
)

(for {
con <- ldap
entries <- //here, I have to rely on low-level LDAP connection, because I need to proceed size-limit exceeded as OK
(Task.effect(con.backed.search(searchRequest).getSearchEntries) catchAll {
case e:LDAPSearchException if(e.getResultCode == ResultCode.SIZE_LIMIT_EXCEEDED) =>
e.getSearchEntries().succeed
case e:LDAPException =>
SystemError("Error when searching dyngroup information", e).fail
}).foldM(
err =>
ScheduledJobLoggerPure.debug(s"Error when checking if dynamic group need update. Error was: ${err.fullMsg}") *> true.succeed
, seq => {
//we only have interesting entries in the result, so it's up to date if we have exactly 0 entries
(!seq.isEmpty).succeed
}
)
n1 <- UIO(System.currentTimeMillis)
_ <- TimingDebugLoggerPure.debug(s"Check if dynamic groups may need update (${entries}): ${n1 - n0}ms")
} yield {
entries
}).toBox
}
}
}

object CheckPendingNodeInDynGroups {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import net.liftweb.common.Box
import net.liftweb.common.EmptyBox
import net.liftweb.common.Failure
import net.liftweb.common.Full
import org.joda.time.DateTime
import org.junit.runner.RunWith
import org.specs2.mutable.Specification
import org.specs2.runner.JUnitRunner
Expand Down Expand Up @@ -148,6 +149,7 @@ class TestPendingNodePolicies extends Specification {
override def getAllDynGroups(): Box[Seq[NodeGroup]] = Full(List(
a, b, c, d, e, /*f, static */ g, h, i, j, k, l, m, n, o, pp
))
override def changesSince(lastTime: DateTime): Box[Boolean] = Full(true)
}

// a fake query checker
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ class SystemApiService11(
//For now we are not able to give information about the group reload process.
//We still send OK instead to inform the endpoint has correctly triggered.
private[this] def reloadDyngroupsWrapper() : Either[String, JField] = {
updateDynamicGroups.startManualUpdate
updateDynamicGroups.forceStartUpdate
Right(JField("groups", "Started"))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ class RestDyngroupReload(

serve {
case Get("api" :: "dyngroup" :: "reload" :: Nil, req) =>
updateDynamicGroups.startManualUpdate
updateDynamicGroups.forceStartUpdate
PlainTextResponse("OK")
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class DyngroupReloading extends DispatchSnippet with Loggable {
// JsCmd which will be sent back to the browser
// as part of the response
def process(): JsCmd = {
updateDynamicGroups.startManualUpdate
updateDynamicGroups.forceStartUpdate
Replace("dyngroupReloadingForm", outerXml.applyAgain) & JsRaw("""createSuccessNotification("Dynamic group reloading started")""")
}

Expand Down

0 comments on commit 3164d5b

Please sign in to comment.