From 3c5e04b449900e84e0f011c30d4b0309e0011a0e Mon Sep 17 00:00:00 2001 From: Clark Andrianasolo Date: Thu, 5 Dec 2024 16:49:00 +0100 Subject: [PATCH] Fixes #26023: Technique compilation errors doesn't seems to be reloaded when the technique is deleted --- .../rudder/ncf/DeleteEditorTechnique.scala | 20 ++-- .../ncf/TechniqueCompilationCache.scala | 74 +++++++++++---- .../rudder/ncf/TechniqueWriter.scala | 86 ++++++++++++------ .../TechniqueReloadingCallbacks.scala | 18 ++++ .../ncf/TestTechniqueCompilationCache.scala | 91 +++++++++++++++---- .../rudder/rest/lift/SystemApi.scala | 2 +- .../rudder/rest/lift/TechniqueApi.scala | 4 +- .../bootstrap/liftweb/RudderConfig.scala | 8 +- 8 files changed, 225 insertions(+), 78 deletions(-) diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/DeleteEditorTechnique.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/DeleteEditorTechnique.scala index 74cb3b011ba..7baa46bbdc5 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/DeleteEditorTechnique.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/DeleteEditorTechnique.scala @@ -48,6 +48,7 @@ import com.normation.cfclerk.services.UpdateTechniqueLibrary import com.normation.errors.* import com.normation.errors.IOResult import com.normation.eventlog.ModificationId +import com.normation.inventory.domain.Version import com.normation.rudder.domain.logger.ApplicationLogger import com.normation.rudder.domain.policies.DeleteDirectiveDiff import com.normation.rudder.domain.policies.Directive @@ -79,13 +80,14 @@ trait DeleteEditorTechnique { } class DeleteEditorTechniqueImpl( - archiver: TechniqueArchiver, - techLibUpdate: UpdateTechniqueLibrary, - readDirectives: RoDirectiveRepository, - writeDirectives: WoDirectiveRepository, - techniqueRepository: TechniqueRepository, - workflowLevelService: WorkflowLevelService, - baseConfigRepoPath: String // root of config repos + archiver: TechniqueArchiver, + techLibUpdate: UpdateTechniqueLibrary, + readDirectives: RoDirectiveRepository, + writeDirectives: WoDirectiveRepository, + techniqueRepository: TechniqueRepository, + workflowLevelService: WorkflowLevelService, + compilationStatusService: TechniqueCompilationStatusSyncService, + baseConfigRepoPath: String // root of config repos ) extends DeleteEditorTechnique { // root of technique repository val techniquesDir: File = File(baseConfigRepoPath) / "techniques" @@ -237,6 +239,10 @@ class DeleteEditorTechniqueImpl( case Some(technique) => removeTechnique(techniqueId, technique) case None => removeInvalidTechnique(techniquesDir, techniqueId) } + + _ <- compilationStatusService.unsyncOne( + BundleName(techniqueName) -> new Version(techniqueVersion) + ) // to delete the status of the technique } yield () } } diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/TechniqueCompilationCache.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/TechniqueCompilationCache.scala index a6f49235701..1613b8a5f07 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/TechniqueCompilationCache.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/TechniqueCompilationCache.scala @@ -43,6 +43,7 @@ import com.normation.rudder.batch.UpdateCompilationStatus import com.normation.rudder.domain.logger.StatusLoggerPure import net.liftweb.common.SimpleActor import zio.* +import zio.syntax.* sealed trait CompilationResult object CompilationResult { @@ -100,10 +101,13 @@ trait TechniqueCompilationStatusSyncService { */ def syncOne(result: EditorTechniqueCompilationResult): IOResult[Unit] - /* - * The whole process that lookup for compilation status and update everything + def unsyncOne(id: (BundleName, Version)): IOResult[Unit] + + /** + * The whole process that lookup for compilation status and update everything. + * @param results if none all results are looked up, if some only consider these ones */ - def getUpdateAndSync(): IOResult[Unit] + def getUpdateAndSync(results: Option[List[EditorTechniqueCompilationResult]] = None): IOResult[Unit] } /** @@ -158,36 +162,35 @@ class TechniqueCompilationErrorsActorSync( errorBase: Ref[Map[(BundleName, Version), EditorTechniqueError]] ) extends TechniqueCompilationStatusSyncService { - /* - * Update the internal cache and build a Compilation status - */ - private[ncf] def updateStatus(results: List[EditorTechniqueCompilationResult]): UIO[CompilationStatus] = { - errorBase.updateAndGet { m => - results.foldLeft(m) { - case (current, EditorTechniqueCompilationResult(id, version, name, CompilationResult.Error(error))) => - current + ((id, version) -> EditorTechniqueError(id, version, name, error)) - case (current, EditorTechniqueCompilationResult(id, version, _, CompilationResult.Success)) => - current - ((id, version)) - } - }.map(m => getStatus(m.values)) - } - /* * Given new editor technique compilation results, update current status and sync it with UI */ def syncOne(result: EditorTechniqueCompilationResult): IOResult[Unit] = { for { - status <- updateStatus(List(result)) + status <- updateOneStatus(result) _ <- syncStatusWithUi(status) } yield () } + /** + * Drop a value from the error base if it exists, sync the status to forget the specified one + */ + def unsyncOne(id: (BundleName, Version)): IOResult[Unit] = { + for { + base <- + errorBase.updateAndGet(_ - id) + status = getStatus(base.values) + + _ <- syncStatusWithUi(status) + } yield () + } + /* * The whole process that lookup for compilation status and update everything */ - def getUpdateAndSync(): IOResult[Unit] = { + def getUpdateAndSync(results: Option[List[EditorTechniqueCompilationResult]]): IOResult[Unit] = { (for { - results <- reader.get() + results <- results.map(_.succeed).getOrElse(reader.get()) status <- updateStatus(results) _ <- syncStatusWithUi(status) } yield status).flatMap { @@ -214,6 +217,37 @@ class TechniqueCompilationErrorsActorSync( } } + /* + * Update the internal cache and build a Compilation status + */ + private[ncf] def updateStatus(results: List[EditorTechniqueCompilationResult]): UIO[CompilationStatus] = { + errorBase.updateAndGet { m => + results.collect { + case r @ EditorTechniqueCompilationResult(id, version, name, CompilationResult.Error(error)) => + (getKey(r) -> EditorTechniqueError(id, version, 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] = { + // 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)) + case _ => + _ => None + } + errorBase + .updateAndGet(_.updatedWith(getKey(result))(replacement(_))) + .map(m => getStatus(m.values)) + } + + private def getKey(result: EditorTechniqueCompilationResult): (BundleName, Version) = { + result.id -> result.version + } } object TechniqueCompilationErrorsActorSync { diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/TechniqueWriter.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/TechniqueWriter.scala index 24d84d88313..5bcc35b09ed 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/TechniqueWriter.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/TechniqueWriter.scala @@ -84,6 +84,12 @@ trait TechniqueWriter { modId: ModificationId, committer: EventActor ): IOResult[EditorTechnique] + + def writeTechniques( + techniques: List[EditorTechnique], + modId: ModificationId, + committer: EventActor + ): IOResult[List[EditorTechnique]] } /** @@ -106,7 +112,9 @@ class TechniqueWriterImpl( deleteDirective: Boolean, modId: ModificationId, committer: QueryContext - ): IOResult[Unit] = deleteService.deleteTechnique(techniqueName, techniqueVersion, deleteDirective, modId, committer) + ): IOResult[Unit] = { + deleteService.deleteTechnique(techniqueName, techniqueVersion, deleteDirective, modId, committer) + } def writeTechniqueAndUpdateLib( technique: EditorTechnique, @@ -114,8 +122,10 @@ class TechniqueWriterImpl( committer: EventActor ): IOResult[EditorTechnique] = { for { - updatedTechnique <- compileArchiveTechnique(technique, modId, committer) - libUpdate <- + updated <- + compileArchiveTechnique(technique, modId, committer, syncStatus = false) // sync is already0done in library update + (updatedTechnique, _) = updated + libUpdate <- techLibUpdate .update(modId, committer, Some(s"Update Technique library after creating files for ncf Technique ${technique.name}")) .toIO @@ -130,17 +140,32 @@ class TechniqueWriterImpl( modId: ModificationId, committer: EventActor ): IOResult[EditorTechnique] = { - compileArchiveTechnique(technique, modId, committer) + compileArchiveTechnique(technique, modId, committer).map { case (t, _) => t } + } + + override def writeTechniques( + techniques: List[EditorTechnique], + modId: ModificationId, + committer: EventActor + ): IOResult[List[EditorTechnique]] = { + for { + updated <- ZIO.foreach(techniques)(compileArchiveTechnique(_, modId, committer, syncStatus = false)) + (updatedTechniques, results) = updated.unzip + _ <- compilationStatusService.getUpdateAndSync(Some(results)) + } yield { + updatedTechniques + } } ///// utility methods ///// // Write and commit all techniques files private def compileArchiveTechnique( - technique: EditorTechnique, - modId: ModificationId, - committer: EventActor - ): IOResult[EditorTechnique] = { + technique: EditorTechnique, + modId: ModificationId, + committer: EventActor, + syncStatus: Boolean = true // should update the compilation status ? + ): IOResult[(EditorTechnique, EditorTechniqueCompilationResult)] = { for { time_0 <- currentTimeMillis _ <- TechniqueWriterLoggerPure.debug(s"Writing technique '${technique.name}'") @@ -150,33 +175,34 @@ class TechniqueWriterImpl( } techniqueWithResourceUpdated = technique.copy(resources = updateResources) - _ <- TechniqueWriterImpl.writeYaml(techniqueWithResourceUpdated)(baseConfigRepoPath) - time_1 <- currentTimeMillis - _ <- + _ <- TechniqueWriterImpl.writeYaml(techniqueWithResourceUpdated)(baseConfigRepoPath) + time_1 <- currentTimeMillis + _ <- TimingDebugLoggerPure.trace(s"writeTechnique: writing yaml for technique '${technique.name}' took ${time_1 - time_0}ms") - compiled <- compiler.compileTechnique(techniqueWithResourceUpdated) - _ <- compilationStatusService.syncOne(EditorTechniqueCompilationResult.from(techniqueWithResourceUpdated, compiled)) - time_3 <- currentTimeMillis - id <- TechniqueVersion.parse(technique.version.value).toIO.map(v => TechniqueId(TechniqueName(technique.id.value), v)) + compiled <- compiler.compileTechnique(techniqueWithResourceUpdated) + compilationResult = EditorTechniqueCompilationResult.from(techniqueWithResourceUpdated, compiled) + _ <- compilationStatusService.syncOne(compilationResult) + time_3 <- currentTimeMillis + id <- TechniqueVersion.parse(technique.version.value).toIO.map(v => TechniqueId(TechniqueName(technique.id.value), v)) // resources files are missing the the "resources/" prefix - resources = technique.resources.map(r => ResourceFile("resources/" + r.path, r.state)) - _ <- archiver.saveTechnique( - id, - technique.category.split('/').toIndexedSeq, - Chunk.fromIterable(resources), - modId, - committer, - s"Committing technique ${technique.name}" - ) - time_4 <- currentTimeMillis - _ <- TimingDebugLoggerPure.trace(s"writeTechnique: committing technique '${technique.name}' took ${time_4 - time_3}ms") - _ <- TimingDebugLoggerPure.debug(s"writeTechnique: writing technique '${technique.name}' took ${time_4 - time_0}ms") - time_5 <- currentTimeMillis - _ <- + resources = technique.resources.map(r => ResourceFile("resources/" + r.path, r.state)) + _ <- archiver.saveTechnique( + id, + technique.category.split('/').toIndexedSeq, + Chunk.fromIterable(resources), + modId, + committer, + s"Committing technique ${technique.name}" + ) + time_4 <- currentTimeMillis + _ <- TimingDebugLoggerPure.trace(s"writeTechnique: committing technique '${technique.name}' took ${time_4 - time_3}ms") + _ <- TimingDebugLoggerPure.debug(s"writeTechnique: writing technique '${technique.name}' took ${time_4 - time_0}ms") + time_5 <- currentTimeMillis + _ <- TimingDebugLoggerPure.trace(s"writeTechnique: writing technique '${technique.name}' in cache took ${time_5 - time_4}ms") } yield { - techniqueWithResourceUpdated + (techniqueWithResourceUpdated, compilationResult) } } } diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/TechniqueReloadingCallbacks.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/TechniqueReloadingCallbacks.scala index e4cb9280ba3..8dd8e9c2b59 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/TechniqueReloadingCallbacks.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/TechniqueReloadingCallbacks.scala @@ -48,6 +48,7 @@ import com.normation.eventlog.ModificationId import com.normation.rudder.batch.AsyncDeploymentActor import com.normation.rudder.batch.AutomaticStartDeployment import com.normation.rudder.domain.eventlog.ReloadTechniqueLibrary +import com.normation.rudder.ncf.TechniqueCompilationStatusSyncService import com.normation.rudder.repository.EventLogRepository import net.liftweb.common.* @@ -105,3 +106,20 @@ class LogEventOnTechniqueReloadCallback( .toBox } } + +class SyncCompilationStatusOnTechniqueCallback( + override val name: String, + override val order: Int, + techniqueCompilationStatusService: TechniqueCompilationStatusSyncService +) extends TechniquesLibraryUpdateNotification with Loggable { + override def updatedTechniques( + gitRev: String, + techniqueIds: Map[TechniqueName, TechniquesLibraryUpdateType], + updatedCategories: Set[TechniqueCategoryModType], + modId: ModificationId, + actor: EventActor, + reason: Option[String] + ): Box[Unit] = { + techniqueCompilationStatusService.getUpdateAndSync().toBox + } +} diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/ncf/TestTechniqueCompilationCache.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/ncf/TestTechniqueCompilationCache.scala index 64fa483dc5f..ada3b3c35df 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/ncf/TestTechniqueCompilationCache.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/ncf/TestTechniqueCompilationCache.scala @@ -200,29 +200,88 @@ class TestTechniqueCompilationCache extends Specification with BeforeAfterAll { writeCache.updateStatus(expectedListOfOutputs).runNow must beEqualTo(expectedCompilationStatus) } + val newError = { + EditorTechniqueCompilationResult( + BundleName("new tech"), + technique1.version, + "new tech", + CompilationResult.Error("new tech error") + ) + } + val newErrorStatus = CompilationStatusErrors( + NonEmptyChunk( + EditorTechniqueError( + newError.id, + newError.version, + newError.name, + "new tech error" + ) + ) + ) "sync with UI" in { - val newError = { + (writeCache.syncOne(newError) *> // println(mockActor.messages.head).succeed *> + msgLock.withPermit( + (mockActor hasReceivedMessage_? UpdateCompilationStatus(expectedCompilationStatus ++ newErrorStatus)).succeed + )).runNow.aka("actor received message") must beTrue and (mockActor.messageCount must beEqualTo(1)) + } + + "update an existing technique in error" in { + val updatedResult = { EditorTechniqueCompilationResult( - BundleName("new tech"), - technique1.version, - "new tech", - CompilationResult.Error("new tech error") + technique3.id, + technique3.version, + technique3.name, + CompilationResult.Error("new tech3 error") ) } - val expectedStatus = CompilationStatusErrors( - NonEmptyChunk( - EditorTechniqueError( - newError.id, - newError.version, - newError.name, - "new tech error" + writeCache.updateOneStatus(updatedResult).runNow must beEqualTo( + CompilationStatusErrors( + NonEmptyChunk( + EditorTechniqueError( + technique1.id, + technique1.version, + technique1.name, + "tech1 error" + ), + EditorTechniqueError( + technique3.id, + technique3.version, + technique3.name, + "new tech3 error" + ) ) - ) + ) ++ newErrorStatus ) - (writeCache.syncOne(newError) *> // println(mockActor.messages.head).succeed *> + } + + "update with an technique in success" in { + val success = + EditorTechniqueCompilationResult(technique1.id, technique1.version, technique1.name, CompilationResult.Success) + writeCache.updateOneStatus(success).runNow must beEqualTo( + CompilationStatusErrors( + NonEmptyChunk( + EditorTechniqueError( + technique3.id, + technique3.version, + technique3.name, + "new tech3 error" + ) + ) + ) ++ newErrorStatus + ) + } + + "delete no longer existing techniques, keep only new one" in { + writeCache.updateStatus(List(newError)).runNow must beEqualTo( + newErrorStatus + ) + } + + "unsync one" in { + (writeCache.unsyncOne(newError.id -> newError.version) *> msgLock.withPermit( - (mockActor hasReceivedMessage_? UpdateCompilationStatus(expectedCompilationStatus ++ expectedStatus)).succeed - )).runNow must beTrue and (mockActor.messageCount must beEqualTo(1)) + (mockActor hasReceivedMessage_? UpdateCompilationStatus(CompilationStatusAllSuccess)).succeed + )).runNow.aka("actor received message") must beTrue and (mockActor.messageCount must beEqualTo(2)) } } } diff --git a/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/SystemApi.scala b/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/SystemApi.scala index 9990838d836..4dcf9eb383c 100644 --- a/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/SystemApi.scala +++ b/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/SystemApi.scala @@ -685,7 +685,7 @@ class SystemApiService11( getActor(req), Some("Technique library reloaded from REST API") ) match { - case Full(x) => Right(JField("techniques", "Started")) + case Full(_) => Right(JField("techniques", "Started")) case eb: EmptyBox => val e = eb ?~! "An error occurred when updating the Technique library from file system" logger.error(e.messageChain) diff --git a/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/TechniqueApi.scala b/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/TechniqueApi.scala index 1020c1d51f0..8f57b152e6b 100644 --- a/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/TechniqueApi.scala +++ b/webapp/sources/rudder/rudder-rest/src/main/scala/com/normation/rudder/rest/lift/TechniqueApi.scala @@ -366,8 +366,8 @@ class TechniqueApi( s"An error occurred while reading techniques when updating them: ${errors.map(_.msg).mkString("\n ->", "\n ->", "")}" ) } - _ <- ZIO.foreach(techniques)(t => techniqueWriter.writeTechnique(t, modId, authzToken.qc.actor)) - json <- ZIO.foreach(techniques)(_.toJsonAST.toIO) + res <- techniqueWriter.writeTechniques(techniques, modId, authzToken.qc.actor) + json <- ZIO.foreach(res)(_.toJsonAST.toIO) } yield { json } diff --git a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala index f17b7df4724..f5291015dca 100644 --- a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala +++ b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala @@ -1919,8 +1919,11 @@ object RudderConfigInit { techniqueCompiler ) - lazy val techniqueCompilationCache: TechniqueCompilationStatusSyncService = - TechniqueCompilationErrorsActorSync.make(asyncDeploymentAgent, techniqueCompilationStatusService).runNow + lazy val techniqueCompilationCache: TechniqueCompilationStatusSyncService = { + val sync = TechniqueCompilationErrorsActorSync.make(asyncDeploymentAgent, techniqueCompilationStatusService).runNow + techniqueRepositoryImpl.registerCallback(new SyncCompilationStatusOnTechniqueCallback("SyncCompilationStatus", 10000, sync)) + sync + } lazy val ncfTechniqueWriter: TechniqueWriter = new TechniqueWriterImpl( techniqueArchiver, @@ -1932,6 +1935,7 @@ object RudderConfigInit { woDirectiveRepository, techniqueRepository, workflowLevelService, + techniqueCompilationCache, RUDDER_GIT_ROOT_CONFIG_REPO ), techniqueCompiler,