From e70c8bdf7d9ccdb2b652a4d0c4bdf7ed77e9c75c Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sat, 14 Sep 2024 16:35:17 +0200 Subject: [PATCH 01/24] pause and resume agents server side --- .../service/BuildJobManagementService.java | 9 +- .../service/SharedQueueProcessingService.java | 96 ++++++++++++++++--- .../web/admin/AdminBuildJobQueueResource.java | 43 +++++++++ .../localci/SharedQueueManagementService.java | 14 +++ 4 files changed, 146 insertions(+), 16 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java index f514d59496cd..dbf299204d41 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java @@ -170,9 +170,12 @@ public CompletableFuture executeBuildJob(BuildJobQueueItem buildJob } } }); - futureResult.whenComplete(((result, throwable) -> runningFutures.remove(buildJobItem.id()))); - return futureResult; + return futureResult.whenComplete(((result, throwable) -> runningFutures.remove(buildJobItem.id()))); + } + + Set getRunningBuildJobIds() { + return Set.copyOf(runningFutures.keySet()); } /** @@ -227,7 +230,7 @@ private void finishBuildJobExceptionally(String buildJobId, String containerName * * @param buildJobId The id of the build job that should be cancelled. */ - private void cancelBuildJob(String buildJobId) { + void cancelBuildJob(String buildJobId) { Future future = runningFutures.get(buildJobId); if (future != null) { try { diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index f82f7aa7a36f..8f43cce7d9cf 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -2,6 +2,7 @@ import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_BUILDAGENT; +import java.time.Duration; import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.List; @@ -12,6 +13,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantLock; @@ -24,6 +26,7 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.annotation.Profile; +import org.springframework.scheduling.TaskScheduler; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Service; @@ -33,6 +36,7 @@ import com.hazelcast.collection.ItemListener; import com.hazelcast.core.HazelcastInstance; import com.hazelcast.map.IMap; +import com.hazelcast.topic.ITopic; import de.tum.cit.aet.artemis.buildagent.dto.BuildAgentInformation; import de.tum.cit.aet.artemis.buildagent.dto.BuildJobQueueItem; @@ -64,6 +68,8 @@ public class SharedQueueProcessingService { private final BuildAgentSshKeyService buildAgentSSHKeyService; + private final TaskScheduler taskScheduler; + private IQueue queue; private IQueue resultQueue; @@ -82,13 +88,21 @@ public class SharedQueueProcessingService { private UUID listenerId; + /** + * Scheduled future for checking availability and processing next build job. + */ + private ScheduledFuture scheduledFuture; + + private boolean isPaused = false; + public SharedQueueProcessingService(@Qualifier("hazelcastInstance") HazelcastInstance hazelcastInstance, ExecutorService localCIBuildExecutorService, - BuildJobManagementService buildJobManagementService, BuildLogsMap buildLogsMap, BuildAgentSshKeyService buildAgentSSHKeyService) { + BuildJobManagementService buildJobManagementService, BuildLogsMap buildLogsMap, BuildAgentSshKeyService buildAgentSSHKeyService, TaskScheduler taskScheduler) { this.hazelcastInstance = hazelcastInstance; this.localCIBuildExecutorService = (ThreadPoolExecutor) localCIBuildExecutorService; this.buildJobManagementService = buildJobManagementService; this.buildLogsMap = buildLogsMap; this.buildAgentSSHKeyService = buildAgentSSHKeyService; + this.taskScheduler = taskScheduler; } /** @@ -101,6 +115,28 @@ public void init() { this.queue = this.hazelcastInstance.getQueue("buildJobQueue"); this.resultQueue = this.hazelcastInstance.getQueue("buildResultQueue"); this.listenerId = this.queue.addItemListener(new QueuedBuildJobItemListener(), true); + + /* + * Check every 10 seconds whether the node has at least one thread available for a new build job. + * If so, process the next build job. + * This is a backup mechanism in case the build queue is not empty, no new build jobs are entering the queue and the + * node otherwise stopped checking for build jobs in the queue. + */ + scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10)); + + ITopic pauseBuildAgentTopic = hazelcastInstance.getTopic("pauseBuildAgentTopic"); + pauseBuildAgentTopic.addMessageListener(message -> { + if (message.getMessageObject().equals(hazelcastInstance.getCluster().getLocalMember().getAddress().toString())) { + pauseBuildAgent(); + } + }); + + ITopic resumeBuildAgentTopic = hazelcastInstance.getTopic("resumeBuildAgentTopic"); + resumeBuildAgentTopic.addMessageListener(message -> { + if (message.getMessageObject().equals(hazelcastInstance.getCluster().getLocalMember().getAddress().toString())) { + resumeBuildAgent(); + } + }); } @PreDestroy @@ -127,17 +163,6 @@ public void updateBuildAgentInformation() { } } - /** - * Check every 10 seconds whether the node has at least one thread available for a new build job. - * If so, process the next build job. - * This is a backup mechanism in case the build queue is not empty, no new build jobs are entering the queue and the - * node otherwise stopped checking for build jobs in the queue. - */ - @Scheduled(fixedRate = 10000) - public void checkForBuildJobs() { - checkAvailabilityAndProcessNextBuild(); - } - /** * Checks whether the node has at least one thread available for a new build job. * If so, process the next build job. @@ -165,7 +190,7 @@ private void checkAvailabilityAndProcessNextBuild() { instanceLock.lock(); try { // Recheck conditions after acquiring the lock to ensure they are still valid - if (!nodeIsAvailable() || queue.isEmpty()) { + if (!nodeIsAvailable() || queue.isEmpty() || isPaused) { return; } @@ -350,6 +375,51 @@ private void processBuild(BuildJobQueueItem buildJob) { }); } + private void pauseBuildAgent() { + log.info("Pausing build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); + + this.isPaused = true; + this.removeListener(); + if (this.scheduledFuture != null && !this.scheduledFuture.isCancelled()) { + this.scheduledFuture.cancel(false); + } + + log.info("Gracefully cancelling running build jobs"); + + if (buildJobManagementService.getRunningBuildJobIds().isEmpty()) { + log.info("No running build jobs to cancel"); + } + else { + // Sleep for 10 seconds to allow the build jobs to be finished. If they are not finished, they will be cancelled. + try { + Thread.sleep(10000); + } + catch (InterruptedException e) { + log.error("Error while pausing build agent", e); + } + + if (!this.isPaused) { + log.info("Build agent was resumed before the build jobs could be cancelled"); + return; + } + + Set runningBuildJobIds = buildJobManagementService.getRunningBuildJobIds(); + List runningBuildJobs = processingJobs.getAll(runningBuildJobIds).values().stream().toList(); + runningBuildJobIds.forEach(buildJobManagementService::cancelBuildJob); + this.queue.addAll(runningBuildJobs); + log.info("Cancelled running build jobs and added them back to the queue with Ids {}", runningBuildJobIds); + log.debug("Cancelled running build jobs: {}", runningBuildJobs); + + } + } + + private void resumeBuildAgent() { + log.info("Resuming build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); + this.isPaused = false; + this.listenerId = this.queue.addItemListener(new QueuedBuildJobItemListener(), true); + this.scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10)); + } + /** * Checks whether the node has at least one thread available for a new build job. */ diff --git a/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java b/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java index bd1bcc4dea1b..137be1b5e8b3 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java @@ -15,6 +15,7 @@ import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; @@ -197,4 +198,46 @@ public ResponseEntity getBuildJobStatistics(@RequestPara BuildJobsStatisticsDTO buildJobStatistics = BuildJobsStatisticsDTO.of(buildJobResultCountDtos); return ResponseEntity.ok(buildJobStatistics); } + + /** + * {@code PUT /api/admin/agent/{agentName}/pause} : Pause the specified build agent. + * This endpoint allows administrators to pause a specific build agent by its name. + * Pausing a build agent will prevent it from picking up any new build jobs until it is resumed. + * + *

+ * Authorization: This operation requires admin privileges, enforced by {@code @EnforceAdmin}. + *

+ * + * @param agentName the name of the build agent to be paused (provided as a path variable) + * @return {@link ResponseEntity} with status code 204 (No Content) if the agent was successfully paused + * or an appropriate error response if something went wrong + */ + @PutMapping("agent/{agentName}/pause") + @EnforceAdmin + public ResponseEntity pauseBuildAgent(@PathVariable String agentName) { + log.debug("REST request to pause agent {}", agentName); + localCIBuildJobQueueService.pauseBuildAgent(agentName); + return ResponseEntity.noContent().build(); + } + + /** + * {@code PUT /api/admin/agent/{agentName}/resume} : Resume the specified build agent. + * This endpoint allows administrators to resume a specific build agent by its name. + * Resuming a build agent will allow it to pick up new build jobs again. + * + *

+ * Authorization: This operation requires admin privileges, enforced by {@code @EnforceAdmin}. + *

+ * + * @param agentName the name of the build agent to be resumed (provided as a path variable) + * @return {@link ResponseEntity} with status code 204 (No Content) if the agent was successfully resumed + * or an appropriate error response if something went wrong + */ + @PutMapping("agent/{agentName}/resume") + @EnforceAdmin + public ResponseEntity resumeBuildAgent(@PathVariable String agentName) { + log.debug("REST request to resume agent {}", agentName); + localCIBuildJobQueueService.resumeBuildAgent(agentName); + return ResponseEntity.noContent().build(); + } } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java index a99766da005b..9af5fe3b45c1 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localci/SharedQueueManagementService.java @@ -67,6 +67,10 @@ public class SharedQueueManagementService { private ITopic canceledBuildJobsTopic; + private ITopic pauseBuildAgentTopic; + + private ITopic resumeBuildAgentTopic; + public SharedQueueManagementService(BuildJobRepository buildJobRepository, @Qualifier("hazelcastInstance") HazelcastInstance hazelcastInstance, ProfileService profileService) { this.buildJobRepository = buildJobRepository; this.hazelcastInstance = hazelcastInstance; @@ -83,6 +87,8 @@ public void init() { this.queue = this.hazelcastInstance.getQueue("buildJobQueue"); this.canceledBuildJobsTopic = hazelcastInstance.getTopic("canceledBuildJobsTopic"); this.dockerImageCleanupInfo = this.hazelcastInstance.getMap("dockerImageCleanupInfo"); + this.pauseBuildAgentTopic = hazelcastInstance.getTopic("pauseBuildAgentTopic"); + this.resumeBuildAgentTopic = hazelcastInstance.getTopic("resumeBuildAgentTopic"); } /** @@ -135,6 +141,14 @@ public List getBuildAgentInformationWithoutRecentBuildJob agent.numberOfCurrentBuildJobs(), agent.runningBuildJobs(), agent.status(), null, null)).toList(); } + public void pauseBuildAgent(String agent) { + pauseBuildAgentTopic.publish(agent); + } + + public void resumeBuildAgent(String agent) { + resumeBuildAgentTopic.publish(agent); + } + /** * Cancel a build job by removing it from the queue or stopping the build process. * From cf62138880d7b8ca2fb5486a9192ed199ed605cb Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sat, 14 Sep 2024 17:40:23 +0200 Subject: [PATCH 02/24] validate build agent status before action --- .../service/SharedQueueProcessingService.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index 8f43cce7d9cf..37f5057ea143 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -376,6 +376,11 @@ private void processBuild(BuildJobQueueItem buildJob) { } private void pauseBuildAgent() { + if (this.isPaused) { + log.info("Build agent is already paused"); + return; + } + log.info("Pausing build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); this.isPaused = true; @@ -414,6 +419,11 @@ private void pauseBuildAgent() { } private void resumeBuildAgent() { + if (!this.isPaused) { + log.info("Build agent is already running"); + return; + } + log.info("Resuming build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); this.isPaused = false; this.listenerId = this.queue.addItemListener(new QueuedBuildJobItemListener(), true); From 7fc872abc776ebd496d037b4bf9c343bf61e9dda Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 15 Sep 2024 06:47:19 +0200 Subject: [PATCH 03/24] add client buttons --- .../build-agent-details.component.html | 14 ++++-- .../build-agent-details.component.ts | 50 +++++++++++++++++++ .../build-agents/build-agents.service.ts | 24 +++++++++ src/main/webapp/i18n/de/buildAgents.json | 11 +++- src/main/webapp/i18n/en/buildAgents.json | 11 +++- 5 files changed, 104 insertions(+), 6 deletions(-) diff --git a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html index ea1b2da0f1d2..1e8a4b7e4f34 100644 --- a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html +++ b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html @@ -1,10 +1,16 @@
@if (buildAgent) {
-
-

- : -

{{ buildAgent.name }}

+
+
+

+ : +

{{ buildAgent.name }}

+
+
+ + +
diff --git a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts index 074c1e704695..00164ebe4958 100644 --- a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts +++ b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts @@ -8,6 +8,7 @@ import { TriggeredByPushTo } from 'app/entities/programming/repository-info.mode import { ActivatedRoute } from '@angular/router'; import { JhiWebsocketService } from 'app/core/websocket/websocket.service'; import { BuildQueueService } from 'app/localci/build-queue/build-queue.service'; +import { AlertService, AlertType } from 'app/core/util/alert.service'; @Component({ selector: 'jhi-build-agent-details', @@ -34,6 +35,7 @@ export class BuildAgentDetailsComponent implements OnInit, OnDestroy { private buildAgentsService: BuildAgentsService, private route: ActivatedRoute, private buildQueueService: BuildQueueService, + private alertService: AlertService, ) {} ngOnInit() { @@ -105,4 +107,52 @@ export class BuildAgentDetailsComponent implements OnInit, OnDestroy { const url = `/api/build-log/${resultId}`; window.open(url, '_blank'); } + + pauseBuildAgent() { + if (this.buildAgent.name) { + this.buildAgentsService.pauseBuildAgent(this.buildAgent.name).subscribe({ + next: () => { + this.alertService.addAlert({ + type: AlertType.SUCCESS, + message: 'artemisApp.buildAgents.alerts.buildAgentPaused', + }); + }, + error: () => { + this.alertService.addAlert({ + type: AlertType.DANGER, + message: 'artemisApp.buildAgents.alerts.buildAgentPauseFailed', + }); + }, + }); + } else { + this.alertService.addAlert({ + type: AlertType.WARNING, + message: 'artemisApp.buildAgents.alerts.buildAgentWithoutName', + }); + } + } + + resumeBuildAgent() { + if (this.buildAgent.name) { + this.buildAgentsService.resumeBuildAgent(this.buildAgent.name).subscribe({ + next: () => { + this.alertService.addAlert({ + type: AlertType.SUCCESS, + message: 'artemisApp.buildAgents.alerts.buildAgentResumed', + }); + }, + error: () => { + this.alertService.addAlert({ + type: AlertType.DANGER, + message: 'artemisApp.buildAgents.alerts.buildAgentResumeFailed', + }); + }, + }); + } else { + this.alertService.addAlert({ + type: AlertType.WARNING, + message: 'artemisApp.buildAgents.alerts.buildAgentWithoutName', + }); + } + } } diff --git a/src/main/webapp/app/localci/build-agents/build-agents.service.ts b/src/main/webapp/app/localci/build-agents/build-agents.service.ts index 99905ad80c51..b701b85fe5d0 100644 --- a/src/main/webapp/app/localci/build-agents/build-agents.service.ts +++ b/src/main/webapp/app/localci/build-agents/build-agents.service.ts @@ -27,4 +27,28 @@ export class BuildAgentsService { }), ); } + + /** + * Pause Build Agent + */ + pauseBuildAgent(agentName: string): Observable { + const encodedAgentName = encodeURIComponent(agentName); + return this.http.put(`${this.adminResourceUrl}/agent/${encodedAgentName}/pause`, null).pipe( + catchError((err) => { + return throwError(() => new Error(`Failed to pause build agent ${agentName}\n${err.message}`)); + }), + ); + } + + /** + * Resume Build Agent + */ + resumeBuildAgent(agentName: string): Observable { + const encodedAgentName = encodeURIComponent(agentName); + return this.http.put(`${this.adminResourceUrl}/agent/${encodedAgentName}/resume`, null).pipe( + catchError((err) => { + return throwError(() => new Error(`Failed to resume build agent ${agentName}\n${err.message}`)); + }), + ); + } } diff --git a/src/main/webapp/i18n/de/buildAgents.json b/src/main/webapp/i18n/de/buildAgents.json index cabe778eee21..bc278dd51f50 100644 --- a/src/main/webapp/i18n/de/buildAgents.json +++ b/src/main/webapp/i18n/de/buildAgents.json @@ -14,7 +14,16 @@ "idle": "Inaktiv", "onlineAgents": "online Agent(en)", "of": "von", - "buildJobsRunning": "Build Jobs laufen" + "buildJobsRunning": "Build Jobs laufen", + "alerts": { + "buildAgentPaused": "Anfrage zum Anhalten des Build-Agenten erfolgreich gesendet. Der Agent akzeptiert keine neuen BuildJobs und wird die aktuellen BuildJobs entweder ordnungsgemäß beenden oder nach 10 Sekunden abbrechen.", + "buildAgentResumed": "Anfrage zum Fortsetzen des BuildJobs erfolgreich gesendet. Der Agent wird wieder neue BuildJobs annehmen.", + "buildAgentPausedFailed": "Anhalten des Build-Agenten fehlgeschlagen.", + "buildAgentResumedFailed": "Fortsetzen des Build-Agenten fehlgeschlagen.", + "buildAgentWithoutName": "Der Name des Build-Agenten ist erforderlich." + }, + "pause": "Anhalten", + "resume": "Fortsetzen" } } } diff --git a/src/main/webapp/i18n/en/buildAgents.json b/src/main/webapp/i18n/en/buildAgents.json index 195a549cda0f..ef71b57e7422 100644 --- a/src/main/webapp/i18n/en/buildAgents.json +++ b/src/main/webapp/i18n/en/buildAgents.json @@ -14,7 +14,16 @@ "idle": "Idle", "onlineAgents": "online agent(s)", "of": "of", - "buildJobsRunning": "build jobs running" + "buildJobsRunning": "build jobs running", + "alerts": { + "buildAgentPaused": "Build agent pause request sent successfully. The agent will not accept new build jobs and will gracefully finish the current build jobs or will cancel them after 10 seconds.", + "buildAgentResumed": "Build agent resume request sent successfully. The agent will start accepting new build jobs.", + "buildAgentPausedFailed": "Failed to pause the build agent.", + "buildAgentResumedFailed": "Failed to resume the build agent.", + "buildAgentWithoutName": "Build agent name is required." + }, + "pause": "Pause", + "resume": "Resume" } } } From 27a83aad086c3fcedfc838670efacc29b28e909a Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 15 Sep 2024 18:22:50 +0200 Subject: [PATCH 04/24] update build agent status in ui --- .../buildagent/dto/BuildAgentInformation.java | 8 +++++-- .../service/SharedQueueProcessingService.java | 8 +++++-- .../entities/programming/build-agent.model.ts | 8 ++++++- .../build-agent-details.component.html | 24 ++++++++++++++----- .../build-agent-details.component.ts | 4 +++- .../build-agent-summary.component.html | 14 +++++++---- src/main/webapp/i18n/de/buildAgents.json | 1 + src/main/webapp/i18n/en/buildAgents.json | 1 + .../LocalCIResourceIntegrationTest.java | 2 +- .../build-agent-details.component.spec.ts | 4 ++-- .../build-agent-summary.component.spec.ts | 6 ++--- 11 files changed, 58 insertions(+), 22 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildAgentInformation.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildAgentInformation.java index 35625765d858..ab24012f51fa 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildAgentInformation.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/dto/BuildAgentInformation.java @@ -11,8 +11,8 @@ // in the future are migrated or cleared. Changes should be communicated in release notes as potentially breaking changes. @JsonIgnoreProperties(ignoreUnknown = true) @JsonInclude(JsonInclude.Include.NON_EMPTY) -public record BuildAgentInformation(String name, int maxNumberOfConcurrentBuildJobs, int numberOfCurrentBuildJobs, List runningBuildJobs, boolean status, - List recentBuildJobs, String publicSshKey) implements Serializable { +public record BuildAgentInformation(String name, int maxNumberOfConcurrentBuildJobs, int numberOfCurrentBuildJobs, List runningBuildJobs, + BuildAgentStatus status, List recentBuildJobs, String publicSshKey) implements Serializable { @Serial private static final long serialVersionUID = 1L; @@ -27,4 +27,8 @@ public BuildAgentInformation(BuildAgentInformation agentInformation, List processingJobsOfMember = getProcessingJobsOfNode(memberAddress); int numberOfCurrentBuildJobs = processingJobsOfMember.size(); int maxNumberOfConcurrentBuilds = localCIBuildExecutorService.getMaximumPoolSize(); - boolean active = numberOfCurrentBuildJobs > 0; + boolean hasJobs = numberOfCurrentBuildJobs > 0; + BuildAgentInformation.BuildAgentStatus status = isPaused ? BuildAgentInformation.BuildAgentStatus.PAUSED + : hasJobs ? BuildAgentInformation.BuildAgentStatus.ACTIVE : BuildAgentInformation.BuildAgentStatus.IDLE; BuildAgentInformation agent = buildAgentInformation.get(memberAddress); List recentBuildJobs; if (agent != null) { @@ -285,7 +287,7 @@ private BuildAgentInformation getUpdatedLocalBuildAgentInformation(BuildJobQueue String publicSshKey = buildAgentSSHKeyService.getPublicKeyAsString(); - return new BuildAgentInformation(memberAddress, maxNumberOfConcurrentBuilds, numberOfCurrentBuildJobs, processingJobsOfMember, active, recentBuildJobs, publicSshKey); + return new BuildAgentInformation(memberAddress, maxNumberOfConcurrentBuilds, numberOfCurrentBuildJobs, processingJobsOfMember, status, recentBuildJobs, publicSshKey); } private List getProcessingJobsOfNode(String memberAddress) { @@ -388,6 +390,7 @@ private void pauseBuildAgent() { if (this.scheduledFuture != null && !this.scheduledFuture.isCancelled()) { this.scheduledFuture.cancel(false); } + updateLocalBuildAgentInformation(); log.info("Gracefully cancelling running build jobs"); @@ -428,6 +431,7 @@ private void resumeBuildAgent() { this.isPaused = false; this.listenerId = this.queue.addItemListener(new QueuedBuildJobItemListener(), true); this.scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10)); + updateLocalBuildAgentInformation(); } /** diff --git a/src/main/webapp/app/entities/programming/build-agent.model.ts b/src/main/webapp/app/entities/programming/build-agent.model.ts index 172a8596b299..86c51ffc8b35 100644 --- a/src/main/webapp/app/entities/programming/build-agent.model.ts +++ b/src/main/webapp/app/entities/programming/build-agent.model.ts @@ -1,12 +1,18 @@ import { BaseEntity } from 'app/shared/model/base-entity'; import { BuildJob } from 'app/entities/programming/build-job.model'; +export enum BuildAgentStatus { + ACTIVE = 'ACTIVE', + PAUSED = 'PAUSED', + IDLE = 'IDLE', +} + export class BuildAgent implements BaseEntity { public id?: number; public name?: string; public maxNumberOfConcurrentBuildJobs?: number; public numberOfCurrentBuildJobs?: number; public runningBuildJobs?: BuildJob[]; - public status?: boolean; + public status?: BuildAgentStatus; public recentBuildJobs?: BuildJob[]; } diff --git a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html index 1e8a4b7e4f34..94ea21ead492 100644 --- a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html +++ b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html @@ -8,8 +8,14 @@

{{ buildAgent.name }}

- - + +
@@ -34,10 +40,16 @@

{{ buildAgent.nam - @if (value) { - - } @else { - + @switch (value) { + @case ('ACTIVE') { + + } + @case ('IDLE') { + + } + @case ('PAUSED') { + + } } diff --git a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts index 00164ebe4958..25214e2f6fe5 100644 --- a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts +++ b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts @@ -2,7 +2,7 @@ import { Component, OnDestroy, OnInit } from '@angular/core'; import { BuildAgent } from 'app/entities/programming/build-agent.model'; import { BuildAgentsService } from 'app/localci/build-agents/build-agents.service'; import { Subscription } from 'rxjs'; -import { faCircleCheck, faExclamationCircle, faExclamationTriangle, faTimes } from '@fortawesome/free-solid-svg-icons'; +import { faCircleCheck, faExclamationCircle, faExclamationTriangle, faPause, faPlay, faTimes } from '@fortawesome/free-solid-svg-icons'; import dayjs from 'dayjs/esm'; import { TriggeredByPushTo } from 'app/entities/programming/repository-info.model'; import { ActivatedRoute } from '@angular/router'; @@ -29,6 +29,8 @@ export class BuildAgentDetailsComponent implements OnInit, OnDestroy { faExclamationCircle = faExclamationCircle; faExclamationTriangle = faExclamationTriangle; faTimes = faTimes; + faPause = faPause; + faPlay = faPlay; constructor( private websocketService: JhiWebsocketService, diff --git a/src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.html b/src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.html index a9b878d14d33..0982463d75a9 100644 --- a/src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.html +++ b/src/main/webapp/app/localci/build-agents/build-agent-summary/build-agent-summary.component.html @@ -41,10 +41,16 @@

- @if (value) { - - } @else { - + @switch (value) { + @case ('ACTIVE') { + + } + @case ('IDLE') { + + } + @case ('PAUSED') { + + } } diff --git a/src/main/webapp/i18n/de/buildAgents.json b/src/main/webapp/i18n/de/buildAgents.json index bc278dd51f50..b5e7f75575c8 100644 --- a/src/main/webapp/i18n/de/buildAgents.json +++ b/src/main/webapp/i18n/de/buildAgents.json @@ -12,6 +12,7 @@ "recentBuildJobs": "Letzte Build Jobs", "running": "Laufend", "idle": "Inaktiv", + "paused": "Angehalten", "onlineAgents": "online Agent(en)", "of": "von", "buildJobsRunning": "Build Jobs laufen", diff --git a/src/main/webapp/i18n/en/buildAgents.json b/src/main/webapp/i18n/en/buildAgents.json index ef71b57e7422..f8b5c4689dbf 100644 --- a/src/main/webapp/i18n/en/buildAgents.json +++ b/src/main/webapp/i18n/en/buildAgents.json @@ -12,6 +12,7 @@ "recentBuildJobs": "Recent build jobs", "running": "Running", "idle": "Idle", + "paused": "Paused", "onlineAgents": "online agent(s)", "of": "of", "buildJobsRunning": "build jobs running", diff --git a/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIResourceIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIResourceIntegrationTest.java index 0a553702716a..36c5aa078f7c 100644 --- a/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIResourceIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIResourceIntegrationTest.java @@ -88,7 +88,7 @@ void createJobs() { job1 = new BuildJobQueueItem("1", "job1", "address1", 1, course.getId(), 1, 1, 1, BuildStatus.SUCCESSFUL, repositoryInfo, jobTimingInfo1, buildConfig, null); job2 = new BuildJobQueueItem("2", "job2", "address1", 2, course.getId(), 1, 1, 1, BuildStatus.SUCCESSFUL, repositoryInfo, jobTimingInfo2, buildConfig, null); String memberAddress = hazelcastInstance.getCluster().getLocalMember().getAddress().toString(); - agent1 = new BuildAgentInformation(memberAddress, 1, 0, new ArrayList<>(List.of(job1)), false, new ArrayList<>(List.of(job2)), null); + agent1 = new BuildAgentInformation(memberAddress, 1, 0, new ArrayList<>(List.of(job1)), BuildAgentInformation.BuildAgentStatus.IDLE, new ArrayList<>(List.of(job2)), null); BuildJobQueueItem finishedJobQueueItem1 = new BuildJobQueueItem("3", "job3", "address1", 3, course.getId(), 1, 1, 1, BuildStatus.SUCCESSFUL, repositoryInfo, jobTimingInfo1, buildConfig, null); BuildJobQueueItem finishedJobQueueItem2 = new BuildJobQueueItem("4", "job4", "address1", 4, course.getId() + 1, 1, 1, 1, BuildStatus.FAILED, repositoryInfo, jobTimingInfo2, diff --git a/src/test/javascript/spec/component/localci/build-agents/build-agent-details.component.spec.ts b/src/test/javascript/spec/component/localci/build-agents/build-agent-details.component.spec.ts index 41ab0dfcea81..35733b12ec34 100644 --- a/src/test/javascript/spec/component/localci/build-agents/build-agent-details.component.spec.ts +++ b/src/test/javascript/spec/component/localci/build-agents/build-agent-details.component.spec.ts @@ -9,7 +9,7 @@ import { ArtemisTranslatePipe } from 'app/shared/pipes/artemis-translate.pipe'; import { DataTableComponent } from 'app/shared/data-table/data-table.component'; import { MockComponent, MockPipe } from 'ng-mocks'; import { NgxDatatableModule } from '@flaviosantoro92/ngx-datatable'; -import { BuildAgent } from 'app/entities/programming/build-agent.model'; +import { BuildAgent, BuildAgentStatus } from 'app/entities/programming/build-agent.model'; import { RepositoryInfo, TriggeredByPushTo } from 'app/entities/programming/repository-info.model'; import { JobTimingInfo } from 'app/entities/job-timing-info.model'; import { BuildConfig } from 'app/entities/programming/build-config.model'; @@ -114,7 +114,7 @@ describe('BuildAgentDetailsComponent', () => { numberOfCurrentBuildJobs: 2, runningBuildJobs: mockRunningJobs1, recentBuildJobs: mockRecentBuildJobs1, - status: true, + status: BuildAgentStatus.ACTIVE, }; beforeEach(waitForAsync(() => { diff --git a/src/test/javascript/spec/component/localci/build-agents/build-agent-summary.component.spec.ts b/src/test/javascript/spec/component/localci/build-agents/build-agent-summary.component.spec.ts index 76a76f16c094..24b91532a43f 100644 --- a/src/test/javascript/spec/component/localci/build-agents/build-agent-summary.component.spec.ts +++ b/src/test/javascript/spec/component/localci/build-agents/build-agent-summary.component.spec.ts @@ -10,7 +10,7 @@ import { ArtemisTranslatePipe } from 'app/shared/pipes/artemis-translate.pipe'; import { DataTableComponent } from 'app/shared/data-table/data-table.component'; import { MockComponent, MockPipe } from 'ng-mocks'; import { NgxDatatableModule } from '@flaviosantoro92/ngx-datatable'; -import { BuildAgent } from 'app/entities/programming/build-agent.model'; +import { BuildAgent, BuildAgentStatus } from 'app/entities/programming/build-agent.model'; import { RepositoryInfo, TriggeredByPushTo } from 'app/entities/programming/repository-info.model'; import { JobTimingInfo } from 'app/entities/job-timing-info.model'; import { BuildConfig } from 'app/entities/programming/build-config.model'; @@ -124,7 +124,7 @@ describe('BuildAgentSummaryComponent', () => { maxNumberOfConcurrentBuildJobs: 2, numberOfCurrentBuildJobs: 2, runningBuildJobs: mockRunningJobs1, - status: true, + status: BuildAgentStatus.ACTIVE, }, { id: 2, @@ -132,7 +132,7 @@ describe('BuildAgentSummaryComponent', () => { maxNumberOfConcurrentBuildJobs: 2, numberOfCurrentBuildJobs: 2, runningBuildJobs: mockRunningJobs2, - status: true, + status: BuildAgentStatus.ACTIVE, }, ]; From 0ccc12584287771d5709a1154dc43a0438177826 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 15 Sep 2024 18:46:34 +0200 Subject: [PATCH 05/24] add client tests --- .../build-agent-details.component.spec.ts | 75 ++++++++++++++++++- .../build-agents/build-agents.service.spec.ts | 16 ++++ 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/test/javascript/spec/component/localci/build-agents/build-agent-details.component.spec.ts b/src/test/javascript/spec/component/localci/build-agents/build-agent-details.component.spec.ts index 35733b12ec34..7b403995c8af 100644 --- a/src/test/javascript/spec/component/localci/build-agents/build-agent-details.component.spec.ts +++ b/src/test/javascript/spec/component/localci/build-agents/build-agent-details.component.spec.ts @@ -1,13 +1,13 @@ import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { JhiWebsocketService } from 'app/core/websocket/websocket.service'; import { BuildAgentsService } from 'app/localci/build-agents/build-agents.service'; -import { of } from 'rxjs'; +import { of, throwError } from 'rxjs'; import { BuildJob } from 'app/entities/programming/build-job.model'; import dayjs from 'dayjs/esm'; import { ArtemisTestModule } from '../../../test.module'; import { ArtemisTranslatePipe } from 'app/shared/pipes/artemis-translate.pipe'; import { DataTableComponent } from 'app/shared/data-table/data-table.component'; -import { MockComponent, MockPipe } from 'ng-mocks'; +import { MockComponent, MockPipe, MockProvider } from 'ng-mocks'; import { NgxDatatableModule } from '@flaviosantoro92/ngx-datatable'; import { BuildAgent, BuildAgentStatus } from 'app/entities/programming/build-agent.model'; import { RepositoryInfo, TriggeredByPushTo } from 'app/entities/programming/repository-info.model'; @@ -16,6 +16,7 @@ import { BuildConfig } from 'app/entities/programming/build-config.model'; import { BuildAgentDetailsComponent } from 'app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component'; import { MockActivatedRoute } from '../../../helpers/mocks/activated-route/mock-activated-route'; import { ActivatedRoute } from '@angular/router'; +import { AlertService, AlertType } from 'app/core/util/alert.service'; describe('BuildAgentDetailsComponent', () => { let component: BuildAgentDetailsComponent; @@ -30,6 +31,8 @@ describe('BuildAgentDetailsComponent', () => { const mockBuildAgentsService = { getBuildAgentDetails: jest.fn().mockReturnValue(of([])), + pauseBuildAgent: jest.fn().mockReturnValue(of({})), + resumeBuildAgent: jest.fn().mockReturnValue(of({})), }; const repositoryInfo: RepositoryInfo = { @@ -117,6 +120,9 @@ describe('BuildAgentDetailsComponent', () => { status: BuildAgentStatus.ACTIVE, }; + let alertService: AlertService; + let alertServiceAddAlertStub: jest.SpyInstance; + beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ imports: [ArtemisTestModule, NgxDatatableModule], @@ -126,6 +132,7 @@ describe('BuildAgentDetailsComponent', () => { { provide: ActivatedRoute, useValue: new MockActivatedRoute({ key: 'ABC123' }) }, { provide: BuildAgentsService, useValue: mockBuildAgentsService }, { provide: DataTableComponent, useClass: DataTableComponent }, + MockProvider(AlertService), ], }).compileComponents(); @@ -133,6 +140,8 @@ describe('BuildAgentDetailsComponent', () => { component = fixture.componentInstance; activatedRoute = fixture.debugElement.injector.get(ActivatedRoute) as MockActivatedRoute; activatedRoute.setParameters({ agentName: mockBuildAgent.name }); + alertService = TestBed.inject(AlertService); + alertServiceAddAlertStub = jest.spyOn(alertService, 'addAlert'); })); beforeEach(() => { @@ -202,4 +211,66 @@ describe('BuildAgentDetailsComponent', () => { expect(spy).toHaveBeenCalledOnce(); }); + + it('should show an alert when pausing build agent without a name', () => { + component.buildAgent = { ...mockBuildAgent, name: '' }; + component.pauseBuildAgent(); + + expect(alertServiceAddAlertStub).toHaveBeenCalledWith({ + type: AlertType.WARNING, + message: 'artemisApp.buildAgents.alerts.buildAgentWithoutName', + }); + }); + + it('should show an alert when resuming build agent without a name', () => { + component.buildAgent = { ...mockBuildAgent, name: '' }; + component.resumeBuildAgent(); + + expect(alertServiceAddAlertStub).toHaveBeenCalledWith({ + type: AlertType.WARNING, + message: 'artemisApp.buildAgents.alerts.buildAgentWithoutName', + }); + }); + + it('should show success alert when pausing build agent', () => { + component.buildAgent = mockBuildAgent; + + component.pauseBuildAgent(); + expect(alertServiceAddAlertStub).toHaveBeenCalledWith({ + type: AlertType.SUCCESS, + message: 'artemisApp.buildAgents.alerts.buildAgentPaused', + }); + }); + + it('should show error alert when pausing build agent fails', () => { + mockBuildAgentsService.pauseBuildAgent.mockReturnValue(throwError(() => new Error())); + component.buildAgent = mockBuildAgent; + + component.pauseBuildAgent(); + expect(alertServiceAddAlertStub).toHaveBeenCalledWith({ + type: AlertType.DANGER, + message: 'artemisApp.buildAgents.alerts.buildAgentPauseFailed', + }); + }); + + it('should show success alert when resuming build agent', () => { + component.buildAgent = mockBuildAgent; + + component.resumeBuildAgent(); + expect(alertServiceAddAlertStub).toHaveBeenCalledWith({ + type: AlertType.SUCCESS, + message: 'artemisApp.buildAgents.alerts.buildAgentResumed', + }); + }); + + it('should show error alert when resuming build agent fails', () => { + mockBuildAgentsService.resumeBuildAgent.mockReturnValue(throwError(() => new Error())); + component.buildAgent = mockBuildAgent; + + component.resumeBuildAgent(); + expect(alertServiceAddAlertStub).toHaveBeenCalledWith({ + type: AlertType.DANGER, + message: 'artemisApp.buildAgents.alerts.buildAgentResumeFailed', + }); + }); }); diff --git a/src/test/javascript/spec/component/localci/build-agents/build-agents.service.spec.ts b/src/test/javascript/spec/component/localci/build-agents/build-agents.service.spec.ts index 494f211ee2b3..9ad5c6b8246e 100644 --- a/src/test/javascript/spec/component/localci/build-agents/build-agents.service.spec.ts +++ b/src/test/javascript/spec/component/localci/build-agents/build-agents.service.spec.ts @@ -112,6 +112,22 @@ describe('BuildAgentsService', () => { req.flush(expectedResponse); }); + it('should pause build agent', () => { + service.pauseBuildAgent('buildAgent1').subscribe(); + + const req = httpMock.expectOne(`${service.adminResourceUrl}/agent/buildAgent1/pause`); + expect(req.request.method).toBe('PUT'); + req.flush({}); + }); + + it('should resume build agent', () => { + service.resumeBuildAgent('buildAgent1').subscribe(); + + const req = httpMock.expectOne(`${service.adminResourceUrl}/agent/buildAgent1/resume`); + expect(req.request.method).toBe('PUT'); + req.flush({}); + }); + afterEach(() => { httpMock.verify(); // Verify that there are no outstanding requests. }); From 70c3029a43c117bb9b4a4c79574a7390d643e8ab Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 15 Sep 2024 22:18:21 +0200 Subject: [PATCH 06/24] add server tests --- .../service/SharedQueueProcessingService.java | 1 + .../localvcci/LocalCIIntegrationTest.java | 30 +++++++++++++++++++ .../LocalCIResourceIntegrationTest.java | 13 ++++++++ 3 files changed, 44 insertions(+) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index aeb1be16f80a..6507944b2081 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -431,6 +431,7 @@ private void resumeBuildAgent() { this.isPaused = false; this.listenerId = this.queue.addItemListener(new QueuedBuildJobItemListener(), true); this.scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10)); + checkAvailabilityAndProcessNextBuild(); updateLocalBuildAgentInformation(); } diff --git a/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIIntegrationTest.java index 10fea95376be..f508ce3b2c73 100644 --- a/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIIntegrationTest.java @@ -39,6 +39,7 @@ import org.mockito.ArgumentMatcher; import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.beans.factory.annotation.Value; import org.springframework.core.io.FileSystemResource; import org.springframework.http.HttpStatus; @@ -49,8 +50,12 @@ import com.github.dockerjava.api.command.ExecStartCmd; import com.github.dockerjava.api.exception.NotFoundException; import com.github.dockerjava.api.model.Frame; +import com.hazelcast.collection.IQueue; +import com.hazelcast.core.HazelcastInstance; +import com.hazelcast.map.IMap; import de.tum.cit.aet.artemis.assessment.domain.Result; +import de.tum.cit.aet.artemis.buildagent.dto.BuildJobQueueItem; import de.tum.cit.aet.artemis.buildagent.dto.ResultBuildJob; import de.tum.cit.aet.artemis.core.exception.VersionControlException; import de.tum.cit.aet.artemis.core.repository.ProgrammingSubmissionTestRepository; @@ -81,6 +86,10 @@ class LocalCIIntegrationTest extends AbstractLocalCILocalVCIntegrationTest { @Autowired private BuildLogEntryService buildLogEntryService; + @Autowired + @Qualifier("hazelcastInstance") + private HazelcastInstance hazelcastInstance; + private LocalRepository studentAssignmentRepository; private LocalRepository testsRepository; @@ -470,4 +479,25 @@ private void verifyUserNotification(ProgrammingExerciseStudentParticipation part return true; }), Mockito.eq(participation))); } + + @Test + @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") + void testPauseAndResumeBuildAgent() { + String memberAddress = hazelcastInstance.getCluster().getLocalMember().getAddress().toString(); + hazelcastInstance.getTopic("pauseBuildAgentTopic").publish(memberAddress); + + ProgrammingExerciseStudentParticipation studentParticipation = localVCLocalCITestService.createParticipation(programmingExercise, student1Login); + + localVCServletService.processNewPush(commitHash, studentAssignmentRepository.originGit.getRepository()); + await().until(() -> { + IQueue buildQueue = hazelcastInstance.getQueue("buildJobQueue"); + IMap buildJobMap = hazelcastInstance.getMap("processingJobs"); + BuildJobQueueItem buildJobQueueItem = buildQueue.peek(); + + return buildJobQueueItem != null && buildJobQueueItem.buildConfig().commitHashToBuild().equals(commitHash) && !buildJobMap.containsKey(buildJobQueueItem.id()); + }); + + hazelcastInstance.getTopic("resumeBuildAgentTopic").publish(memberAddress); + localVCLocalCITestService.testLatestSubmission(studentParticipation.getId(), commitHash, 1, false); + } } diff --git a/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIResourceIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIResourceIntegrationTest.java index 36c5aa078f7c..add565296490 100644 --- a/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIResourceIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/localvcci/LocalCIResourceIntegrationTest.java @@ -1,7 +1,10 @@ package de.tum.cit.aet.artemis.localvcci; import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.time.ZonedDateTime; @@ -345,4 +348,14 @@ void testGetBuildJobStatistics() throws Exception { assertThat(response.failedBuilds()).isEqualTo(1); assertThat(response.cancelledBuilds()).isEqualTo(0); } + + @Test + @WithMockUser(username = TEST_PREFIX + "admin", roles = "ADMIN") + void testPauseBuildAgent() throws Exception { + request.put("/api/admin/agent/" + URLEncoder.encode(agent1.name(), StandardCharsets.UTF_8) + "/pause", null, HttpStatus.NO_CONTENT); + await().until(() -> buildAgentInformation.get(agent1.name()).status() == BuildAgentInformation.BuildAgentStatus.PAUSED); + + request.put("/api/admin/agent/" + URLEncoder.encode(agent1.name(), StandardCharsets.UTF_8) + "/resume", null, HttpStatus.NO_CONTENT); + await().until(() -> buildAgentInformation.get(agent1.name()).status() == BuildAgentInformation.BuildAgentStatus.IDLE); + } } From 991ac594185297146acdb7b285f6505697fd5bd1 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Tue, 17 Sep 2024 23:11:14 +0200 Subject: [PATCH 07/24] dont save result after grace period --- .../service/SharedQueueProcessingService.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index 6507944b2081..afc63ad607b4 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -95,6 +95,8 @@ public class SharedQueueProcessingService { private boolean isPaused = false; + private boolean processResults = true; + public SharedQueueProcessingService(@Qualifier("hazelcastInstance") HazelcastInstance hazelcastInstance, ExecutorService localCIBuildExecutorService, BuildJobManagementService buildJobManagementService, BuildLogsMap buildLogsMap, BuildAgentSshKeyService buildAgentSSHKeyService, TaskScheduler taskScheduler) { this.hazelcastInstance = hazelcastInstance; @@ -331,7 +333,12 @@ private void processBuild(BuildJobQueueItem buildJob) { buildLogsMap.removeBuildLogs(buildJob.id()); ResultQueueItem resultQueueItem = new ResultQueueItem(buildResult, finishedJob, buildLogs, null); - resultQueue.add(resultQueueItem); + if (processResults) { + resultQueue.add(resultQueueItem); + } + else { + log.info("Build agent is paused. Not adding build result to result queue for build job: {}", buildJob); + } // after processing a build job, remove it from the processing jobs processingJobs.remove(buildJob.id()); @@ -366,7 +373,12 @@ private void processBuild(BuildJobQueueItem buildJob) { failedResult.setBuildLogEntries(buildLogs); ResultQueueItem resultQueueItem = new ResultQueueItem(failedResult, job, buildLogs, ex); - resultQueue.add(resultQueueItem); + if (processResults) { + resultQueue.add(resultQueueItem); + } + else { + log.info("Build agent is paused. Not adding build result to result queue for build job: {}", buildJob); + } processingJobs.remove(buildJob.id()); localProcessingJobs.decrementAndGet(); @@ -411,6 +423,7 @@ private void pauseBuildAgent() { return; } + this.processResults = false; Set runningBuildJobIds = buildJobManagementService.getRunningBuildJobIds(); List runningBuildJobs = processingJobs.getAll(runningBuildJobIds).values().stream().toList(); runningBuildJobIds.forEach(buildJobManagementService::cancelBuildJob); @@ -429,6 +442,7 @@ private void resumeBuildAgent() { log.info("Resuming build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); this.isPaused = false; + this.processResults = true; this.listenerId = this.queue.addItemListener(new QueuedBuildJobItemListener(), true); this.scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10)); checkAvailabilityAndProcessNextBuild(); From 2a9cc33f59f4bf2d4a69f3f0f230ac09e7e2f40e Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sat, 21 Sep 2024 22:14:48 +0200 Subject: [PATCH 08/24] make grace period configurable --- .../service/SharedQueueProcessingService.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index afc63ad607b4..7e5e6efd129a 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -25,6 +25,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Profile; import org.springframework.scheduling.TaskScheduler; import org.springframework.scheduling.annotation.Scheduled; @@ -93,9 +94,12 @@ public class SharedQueueProcessingService { */ private ScheduledFuture scheduledFuture; - private boolean isPaused = false; + private volatile boolean isPaused = false; - private boolean processResults = true; + private volatile boolean processResults = true; + + @Value("${artemis.continuous-integration.pause-grace-period-seconds:15}") + private int pauseGracePeriodSeconds; public SharedQueueProcessingService(@Qualifier("hazelcastInstance") HazelcastInstance hazelcastInstance, ExecutorService localCIBuildExecutorService, BuildJobManagementService buildJobManagementService, BuildLogsMap buildLogsMap, BuildAgentSshKeyService buildAgentSSHKeyService, TaskScheduler taskScheduler) { @@ -412,7 +416,7 @@ private void pauseBuildAgent() { else { // Sleep for 10 seconds to allow the build jobs to be finished. If they are not finished, they will be cancelled. try { - Thread.sleep(10000); + Thread.sleep(pauseGracePeriodSeconds * 1000L); } catch (InterruptedException e) { log.error("Error while pausing build agent", e); @@ -443,7 +447,12 @@ private void resumeBuildAgent() { log.info("Resuming build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); this.isPaused = false; this.processResults = true; + // We remove the listener and scheduledTask first to avoid race conditions + this.removeListener(); this.listenerId = this.queue.addItemListener(new QueuedBuildJobItemListener(), true); + if (this.scheduledFuture != null && !this.scheduledFuture.isCancelled()) { + this.scheduledFuture.cancel(false); + } this.scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10)); checkAvailabilityAndProcessNextBuild(); updateLocalBuildAgentInformation(); From cc960b79a4c753da23214e46e547bff1e0fb2dc4 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 22 Sep 2024 13:48:39 +0200 Subject: [PATCH 09/24] dont check if agent is paused --- .../buildagent/service/SharedQueueProcessingService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index 7e5e6efd129a..b084160d6caf 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -189,7 +189,7 @@ private void checkAvailabilityAndProcessNextBuild() { return; } - if (queue.isEmpty()) { + if (queue.isEmpty() || isPaused) { return; } BuildJobQueueItem buildJob = null; From fe6ba08935993b3b20bfbccfbed033a5b32342e3 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 22 Sep 2024 17:20:06 +0200 Subject: [PATCH 10/24] feedback --- .../service/SharedQueueProcessingService.java | 34 +++++++++---------- src/main/webapp/i18n/de/buildAgents.json | 6 ++-- src/main/webapp/i18n/en/buildAgents.json | 6 ++-- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index b084160d6caf..d53669eb2e96 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -394,17 +394,17 @@ private void processBuild(BuildJobQueueItem buildJob) { } private void pauseBuildAgent() { - if (this.isPaused) { + if (isPaused) { log.info("Build agent is already paused"); return; } log.info("Pausing build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); - this.isPaused = true; - this.removeListener(); - if (this.scheduledFuture != null && !this.scheduledFuture.isCancelled()) { - this.scheduledFuture.cancel(false); + isPaused = true; + removeListener(); + if (scheduledFuture != null && !scheduledFuture.isCancelled()) { + scheduledFuture.cancel(false); } updateLocalBuildAgentInformation(); @@ -414,7 +414,7 @@ private void pauseBuildAgent() { log.info("No running build jobs to cancel"); } else { - // Sleep for 10 seconds to allow the build jobs to be finished. If they are not finished, they will be cancelled. + // Sleep for the configured grace period to allow the build jobs to finish. If they are not finished, they will be cancelled. try { Thread.sleep(pauseGracePeriodSeconds * 1000L); } @@ -422,16 +422,16 @@ private void pauseBuildAgent() { log.error("Error while pausing build agent", e); } - if (!this.isPaused) { + if (!isPaused) { log.info("Build agent was resumed before the build jobs could be cancelled"); return; } - this.processResults = false; + processResults = false; Set runningBuildJobIds = buildJobManagementService.getRunningBuildJobIds(); List runningBuildJobs = processingJobs.getAll(runningBuildJobIds).values().stream().toList(); runningBuildJobIds.forEach(buildJobManagementService::cancelBuildJob); - this.queue.addAll(runningBuildJobs); + queue.addAll(runningBuildJobs); log.info("Cancelled running build jobs and added them back to the queue with Ids {}", runningBuildJobIds); log.debug("Cancelled running build jobs: {}", runningBuildJobs); @@ -439,21 +439,21 @@ private void pauseBuildAgent() { } private void resumeBuildAgent() { - if (!this.isPaused) { + if (!isPaused) { log.info("Build agent is already running"); return; } log.info("Resuming build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); - this.isPaused = false; - this.processResults = true; + isPaused = false; + processResults = true; // We remove the listener and scheduledTask first to avoid race conditions - this.removeListener(); - this.listenerId = this.queue.addItemListener(new QueuedBuildJobItemListener(), true); - if (this.scheduledFuture != null && !this.scheduledFuture.isCancelled()) { - this.scheduledFuture.cancel(false); + removeListener(); + listenerId = queue.addItemListener(new QueuedBuildJobItemListener(), true); + if (scheduledFuture != null && !scheduledFuture.isCancelled()) { + scheduledFuture.cancel(false); } - this.scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10)); + scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10)); checkAvailabilityAndProcessNextBuild(); updateLocalBuildAgentInformation(); } diff --git a/src/main/webapp/i18n/de/buildAgents.json b/src/main/webapp/i18n/de/buildAgents.json index b5e7f75575c8..2cd4ca72def3 100644 --- a/src/main/webapp/i18n/de/buildAgents.json +++ b/src/main/webapp/i18n/de/buildAgents.json @@ -17,10 +17,10 @@ "of": "von", "buildJobsRunning": "Build Jobs laufen", "alerts": { - "buildAgentPaused": "Anfrage zum Anhalten des Build-Agenten erfolgreich gesendet. Der Agent akzeptiert keine neuen BuildJobs und wird die aktuellen BuildJobs entweder ordnungsgemäß beenden oder nach 10 Sekunden abbrechen.", + "buildAgentPaused": "Anfrage zum Anhalten des Build-Agenten erfolgreich gesendet. Der Agent akzeptiert keine neuen BuildJobs und wird die aktuellen BuildJobs entweder ordnungsgemäß beenden oder nach einer konfigurierbaren Anzahl von Sekunden abbrechen.", "buildAgentResumed": "Anfrage zum Fortsetzen des BuildJobs erfolgreich gesendet. Der Agent wird wieder neue BuildJobs annehmen.", - "buildAgentPausedFailed": "Anhalten des Build-Agenten fehlgeschlagen.", - "buildAgentResumedFailed": "Fortsetzen des Build-Agenten fehlgeschlagen.", + "buildAgentPauseFailed": "Anhalten des Build-Agenten fehlgeschlagen.", + "buildAgentResumeFailed": "Fortsetzen des Build-Agenten fehlgeschlagen.", "buildAgentWithoutName": "Der Name des Build-Agenten ist erforderlich." }, "pause": "Anhalten", diff --git a/src/main/webapp/i18n/en/buildAgents.json b/src/main/webapp/i18n/en/buildAgents.json index f8b5c4689dbf..e1401d97e272 100644 --- a/src/main/webapp/i18n/en/buildAgents.json +++ b/src/main/webapp/i18n/en/buildAgents.json @@ -17,10 +17,10 @@ "of": "of", "buildJobsRunning": "build jobs running", "alerts": { - "buildAgentPaused": "Build agent pause request sent successfully. The agent will not accept new build jobs and will gracefully finish the current build jobs or will cancel them after 10 seconds.", + "buildAgentPaused": "Build agent pause request sent successfully. The agent will not accept new build jobs and will gracefully finish the current build jobs or will cancel them after a configurable seconds.", "buildAgentResumed": "Build agent resume request sent successfully. The agent will start accepting new build jobs.", - "buildAgentPausedFailed": "Failed to pause the build agent.", - "buildAgentResumedFailed": "Failed to resume the build agent.", + "buildAgentPauseFailed": "Failed to pause the build agent.", + "buildAgentResumeFailed": "Failed to resume the build agent.", "buildAgentWithoutName": "Build agent name is required." }, "pause": "Pause", From 0aab99aa005dc01dc8072952a9e0ef6cdf2f8ab0 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Mon, 23 Sep 2024 18:49:02 +0200 Subject: [PATCH 11/24] add instance locks and use futures instead of sleep --- .../service/BuildJobManagementService.java | 12 +- .../service/SharedQueueProcessingService.java | 118 ++++++++++++------ 2 files changed, 91 insertions(+), 39 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java index dbf299204d41..9878e3f8545c 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobManagementService.java @@ -74,6 +74,8 @@ public class BuildJobManagementService { */ private final Map> runningFutures = new ConcurrentHashMap<>(); + private final Map> runningFuturesWrapper = new ConcurrentHashMap<>(); + /** * A set that contains all build jobs that were cancelled by the user. * This set is unique for each node and contains only the build jobs that were cancelled on this node. @@ -171,13 +173,21 @@ public CompletableFuture executeBuildJob(BuildJobQueueItem buildJob } }); - return futureResult.whenComplete(((result, throwable) -> runningFutures.remove(buildJobItem.id()))); + runningFuturesWrapper.put(buildJobItem.id(), futureResult); + return futureResult.whenComplete(((result, throwable) -> { + runningFutures.remove(buildJobItem.id()); + runningFuturesWrapper.remove(buildJobItem.id()); + })); } Set getRunningBuildJobIds() { return Set.copyOf(runningFutures.keySet()); } + CompletableFuture getRunningBuildJobFutureWrapper(String buildJobId) { + return runningFuturesWrapper.get(buildJobId); + } + /** * Create an asynchronous or a synchronous CompletableFuture depending on the runBuildJobsAsynchronously flag. * diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index d53669eb2e96..9b014af737ae 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -11,10 +11,13 @@ import java.util.UUID; import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; @@ -87,6 +90,11 @@ public class SharedQueueProcessingService { */ private final ReentrantLock instanceLock = new ReentrantLock(); + /** + * Lock for pausing and resuming the build agent. + */ + private final ReentrantLock pauseResumeLock = new ReentrantLock(); + private UUID listenerId; /** @@ -94,8 +102,15 @@ public class SharedQueueProcessingService { */ private ScheduledFuture scheduledFuture; + /** + * Flag to indicate whether the build agent is paused. + */ private volatile boolean isPaused = false; + /** + * Flag to indicate whether the build agent should process build results. This is necessary to differentiate between when the build agent is paused and grace period is not over + * yet. + */ private volatile boolean processResults = true; @Value("${artemis.continuous-integration.pause-grace-period-seconds:15}") @@ -399,42 +414,63 @@ private void pauseBuildAgent() { return; } - log.info("Pausing build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); + pauseResumeLock.lock(); + try { + log.info("Pausing build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); - isPaused = true; - removeListener(); - if (scheduledFuture != null && !scheduledFuture.isCancelled()) { - scheduledFuture.cancel(false); + isPaused = true; + removeListener(); + if (scheduledFuture != null && !scheduledFuture.isCancelled()) { + scheduledFuture.cancel(false); + } + updateLocalBuildAgentInformation(); + } + finally { + pauseResumeLock.unlock(); } - updateLocalBuildAgentInformation(); log.info("Gracefully cancelling running build jobs"); - if (buildJobManagementService.getRunningBuildJobIds().isEmpty()) { + Set runningBuildJobIds = buildJobManagementService.getRunningBuildJobIds(); + if (runningBuildJobIds.isEmpty()) { log.info("No running build jobs to cancel"); } else { - // Sleep for the configured grace period to allow the build jobs to finish. If they are not finished, they will be cancelled. - try { - Thread.sleep(pauseGracePeriodSeconds * 1000L); + List> runningFuturesWrapper = runningBuildJobIds.stream().map(buildJobManagementService::getRunningBuildJobFutureWrapper) + .filter(Objects::nonNull).toList(); + + if (!runningFuturesWrapper.isEmpty()) { + CompletableFuture allFuturesWrapper = CompletableFuture.allOf(runningFuturesWrapper.toArray(new CompletableFuture[0])); + + try { + allFuturesWrapper.get(pauseGracePeriodSeconds, TimeUnit.SECONDS); + log.info("All running build jobs finished during grace period"); + } + catch (TimeoutException e) { + if (!isPaused) { + log.info("Build agent was resumed before the build jobs could be cancelled"); + return; + } + pauseResumeLock.lock(); + try { + log.info("Grace period exceeded. Cancelling running build jobs."); + + processResults = false; + Set runningBuildJobIdsAfterGracePeriod = buildJobManagementService.getRunningBuildJobIds(); + List runningBuildJobsAfterGracePeriod = processingJobs.getAll(runningBuildJobIdsAfterGracePeriod).values().stream().toList(); + runningBuildJobIdsAfterGracePeriod.forEach(buildJobManagementService::cancelBuildJob); + queue.addAll(runningBuildJobsAfterGracePeriod); + log.info("Cancelled running build jobs and added them back to the queue with Ids {}", runningBuildJobIdsAfterGracePeriod); + log.debug("Cancelled running build jobs: {}", runningBuildJobsAfterGracePeriod); + } + finally { + pauseResumeLock.unlock(); + } + } + catch (InterruptedException | ExecutionException e) { + log.error("Error while waiting for running build jobs to finish", e); + } } - catch (InterruptedException e) { - log.error("Error while pausing build agent", e); - } - - if (!isPaused) { - log.info("Build agent was resumed before the build jobs could be cancelled"); - return; - } - - processResults = false; - Set runningBuildJobIds = buildJobManagementService.getRunningBuildJobIds(); - List runningBuildJobs = processingJobs.getAll(runningBuildJobIds).values().stream().toList(); - runningBuildJobIds.forEach(buildJobManagementService::cancelBuildJob); - queue.addAll(runningBuildJobs); - log.info("Cancelled running build jobs and added them back to the queue with Ids {}", runningBuildJobIds); - log.debug("Cancelled running build jobs: {}", runningBuildJobs); - } } @@ -444,18 +480,24 @@ private void resumeBuildAgent() { return; } - log.info("Resuming build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); - isPaused = false; - processResults = true; - // We remove the listener and scheduledTask first to avoid race conditions - removeListener(); - listenerId = queue.addItemListener(new QueuedBuildJobItemListener(), true); - if (scheduledFuture != null && !scheduledFuture.isCancelled()) { - scheduledFuture.cancel(false); + pauseResumeLock.lock(); + try { + log.info("Resuming build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); + isPaused = false; + processResults = true; + // We remove the listener and scheduledTask first to avoid race conditions + removeListener(); + listenerId = queue.addItemListener(new QueuedBuildJobItemListener(), true); + if (scheduledFuture != null && !scheduledFuture.isCancelled()) { + scheduledFuture.cancel(false); + } + scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10)); + checkAvailabilityAndProcessNextBuild(); + updateLocalBuildAgentInformation(); + } + finally { + pauseResumeLock.unlock(); } - scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10)); - checkAvailabilityAndProcessNextBuild(); - updateLocalBuildAgentInformation(); } /** From 74591d1ce4e3527f0db4f4d856e275e5b35e7643 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Mon, 23 Sep 2024 23:37:35 +0200 Subject: [PATCH 12/24] use single lock --- .../service/SharedQueueProcessingService.java | 74 +++++++++---------- 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index 9b014af737ae..c614f3f56014 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -424,54 +424,52 @@ private void pauseBuildAgent() { scheduledFuture.cancel(false); } updateLocalBuildAgentInformation(); - } - finally { - pauseResumeLock.unlock(); - } - log.info("Gracefully cancelling running build jobs"); + log.info("Gracefully cancelling running build jobs"); - Set runningBuildJobIds = buildJobManagementService.getRunningBuildJobIds(); - if (runningBuildJobIds.isEmpty()) { - log.info("No running build jobs to cancel"); - } - else { - List> runningFuturesWrapper = runningBuildJobIds.stream().map(buildJobManagementService::getRunningBuildJobFutureWrapper) - .filter(Objects::nonNull).toList(); + Set runningBuildJobIds = buildJobManagementService.getRunningBuildJobIds(); + if (runningBuildJobIds.isEmpty()) { + log.info("No running build jobs to cancel"); + } + else { + List> runningFuturesWrapper = runningBuildJobIds.stream().map(buildJobManagementService::getRunningBuildJobFutureWrapper) + .filter(Objects::nonNull).toList(); - if (!runningFuturesWrapper.isEmpty()) { - CompletableFuture allFuturesWrapper = CompletableFuture.allOf(runningFuturesWrapper.toArray(new CompletableFuture[0])); + if (!runningFuturesWrapper.isEmpty()) { + CompletableFuture allFuturesWrapper = CompletableFuture.allOf(runningFuturesWrapper.toArray(new CompletableFuture[0])); - try { - allFuturesWrapper.get(pauseGracePeriodSeconds, TimeUnit.SECONDS); - log.info("All running build jobs finished during grace period"); - } - catch (TimeoutException e) { - if (!isPaused) { - log.info("Build agent was resumed before the build jobs could be cancelled"); - return; - } - pauseResumeLock.lock(); try { - log.info("Grace period exceeded. Cancelling running build jobs."); - - processResults = false; - Set runningBuildJobIdsAfterGracePeriod = buildJobManagementService.getRunningBuildJobIds(); - List runningBuildJobsAfterGracePeriod = processingJobs.getAll(runningBuildJobIdsAfterGracePeriod).values().stream().toList(); - runningBuildJobIdsAfterGracePeriod.forEach(buildJobManagementService::cancelBuildJob); - queue.addAll(runningBuildJobsAfterGracePeriod); - log.info("Cancelled running build jobs and added them back to the queue with Ids {}", runningBuildJobIdsAfterGracePeriod); - log.debug("Cancelled running build jobs: {}", runningBuildJobsAfterGracePeriod); + allFuturesWrapper.get(pauseGracePeriodSeconds, TimeUnit.SECONDS); + log.info("All running build jobs finished during grace period"); } - finally { - pauseResumeLock.unlock(); + catch (TimeoutException e) { + handleTimeoutAndCancelRunningJobs(); + } + catch (InterruptedException | ExecutionException e) { + log.error("Error while waiting for running build jobs to finish", e); } - } - catch (InterruptedException | ExecutionException e) { - log.error("Error while waiting for running build jobs to finish", e); } } } + finally { + pauseResumeLock.unlock(); + } + } + + private void handleTimeoutAndCancelRunningJobs() { + if (!isPaused) { + log.info("Build agent was resumed before the build jobs could be cancelled"); + return; + } + log.info("Grace period exceeded. Cancelling running build jobs."); + + processResults = false; + Set runningBuildJobIdsAfterGracePeriod = buildJobManagementService.getRunningBuildJobIds(); + List runningBuildJobsAfterGracePeriod = processingJobs.getAll(runningBuildJobIdsAfterGracePeriod).values().stream().toList(); + runningBuildJobIdsAfterGracePeriod.forEach(buildJobManagementService::cancelBuildJob); + queue.addAll(runningBuildJobsAfterGracePeriod); + log.info("Cancelled running build jobs and added them back to the queue with Ids {}", runningBuildJobIdsAfterGracePeriod); + log.debug("Cancelled running build jobs: {}", runningBuildJobsAfterGracePeriod); } private void resumeBuildAgent() { From 33fbda9453a664663b4c08308a915f60da3b2171 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Tue, 24 Sep 2024 00:50:04 +0200 Subject: [PATCH 13/24] update ui --- .../build-agent-details.component.html | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html index 94ea21ead492..91320b9bbd66 100644 --- a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html +++ b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html @@ -8,14 +8,17 @@

{{ buildAgent.name }}

- - + @if (buildAgent.status === 'PAUSED') { + + } @else { + + }
From b102f65160fafef55219c2a15da6c2b0c955c6e1 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Tue, 24 Sep 2024 21:41:08 +0200 Subject: [PATCH 14/24] feedback --- .../build-agent-details.component.html | 16 ++++++++-------- .../build-agent-details.component.ts | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html index 91320b9bbd66..1a88dba6a6d8 100644 --- a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html +++ b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.html @@ -1,22 +1,22 @@
@if (buildAgent) { -
-
+
+
-

- : -

{{ buildAgent.name }}

+

+ : +

{{ buildAgent.name }}

-
+
@if (buildAgent.status === 'PAUSED') { } @else { }
diff --git a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts index 25214e2f6fe5..de19544b01ad 100644 --- a/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts +++ b/src/main/webapp/app/localci/build-agents/build-agent-details/build-agent-details/build-agent-details.component.ts @@ -29,8 +29,8 @@ export class BuildAgentDetailsComponent implements OnInit, OnDestroy { faExclamationCircle = faExclamationCircle; faExclamationTriangle = faExclamationTriangle; faTimes = faTimes; - faPause = faPause; - faPlay = faPlay; + readonly faPause = faPause; + readonly faPlay = faPlay; constructor( private websocketService: JhiWebsocketService, @@ -110,7 +110,7 @@ export class BuildAgentDetailsComponent implements OnInit, OnDestroy { window.open(url, '_blank'); } - pauseBuildAgent() { + pauseBuildAgent(): void { if (this.buildAgent.name) { this.buildAgentsService.pauseBuildAgent(this.buildAgent.name).subscribe({ next: () => { @@ -134,7 +134,7 @@ export class BuildAgentDetailsComponent implements OnInit, OnDestroy { } } - resumeBuildAgent() { + resumeBuildAgent(): void { if (this.buildAgent.name) { this.buildAgentsService.resumeBuildAgent(this.buildAgent.name).subscribe({ next: () => { From 05650a80ada4587bb534af208dc2e3008063ced9 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Tue, 1 Oct 2024 16:08:02 +0200 Subject: [PATCH 15/24] fix client coverage --- .../build-agents/build-agents.service.spec.ts | 59 ++++++++++++++++++- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/src/test/javascript/spec/component/localci/build-agents/build-agents.service.spec.ts b/src/test/javascript/spec/component/localci/build-agents/build-agents.service.spec.ts index 9ad5c6b8246e..1b13fb1e932c 100644 --- a/src/test/javascript/spec/component/localci/build-agents/build-agents.service.spec.ts +++ b/src/test/javascript/spec/component/localci/build-agents/build-agents.service.spec.ts @@ -1,10 +1,12 @@ import { TestBed } from '@angular/core/testing'; -import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; +import { provideHttpClient } from '@angular/common/http'; +import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; import { MockTranslateService } from '../../../helpers/mocks/service/mock-translate.service'; import { TranslateService } from '@ngx-translate/core'; import { BuildJob } from 'app/entities/programming/build-job.model'; import dayjs from 'dayjs/esm'; +import { lastValueFrom } from 'rxjs'; import { BuildAgentsService } from 'app/localci/build-agents/build-agents.service'; import { BuildAgent } from 'app/entities/programming/build-agent.model'; import { RepositoryInfo, TriggeredByPushTo } from 'app/entities/programming/repository-info.model'; @@ -75,8 +77,7 @@ describe('BuildAgentsService', () => { beforeEach(() => { TestBed.configureTestingModule({ - imports: [HttpClientTestingModule], - providers: [{ provide: TranslateService, useClass: MockTranslateService }], + providers: [provideHttpClient(), provideHttpClientTesting(), { provide: TranslateService, useClass: MockTranslateService }], }); service = TestBed.inject(BuildAgentsService); httpMock = TestBed.inject(HttpTestingController); @@ -112,6 +113,23 @@ describe('BuildAgentsService', () => { req.flush(expectedResponse); }); + it('should handle get build agent details error', async () => { + const errorMessage = 'Failed to fetch build agent details buildAgent1'; + + const observable = lastValueFrom(service.getBuildAgentDetails('buildAgent1')); + + const req = httpMock.expectOne(`${service.adminResourceUrl}/build-agent?agentName=buildAgent1`); + expect(req.request.method).toBe('GET'); + req.flush({ message: errorMessage }, { status: 500, statusText: 'Internal Server Error' }); + + try { + await observable; + throw new Error('expected an error, but got a success'); + } catch (error) { + expect(error.message).toContain(errorMessage); + } + }); + it('should pause build agent', () => { service.pauseBuildAgent('buildAgent1').subscribe(); @@ -128,6 +146,41 @@ describe('BuildAgentsService', () => { req.flush({}); }); + it('should handle pause build agent error', async () => { + const errorMessage = 'Failed to pause build agent buildAgent1'; + + const observable = lastValueFrom(service.pauseBuildAgent('buildAgent1')); + + const req = httpMock.expectOne(`${service.adminResourceUrl}/agent/buildAgent1/pause`); + expect(req.request.method).toBe('PUT'); + req.flush({ message: errorMessage }, { status: 500, statusText: 'Internal Server Error' }); + + try { + await observable; + throw new Error('expected an error, but got a success'); + } catch (error) { + expect(error.message).toContain(errorMessage); + } + }); + + it('should handle resume build agent error', async () => { + const errorMessage = 'Failed to resume build agent buildAgent1'; + + const observable = lastValueFrom(service.resumeBuildAgent('buildAgent1')); + + // Set up the expected HTTP request and flush the response with an error. + const req = httpMock.expectOne(`${service.adminResourceUrl}/agent/buildAgent1/resume`); + expect(req.request.method).toBe('PUT'); + req.flush({ message: errorMessage }, { status: 500, statusText: 'Internal Server Error' }); + + try { + await observable; + throw new Error('expected an error, but got a success'); + } catch (error) { + expect(error.message).toContain(errorMessage); + } + }); + afterEach(() => { httpMock.verify(); // Verify that there are no outstanding requests. }); From 6277db60d330ed69e91a24b576a39166f1927b04 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 13 Oct 2024 04:35:31 +0200 Subject: [PATCH 16/24] fix style --- .../aet/artemis/core/web/admin/AdminBuildJobQueueResource.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java b/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java index 1e84b122dc35..33e7bd099155 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminBuildJobQueueResource.java @@ -204,7 +204,6 @@ public ResponseEntity getBuildJobStatistics(@RequestPara * or an appropriate error response if something went wrong */ @PutMapping("agent/{agentName}/pause") - @EnforceAdmin public ResponseEntity pauseBuildAgent(@PathVariable String agentName) { log.debug("REST request to pause agent {}", agentName); localCIBuildJobQueueService.pauseBuildAgent(agentName); @@ -225,7 +224,6 @@ public ResponseEntity pauseBuildAgent(@PathVariable String agentName) { * or an appropriate error response if something went wrong */ @PutMapping("agent/{agentName}/resume") - @EnforceAdmin public ResponseEntity resumeBuildAgent(@PathVariable String agentName) { log.debug("REST request to resume agent {}", agentName); localCIBuildJobQueueService.resumeBuildAgent(agentName); From 6d33712ea43b56f5334f33aaef5f529394099502 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 13 Oct 2024 16:32:10 +0200 Subject: [PATCH 17/24] fix test --- .../artemis/programming/icl/LocalVCLocalCIIntegrationTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java index 877ccd6493e0..86dd6ac7c070 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java @@ -975,6 +975,7 @@ class BuildJobPriorityTest { @BeforeEach void setUp() { + queuedJobs.clear(); sharedQueueProcessingService.removeListener(); } From 15d862cd0e02ec24202e34c9ac31969f75066a91 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 13 Oct 2024 17:10:43 +0200 Subject: [PATCH 18/24] fix test --- .../service/SharedQueueProcessingService.java | 23 +++++++++++-------- .../icl/LocalCIResourceIntegrationTest.java | 2 +- .../programming/icl/LocalCIServiceTest.java | 2 +- .../icl/LocalVCLocalCIIntegrationTest.java | 2 +- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index 02281305c813..cdd34278c6e8 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -161,10 +161,21 @@ public void init() { } @PreDestroy - public void removeListener() { + public void removeListenerAndCancelScheduledFuture() { + removeListener(); + cancelCheckAvailabilityAndProcessNextBuildScheduledFuture(); + } + + private void removeListener() { this.queue.removeItemListener(this.listenerId); } + private void cancelCheckAvailabilityAndProcessNextBuildScheduledFuture() { + if (scheduledFuture != null && !scheduledFuture.isCancelled()) { + scheduledFuture.cancel(false); + } + } + /** * Wait 1 minute after startup and then every 1 minute update the build agent information of the local hazelcast member. * This is necessary because the build agent information is not updated automatically when a node joins the cluster. @@ -422,10 +433,7 @@ private void pauseBuildAgent() { log.info("Pausing build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); isPaused = true; - removeListener(); - if (scheduledFuture != null && !scheduledFuture.isCancelled()) { - scheduledFuture.cancel(false); - } + removeListenerAndCancelScheduledFuture(); updateLocalBuildAgentInformation(); log.info("Gracefully cancelling running build jobs"); @@ -487,11 +495,8 @@ private void resumeBuildAgent() { isPaused = false; processResults = true; // We remove the listener and scheduledTask first to avoid race conditions - removeListener(); + removeListenerAndCancelScheduledFuture(); listenerId = queue.addItemListener(new QueuedBuildJobItemListener(), true); - if (scheduledFuture != null && !scheduledFuture.isCancelled()) { - scheduledFuture.cancel(false); - } scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10)); checkAvailabilityAndProcessNextBuild(); updateLocalBuildAgentInformation(); diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIResourceIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIResourceIntegrationTest.java index 28a758401b8a..e05b87776154 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIResourceIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIResourceIntegrationTest.java @@ -88,7 +88,7 @@ protected String getTestPrefix() { @BeforeEach void createJobs() { // temporarily remove listener to avoid triggering build job processing - sharedQueueProcessingService.removeListener(); + sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); JobTimingInfo jobTimingInfo1 = new JobTimingInfo(ZonedDateTime.now().plusMinutes(1), ZonedDateTime.now().plusMinutes(2), ZonedDateTime.now().plusMinutes(3)); JobTimingInfo jobTimingInfo2 = new JobTimingInfo(ZonedDateTime.now(), ZonedDateTime.now().plusMinutes(1), ZonedDateTime.now().plusMinutes(2)); diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIServiceTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIServiceTest.java index 040aef91d87e..b935eb8d2d67 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIServiceTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIServiceTest.java @@ -80,7 +80,7 @@ void setUp() { processingJobs = hazelcastInstance.getMap("processingJobs"); // remove listener to avoid triggering build job processing - sharedQueueProcessingService.removeListener(); + sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); } @AfterEach diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java index 86dd6ac7c070..a29429728644 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java @@ -976,7 +976,7 @@ class BuildJobPriorityTest { @BeforeEach void setUp() { queuedJobs.clear(); - sharedQueueProcessingService.removeListener(); + sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); } @AfterEach From 89a652ef279526c5c43a67422a4967bcd8a344c8 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 13 Oct 2024 18:02:33 +0200 Subject: [PATCH 19/24] fix test --- .../service/SharedQueueProcessingService.java | 29 ++++++++++--------- .../icl/LocalVCLocalCIIntegrationTest.java | 1 + 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index cdd34278c6e8..29dd4009b59f 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -18,6 +18,7 @@ import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; @@ -105,13 +106,13 @@ public class SharedQueueProcessingService { /** * Flag to indicate whether the build agent is paused. */ - private volatile boolean isPaused = false; + private final AtomicBoolean isPaused = new AtomicBoolean(false); /** * Flag to indicate whether the build agent should process build results. This is necessary to differentiate between when the build agent is paused and grace period is not over * yet. */ - private volatile boolean processResults = true; + private final AtomicBoolean processResults = new AtomicBoolean(true); @Value("${artemis.continuous-integration.pause-grace-period-seconds:15}") private int pauseGracePeriodSeconds; @@ -215,14 +216,14 @@ private void checkAvailabilityAndProcessNextBuild() { return; } - if (queue.isEmpty() || isPaused) { + if (queue.isEmpty() || isPaused.get()) { return; } BuildJobQueueItem buildJob = null; instanceLock.lock(); try { // Recheck conditions after acquiring the lock to ensure they are still valid - if (!nodeIsAvailable() || queue.isEmpty() || isPaused) { + if (!nodeIsAvailable() || queue.isEmpty() || isPaused.get()) { return; } @@ -299,7 +300,7 @@ private BuildAgentInformation getUpdatedLocalBuildAgentInformation(BuildJobQueue int numberOfCurrentBuildJobs = processingJobsOfMember.size(); int maxNumberOfConcurrentBuilds = localCIBuildExecutorService.getMaximumPoolSize(); boolean hasJobs = numberOfCurrentBuildJobs > 0; - BuildAgentInformation.BuildAgentStatus status = isPaused ? BuildAgentInformation.BuildAgentStatus.PAUSED + BuildAgentInformation.BuildAgentStatus status = isPaused.get() ? BuildAgentInformation.BuildAgentStatus.PAUSED : hasJobs ? BuildAgentInformation.BuildAgentStatus.ACTIVE : BuildAgentInformation.BuildAgentStatus.IDLE; BuildAgentInformation agent = buildAgentInformation.get(memberAddress); List recentBuildJobs; @@ -364,7 +365,7 @@ private void processBuild(BuildJobQueueItem buildJob) { buildLogsMap.removeBuildLogs(buildJob.id()); ResultQueueItem resultQueueItem = new ResultQueueItem(buildResult, finishedJob, buildLogs, null); - if (processResults) { + if (processResults.get()) { resultQueue.add(resultQueueItem); } else { @@ -406,7 +407,7 @@ private void processBuild(BuildJobQueueItem buildJob) { failedResult.setBuildLogEntries(buildLogs); ResultQueueItem resultQueueItem = new ResultQueueItem(failedResult, job, buildLogs, ex); - if (processResults) { + if (processResults.get()) { resultQueue.add(resultQueueItem); } else { @@ -423,7 +424,7 @@ private void processBuild(BuildJobQueueItem buildJob) { } private void pauseBuildAgent() { - if (isPaused) { + if (isPaused.get()) { log.info("Build agent is already paused"); return; } @@ -432,7 +433,7 @@ private void pauseBuildAgent() { try { log.info("Pausing build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); - isPaused = true; + isPaused.set(true); removeListenerAndCancelScheduledFuture(); updateLocalBuildAgentInformation(); @@ -468,13 +469,13 @@ private void pauseBuildAgent() { } private void handleTimeoutAndCancelRunningJobs() { - if (!isPaused) { + if (!isPaused.get()) { log.info("Build agent was resumed before the build jobs could be cancelled"); return; } log.info("Grace period exceeded. Cancelling running build jobs."); - processResults = false; + processResults.set(false); Set runningBuildJobIdsAfterGracePeriod = buildJobManagementService.getRunningBuildJobIds(); List runningBuildJobsAfterGracePeriod = processingJobs.getAll(runningBuildJobIdsAfterGracePeriod).values().stream().toList(); runningBuildJobIdsAfterGracePeriod.forEach(buildJobManagementService::cancelBuildJob); @@ -484,7 +485,7 @@ private void handleTimeoutAndCancelRunningJobs() { } private void resumeBuildAgent() { - if (!isPaused) { + if (!isPaused.get()) { log.info("Build agent is already running"); return; } @@ -492,8 +493,8 @@ private void resumeBuildAgent() { pauseResumeLock.lock(); try { log.info("Resuming build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); - isPaused = false; - processResults = true; + isPaused.set(false); + processResults.set(true); // We remove the listener and scheduledTask first to avoid race conditions removeListenerAndCancelScheduledFuture(); listenerId = queue.addItemListener(new QueuedBuildJobItemListener(), true); diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java index a29429728644..289248a19cd0 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java @@ -1040,6 +1040,7 @@ private void testPriority(String login, int expectedPriority) throws Exception { localVCLocalCITestService.mockInputStreamReturnedFromContainer(dockerClient, LOCALCI_WORKING_DIRECTORY + "/testing-dir/assignment/.git/refs/heads/[^/]+", Map.of("commitHash", commitHash), Map.of("commitHash", commitHash)); localVCLocalCITestService.mockTestResults(dockerClient, PARTLY_SUCCESSFUL_TEST_RESULTS_PATH, LOCALCI_WORKING_DIRECTORY + LOCALCI_RESULTS_DIRECTORY); + sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); localCITriggerService.triggerBuild(studentParticipation, false); log.info("Trigger build done"); From 3f1db83e4b4475ece740979dbaa7bfffe9e5f582 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 13 Oct 2024 18:28:19 +0200 Subject: [PATCH 20/24] fix test --- .../artemis/programming/icl/LocalVCLocalCIIntegrationTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java index 289248a19cd0..a29429728644 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java @@ -1040,7 +1040,6 @@ private void testPriority(String login, int expectedPriority) throws Exception { localVCLocalCITestService.mockInputStreamReturnedFromContainer(dockerClient, LOCALCI_WORKING_DIRECTORY + "/testing-dir/assignment/.git/refs/heads/[^/]+", Map.of("commitHash", commitHash), Map.of("commitHash", commitHash)); localVCLocalCITestService.mockTestResults(dockerClient, PARTLY_SUCCESSFUL_TEST_RESULTS_PATH, LOCALCI_WORKING_DIRECTORY + LOCALCI_RESULTS_DIRECTORY); - sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); localCITriggerService.triggerBuild(studentParticipation, false); log.info("Trigger build done"); From fb398acacb31abd22f9c014f9fd91e8c96cebde5 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 13 Oct 2024 20:02:00 +0200 Subject: [PATCH 21/24] fix test --- .../artemis/programming/icl/LocalVCLocalCIIntegrationTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java index a29429728644..96f794158ae4 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java @@ -1007,6 +1007,8 @@ void testBuildPriorityAfterDueDate() throws Exception { @Test @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") void testPriorityRunningExam() throws Exception { + queuedJobs.clear(); + sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); Exam exam = examUtilService.addExamWithExerciseGroup(course, true); ExerciseGroup exerciseGroup = exam.getExerciseGroups().getFirst(); From c00cdc7344bcfe64b6a97b42ad386d1559c46c2b Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Sun, 13 Oct 2024 20:22:17 +0200 Subject: [PATCH 22/24] fix test --- .../programming/icl/LocalVCLocalCIIntegrationTest.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java index 96f794158ae4..9b12a9d7f5d7 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java @@ -973,12 +973,6 @@ void testFetchPush_instructorPracticeRepository() throws Exception { @Nested class BuildJobPriorityTest { - @BeforeEach - void setUp() { - queuedJobs.clear(); - sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); - } - @AfterEach void tearDown() { queuedJobs.clear(); @@ -991,12 +985,16 @@ void tearDown() { @Test @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") void testBuildPriorityBeforeDueDate() throws Exception { + queuedJobs.clear(); + sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); testPriority(student1Login, PRIORITY_NORMAL); } @Test @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") void testBuildPriorityAfterDueDate() throws Exception { + queuedJobs.clear(); + sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); // Set dueDate before now programmingExercise.setDueDate(ZonedDateTime.now().minusMinutes(1)); programmingExerciseRepository.save(programmingExercise); From 88cd16c8d31fa719b6cb6718bdf30df6d0ac6e38 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Fri, 18 Oct 2024 15:37:40 +0200 Subject: [PATCH 23/24] tests --- .../programming/icl/LocalVCLocalCIIntegrationTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java index 9b12a9d7f5d7..6b8eaf1b4f9b 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java @@ -985,16 +985,12 @@ void tearDown() { @Test @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") void testBuildPriorityBeforeDueDate() throws Exception { - queuedJobs.clear(); - sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); testPriority(student1Login, PRIORITY_NORMAL); } @Test @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") void testBuildPriorityAfterDueDate() throws Exception { - queuedJobs.clear(); - sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); // Set dueDate before now programmingExercise.setDueDate(ZonedDateTime.now().minusMinutes(1)); programmingExerciseRepository.save(programmingExercise); @@ -1005,8 +1001,6 @@ void testBuildPriorityAfterDueDate() throws Exception { @Test @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") void testPriorityRunningExam() throws Exception { - queuedJobs.clear(); - sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); Exam exam = examUtilService.addExamWithExerciseGroup(course, true); ExerciseGroup exerciseGroup = exam.getExerciseGroups().getFirst(); @@ -1032,6 +1026,9 @@ void testPriorityRunningExam() throws Exception { } private void testPriority(String login, int expectedPriority) throws Exception { + queuedJobs.clear(); + sharedQueueProcessingService.removeListenerAndCancelScheduledFuture(); + log.info("Creating participation"); ProgrammingExerciseStudentParticipation studentParticipation = localVCLocalCITestService.createParticipation(programmingExercise, student1Login); From 8aa86fc5e294562132bb161479116b7782860219 Mon Sep 17 00:00:00 2001 From: Mohamed Bilel Besrour Date: Fri, 18 Oct 2024 16:49:27 +0200 Subject: [PATCH 24/24] tests --- .../buildagent/service/SharedQueueProcessingService.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java index 29dd4009b59f..69402e346a1f 100644 --- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java +++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/SharedQueueProcessingService.java @@ -136,6 +136,10 @@ public void init() { this.processingJobs = this.hazelcastInstance.getMap("processingJobs"); this.queue = this.hazelcastInstance.getQueue("buildJobQueue"); this.resultQueue = this.hazelcastInstance.getQueue("buildResultQueue"); + // Remove listener if already present + if (this.listenerId != null) { + this.queue.removeItemListener(this.listenerId); + } this.listenerId = this.queue.addItemListener(new QueuedBuildJobItemListener(), true); /* @@ -495,7 +499,7 @@ private void resumeBuildAgent() { log.info("Resuming build agent with address {}", hazelcastInstance.getCluster().getLocalMember().getAddress().toString()); isPaused.set(false); processResults.set(true); - // We remove the listener and scheduledTask first to avoid race conditions + // We remove the listener and scheduledTask first to avoid having multiple listeners and scheduled tasks running removeListenerAndCancelScheduledFuture(); listenerId = queue.addItemListener(new QueuedBuildJobItemListener(), true); scheduledFuture = taskScheduler.scheduleAtFixedRate(this::checkAvailabilityAndProcessNextBuild, Duration.ofSeconds(10));