Skip to content

Commit

Permalink
Fixes #26022: Status should not be on error when there is technique c…
Browse files Browse the repository at this point in the history 
…ompilator error on disable techniques
  • Loading branch information
@clarktsiory @fanf
clarktsiory authored and fanf committed Dec 17, 2024
1 parent e0616f1 commit 7d6bfac
Show file tree
Hide file tree
Showing 6 changed files with 223 additions and 45 deletions.
158 changes: 140 additions & 18 deletions ...udder/rudder-core/src/main/scala/com/normation/rudder/ncf/TechniqueCompilationCache.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@

package com.normation.rudder.ncf

import com.normation.cfclerk.domain.TechniqueName
import com.normation.errors.IOResult
import com.normation.inventory.domain.Version
import com.normation.rudder.batch.UpdateCompilationStatus
import com.normation.rudder.domain.logger.StatusLoggerPure
import com.normation.rudder.domain.policies.ActiveTechnique
import com.normation.rudder.repository.FullActiveTechnique
import com.normation.rudder.repository.RoDirectiveRepository
import net.liftweb.common.SimpleActor
import zio.*
import zio.syntax.*
Expand Down Expand Up @@ -71,12 +75,30 @@ object EditorTechniqueCompilationResult {
case class EditorTechniqueError(
id: BundleName,
version: Version,
status: TechniqueActiveStatus,
name: String,
errorMessage: String
)

sealed trait CompilationStatus
case object CompilationStatusAllSuccess extends CompilationStatus
object CompilationStatus {
def fromErrors(errors: Chunk[EditorTechniqueError]): CompilationStatus = {
NonEmptyChunk.fromChunk(errors) match {
case None => CompilationStatusAllSuccess
case Some(value) => CompilationStatusErrors(value)
}
}

def ignoreDisabledTechniques(status: CompilationStatus): CompilationStatus = {
status match {
case CompilationStatusErrors(techniquesInError) =>
fromErrors(techniquesInError.collect { case e @ EditorTechniqueError(_, _, TechniqueActiveStatus.Enabled, _, _) => e })
case CompilationStatusAllSuccess =>
CompilationStatusAllSuccess
}
}
}
case object CompilationStatusAllSuccess extends CompilationStatus
case class CompilationStatusErrors(techniquesInError: NonEmptyChunk[EditorTechniqueError]) extends CompilationStatus {
def ++(other: CompilationStatusErrors): CompilationStatusErrors = CompilationStatusErrors(
this.techniquesInError ++ other.techniquesInError
Expand All @@ -92,6 +114,23 @@ trait ReadEditorTechniqueCompilationResult {

}

sealed trait TechniqueActiveStatus
object TechniqueActiveStatus {
case object Enabled extends TechniqueActiveStatus
case object Disabled extends TechniqueActiveStatus
}

/**
* Get attribute from the configuration techniques
*/
trait ReadEditorTechniqueActiveStatus {

def getActiveStatus(id: BundleName): IOResult[Option[TechniqueActiveStatus]]

def getActiveStatuses(): IOResult[Map[BundleName, TechniqueActiveStatus]]

}

/**
* Update technique compilation status from technique compilation output and technique info
*/
Expand All @@ -101,6 +140,12 @@ trait TechniqueCompilationStatusSyncService {
*/
def syncOne(result: EditorTechniqueCompilationResult): IOResult[Unit]

/*
* Given the identifier of technique, only update its known status and sync with the UI.
* Status applies to part of the id : only BundleName, so any version could be updated
*/
def syncTechniqueActiveStatus(bundleName: BundleName): IOResult[Unit]

def unsyncOne(id: (BundleName, Version)): IOResult[Unit]

/**
Expand Down Expand Up @@ -146,6 +191,35 @@ class TechniqueCompilationStatusService(

}

/**
* Service to gather technique attributes using the directive reposistory :
* it knows about active techniques and their status
*/
class TechniqueActiveStatusService(directiveRepo: RoDirectiveRepository) extends ReadEditorTechniqueActiveStatus {

override def getActiveStatus(id: BundleName): IOResult[Option[TechniqueActiveStatus]] = {
directiveRepo.getActiveTechnique(TechniqueName(id.value)).map(_.map(techniqueActiveStatus(_)))
}
override def getActiveStatuses(): IOResult[Map[BundleName, TechniqueActiveStatus]] = {
directiveRepo.getFullDirectiveLibrary().map(_.activeTechniques.map(t => techniqueId(t) -> techniqueActiveStatus(t)).toMap)
}

private def techniqueId(technique: FullActiveTechnique): BundleName = BundleName(technique.techniqueName.value)

private def techniqueActiveStatus(technique: ActiveTechnique): TechniqueActiveStatus = {
technique.isEnabled match {
case true => TechniqueActiveStatus.Enabled
case false => TechniqueActiveStatus.Disabled
}
}
private def techniqueActiveStatus(technique: FullActiveTechnique): TechniqueActiveStatus = {
technique.isEnabled match {
case true => TechniqueActiveStatus.Enabled
case false => TechniqueActiveStatus.Disabled
}
}
}

/**
* Technique compilation output needs to be saved in a cache (frequent reads, we don't want to get files on the FS every time).
*
Expand All @@ -157,18 +231,28 @@ class TechniqueCompilationStatusService(
* when technique library is reloaded
*/
class TechniqueCompilationErrorsActorSync(
actor: SimpleActor[UpdateCompilationStatus],
reader: ReadEditorTechniqueCompilationResult,
errorBase: Ref[Map[(BundleName, Version), EditorTechniqueError]]
actor: SimpleActor[UpdateCompilationStatus],
reader: ReadEditorTechniqueCompilationResult,
attributesReader: ReadEditorTechniqueActiveStatus,
errorBase: Ref[Map[(BundleName, Version), EditorTechniqueError]]
) extends TechniqueCompilationStatusSyncService {

/*
* Given new editor technique compilation results, update current status and sync it with UI
*/
def syncOne(result: EditorTechniqueCompilationResult): IOResult[Unit] = {
for {
status <- updateOneStatus(result)
_ <- syncStatusWithUi(status)
activeStatus <- getActiveStatusOrDisabled(result.id)
status <- updateOneStatus(result, activeStatus)
_ <- syncStatusWithUi(status)
} yield ()
}

def syncTechniqueActiveStatus(bundleName: BundleName): IOResult[Unit] = {
for {
activeStatus <- getActiveStatusOrDisabled(bundleName)
status <- updateOneActiveStatus(bundleName, activeStatus)
_ <- syncStatusWithUi(status)
} yield ()
}

Expand All @@ -186,13 +270,17 @@ class TechniqueCompilationErrorsActorSync(
}

/*
* The whole process that lookup for compilation status and update everything
* The whole process that lookup for compilation status and update everything.
* Sync always looks up for the latest status of techniques to filter out disabled ones.
*/
def getUpdateAndSync(results: Option[List[EditorTechniqueCompilationResult]]): IOResult[Unit] = {
(for {
results <- results.map(_.succeed).getOrElse(reader.get())
status <- updateStatus(results)
_ <- syncStatusWithUi(status)
res <- results.map(_.succeed).getOrElse(reader.get())
activeStatuses <- attributesReader.getActiveStatuses()
// ones without status are filtered out (if the status is unknown, they are ignored)
resWithStatus = res.flatMap(r => activeStatuses.get(r.id).map(r -> _))
status <- updateStatus(resWithStatus)
_ <- syncStatusWithUi(status)
} yield status).flatMap {
case CompilationStatusAllSuccess => StatusLoggerPure.Techniques.info("All techniques have success compilation result")
case e: CompilationStatusErrors =>
Expand All @@ -210,6 +298,16 @@ class TechniqueCompilationErrorsActorSync(
IOResult.attempt(actor ! UpdateCompilationStatus(status))
}

/**
* Get the latest active status from the technique
* Not found technique should mean that it is disabled/deleted.
*/
private def getActiveStatusOrDisabled(id: BundleName): IOResult[TechniqueActiveStatus] = {
attributesReader
.getActiveStatus(id)
.map(_.getOrElse(TechniqueActiveStatus.Disabled))
}

private def getStatus(errors: Iterable[EditorTechniqueError]): CompilationStatus = {
NonEmptyChunk.fromIterableOption(errors) match {
case None => CompilationStatusAllSuccess
Expand All @@ -220,23 +318,31 @@ class TechniqueCompilationErrorsActorSync(
/*
* Update the internal cache and build a Compilation status
*/
private[ncf] def updateStatus(results: List[EditorTechniqueCompilationResult]): UIO[CompilationStatus] = {
private[ncf] def updateStatus(
results: List[(EditorTechniqueCompilationResult, TechniqueActiveStatus)]
): UIO[CompilationStatus] = {
errorBase.updateAndGet { m =>
results.collect {
case r @ EditorTechniqueCompilationResult(id, version, name, CompilationResult.Error(error)) =>
(getKey(r) -> EditorTechniqueError(id, version, name, error))
case (
r @ EditorTechniqueCompilationResult(id, version, name, CompilationResult.Error(error)),
activeStatus
) =>
(getKey(r) -> EditorTechniqueError(id, version, activeStatus, name, error))
}.toMap
}.map(m => getStatus(m.values))
}

/**
* Only take a single result to update the cached technique if it is there, else do nothing
*/
private[ncf] def updateOneStatus(result: EditorTechniqueCompilationResult): UIO[CompilationStatus] = {
private[ncf] def updateOneStatus(
result: EditorTechniqueCompilationResult,
activeStatus: TechniqueActiveStatus
): UIO[CompilationStatus] = {
// only replace when current one is an error, when present or absent we should set the value
val replacement: Option[EditorTechniqueError] => Option[EditorTechniqueError] = result match {
case EditorTechniqueCompilationResult(id, version, name, CompilationResult.Error(error)) =>
_ => Some(EditorTechniqueError(id, version, name, error))
_ => Some(EditorTechniqueError(id, version, activeStatus, name, error))
case _ =>
_ => None
}
Expand All @@ -245,18 +351,34 @@ class TechniqueCompilationErrorsActorSync(
.map(m => getStatus(m.values))
}

/**
* Only update the active status of the technique : replace the active status if the technique exists in cache
*/
private[ncf] def updateOneActiveStatus(
bundleName: BundleName,
activeStatus: TechniqueActiveStatus
): UIO[CompilationStatus] = {
errorBase
.updateAndGet(_.map {
case (id @ (`bundleName`, version), err) => id -> err.copy(status = activeStatus)
case o => o
})
.map(m => getStatus(m.values))
}

private def getKey(result: EditorTechniqueCompilationResult): (BundleName, Version) = {
result.id -> result.version
}
}

object TechniqueCompilationErrorsActorSync {
def make(
actor: SimpleActor[UpdateCompilationStatus],
reader: ReadEditorTechniqueCompilationResult
actor: SimpleActor[UpdateCompilationStatus],
reader: ReadEditorTechniqueCompilationResult,
attributesReader: ReadEditorTechniqueActiveStatus
): UIO[TechniqueCompilationErrorsActorSync] = {
Ref
.make(Map.empty[(BundleName, Version), EditorTechniqueError])
.map(new TechniqueCompilationErrorsActorSync(actor, reader, _))
.map(new TechniqueCompilationErrorsActorSync(actor, reader, attributesReader, _))
}
}
46 changes: 40 additions & 6 deletions ...r/rudder-core/src/test/scala/com/normation/rudder/ncf/TestTechniqueCompilationCache.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@ class TestTechniqueCompilationCache extends Specification with BeforeAfterAll {

}

private val readTechniqueActiveStatus: ReadEditorTechniqueActiveStatus = new ReadEditorTechniqueActiveStatus {
override def getActiveStatus(id: BundleName): IOResult[Option[TechniqueActiveStatus]] = Some(
TechniqueActiveStatus.Enabled
).succeed
override def getActiveStatuses(): IOResult[Map[BundleName, TechniqueActiveStatus]] =
techniques.map(_.id -> TechniqueActiveStatus.Enabled).toMap.succeed
}

// the SUT
private val compilationStatusService: ReadEditorTechniqueCompilationResult = new TechniqueCompilationStatusService(
editorTechniqueReader,
Expand Down Expand Up @@ -150,7 +158,8 @@ class TestTechniqueCompilationCache extends Specification with BeforeAfterAll {
)
.runNow
}
private val writeCache = TechniqueCompilationErrorsActorSync.make(mockActor, compilationStatusService).runNow
private val writeCache =
TechniqueCompilationErrorsActorSync.make(mockActor, compilationStatusService, readTechniqueActiveStatus).runNow

// create output test files for each technique
override def beforeAll(): Unit = {
Expand All @@ -172,12 +181,14 @@ class TestTechniqueCompilationCache extends Specification with BeforeAfterAll {
EditorTechniqueError(
technique1.id,
technique1.version,
TechniqueActiveStatus.Enabled,
technique1.name,
"tech1 error"
),
EditorTechniqueError(
technique3.id,
technique3.version,
TechniqueActiveStatus.Enabled,
technique3.name,
"tech3 error"
)
Expand All @@ -197,7 +208,9 @@ class TestTechniqueCompilationCache extends Specification with BeforeAfterAll {

"Error repository" should {
"Correctly build the status" in {
writeCache.updateStatus(expectedListOfOutputs).runNow must beEqualTo(expectedCompilationStatus)
writeCache.updateStatus(expectedListOfOutputs.map(_ -> TechniqueActiveStatus.Enabled)).runNow must beEqualTo(
expectedCompilationStatus
)
}

val newError = {
Expand All @@ -213,6 +226,7 @@ class TestTechniqueCompilationCache extends Specification with BeforeAfterAll {
EditorTechniqueError(
newError.id,
newError.version,
TechniqueActiveStatus.Enabled,
newError.name,
"new tech error"
)
Expand All @@ -234,18 +248,20 @@ class TestTechniqueCompilationCache extends Specification with BeforeAfterAll {
CompilationResult.Error("new tech3 error")
)
}
writeCache.updateOneStatus(updatedResult).runNow must beEqualTo(
writeCache.updateOneStatus(updatedResult, TechniqueActiveStatus.Enabled).runNow must beEqualTo(
CompilationStatusErrors(
NonEmptyChunk(
EditorTechniqueError(
technique1.id,
technique1.version,
TechniqueActiveStatus.Enabled,
technique1.name,
"tech1 error"
),
EditorTechniqueError(
technique3.id,
technique3.version,
TechniqueActiveStatus.Enabled,
technique3.name,
"new tech3 error"
)
Expand All @@ -257,12 +273,13 @@ class TestTechniqueCompilationCache extends Specification with BeforeAfterAll {
"update with an technique in success" in {
val success =
EditorTechniqueCompilationResult(technique1.id, technique1.version, technique1.name, CompilationResult.Success)
writeCache.updateOneStatus(success).runNow must beEqualTo(
writeCache.updateOneStatus(success, TechniqueActiveStatus.Enabled).runNow must beEqualTo(
CompilationStatusErrors(
NonEmptyChunk(
EditorTechniqueError(
technique3.id,
technique3.version,
TechniqueActiveStatus.Enabled,
technique3.name,
"new tech3 error"
)
Expand All @@ -272,16 +289,33 @@ class TestTechniqueCompilationCache extends Specification with BeforeAfterAll {
}

"delete no longer existing techniques, keep only new one" in {
writeCache.updateStatus(List(newError)).runNow must beEqualTo(
writeCache.updateStatus(List(newError -> TechniqueActiveStatus.Enabled)).runNow must beEqualTo(
newErrorStatus
)
}

"update a technique with disabled status" in {
writeCache.updateOneStatus(newError, TechniqueActiveStatus.Disabled).runNow must beEqualTo(
CompilationStatusErrors(
newErrorStatus.techniquesInError.map(_.copy(status = TechniqueActiveStatus.Disabled))
)
)
}

"sync technique active status" in {
// bring the status back to Enabled
(writeCache.syncTechniqueActiveStatus(newError.id) *>
msgLock.withPermit(
(mockActor hasReceivedMessage_? UpdateCompilationStatus(newErrorStatus)).succeed
)).runNow.aka("actor received message") must beTrue and (mockActor.messageCount must beEqualTo(2))
}

"unsync one" in {
(writeCache.unsyncOne(newError.id -> newError.version) *>
msgLock.withPermit(
(mockActor hasReceivedMessage_? UpdateCompilationStatus(CompilationStatusAllSuccess)).succeed
)).runNow.aka("actor received message") must beTrue and (mockActor.messageCount must beEqualTo(2))
)).runNow.aka("actor received message") must beTrue and (mockActor.messageCount must beEqualTo(3))
}

}
}
Loading

0 comments on commit 7d6bfac

Please sign in to comment.