Merge pull request #159 from mrc-ide/mrc-6122

Update to new orderly.runner interface.

.github/workflows/backend-test-and-build.yml

@@ -37,6 +37,9 @@ jobs:
         uses: ./.github/actions/build-db
       - name: Run dependencies
         run: ./scripts/run-dependencies
+        env:
+          ORDERLY_RUNNER_LOGS_PATH: "${{ runner.temp }}/runner-logs"
+
       - name: Set up Java JDK ${{ matrix.java-version }}
         uses: actions/setup-java@v3
         with:
@@ -59,3 +62,20 @@ jobs:
       - name: Smoke test
         working-directory: api
         run: ./scripts/run && ./scripts/smoke-test
+
+      - name: Collect docker logs
+        if: failure()
+        uses: jwalton/gh-docker-logs@v2
+        with:
+          dest: '${{ runner.temp }}/docker-logs'
+
+      - name: Upload test reports
+        uses: actions/upload-artifact@v4
+        if: ${{ failure() }}
+        with:
+          name: reports
+          path: |
+            api/app/build/reports/
+            api/app/build/test-results/
+            ${{ runner.temp }}/docker-logs
+            ${{ runner.temp }}/runner-logs

api/app/build.gradle.kts

@@ -44,6 +44,7 @@ dependencies {
     testImplementation("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.14.1")
     testImplementation("com.github.fge:json-schema-validator:2.2.6")
     testImplementation("org.mock-server:mockserver-junit-jupiter:5.15.0")
+    testImplementation("org.awaitility:awaitility:4.2.2")
 
     // This dependency is used by the application.
     implementation("com.google.guava:guava:31.1-jre")

api/app/src/main/kotlin/packit/AppConfig.kt

@@ -31,7 +31,6 @@ class AppConfig(private val enviroment: Environment)
     }
 
     val outpackServerUrl: String = requiredEnvValue("outpack.server.url")
-    val orderlyRunnerUrl: String = requiredEnvValue("orderly.runner.url")
     val dbUrl: String = requiredEnvValue("db.url")
     val dbUser: String = requiredEnvValue("db.user")
     val dbPassword: String = requiredEnvValue("db.password")

api/app/src/main/kotlin/packit/config/RunnerConfig.kt

@@ -0,0 +1,16 @@
+package packit.config
+
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
+import org.springframework.boot.context.properties.ConfigurationProperties
+
+data class RunnerRepository(
+  val url: String,
+)
+
+@ConfigurationProperties(prefix = "orderly.runner")
+@ConditionalOnProperty(prefix = "orderly.runner", name = ["enabled"], havingValue = "true")
+data class RunnerConfig(
+  val url: String,
+  val locationUrl: String,
+  val repository: RunnerRepository
+)

api/app/src/main/kotlin/packit/controllers/RunnerController.kt

@@ -6,6 +6,7 @@ import org.springframework.security.access.prepost.PreAuthorize
 import org.springframework.security.core.annotation.AuthenticationPrincipal
 import org.springframework.validation.annotation.Validated
 import org.springframework.web.bind.annotation.*
+import org.springframework.web.client.HttpClientErrorException
 import packit.model.PageablePayload
 import packit.model.dto.*
 import packit.model.toBasicDto
@@ -31,10 +32,22 @@ class RunnerController(private val runnerService: RunnerService)
         return ResponseEntity.noContent().build()
     }
 
-    @GetMapping("git/branches")
+    @GetMapping("/git/branches")
     fun getBranches(): ResponseEntity<GitBranches>
     {
-        return ResponseEntity.ok(runnerService.getBranches())
+        try {
+            return ResponseEntity.ok(runnerService.getBranches())
+        } catch (e: HttpClientErrorException.NotFound) {
+            // The runner API returns a 404 if we've never fetched that repository.
+            // It is simpler for the frontend if this case is simply reported as an empty list of branch.
+            // The user can then sync the repo which will cause the initial clone.
+            return ResponseEntity.ok(
+                GitBranches(
+                    defaultBranch = null,
+                    branches = emptyList(),
+                )
+            )
+        }
     }
 
     @GetMapping("/{packetGroupName}/parameters")
@@ -43,7 +56,7 @@ class RunnerController(private val runnerService: RunnerService)
         @RequestParam(defaultValue = "HEAD") ref: String
     ): ResponseEntity<List<Parameter>>
     {
-        return ResponseEntity.ok(runnerService.getParameters(packetGroupName, ref))
+        return ResponseEntity.ok(runnerService.getParameters(ref, packetGroupName))
     }
 
     @GetMapping("/packetGroups")

api/app/src/main/kotlin/packit/model/dto/GitBranches.kt

@@ -13,5 +13,5 @@ data class GitBranchInfo(
     @JsonAlias("commit_hash")
     val commitHash: String,
     val time: Long,
-    val message: List<String>
+    val message: String
 )

api/app/src/main/kotlin/packit/model/dto/RepositoryFetch.kt

@@ -0,0 +1,3 @@
+package packit.model.dto
+
+data class RepositoryFetch(val url: String)

api/app/src/main/kotlin/packit/model/dto/RunnerSubmitRunInfo.kt

@@ -0,0 +1,25 @@
+package packit.model.dto
+
+import com.fasterxml.jackson.annotation.JsonProperty
+
+data class OrderlyLocation(
+    val type: String,
+    val args: Map<String, Any>,
+) {
+    companion object {
+        fun http(url: String): OrderlyLocation {
+            return OrderlyLocation(type = "http", args = mapOf("url" to url))
+        }
+    }
+}
+
+data class RunnerSubmitRunInfo(
+    val url: String?,
+    @JsonProperty("name")
+    val packetGroupName: String,
+    val branch: String,
+    @JsonProperty("hash")
+    val commitHash: String,
+    val parameters: Map<String, Any>,
+    val location: OrderlyLocation,
+)

api/app/src/main/kotlin/packit/model/dto/SubmitRunInfo.kt

@@ -8,5 +8,5 @@ data class SubmitRunInfo(
     val branch: String,
     @JsonProperty("hash")
     val commitHash: String,
-    val parameters: Map<String, Any>?
+    val parameters: Map<String, Any> = mapOf()
 )

api/app/src/main/kotlin/packit/service/GenericClient.kt

@@ -26,28 +26,28 @@ object GenericClient
         RestTemplate(requestFactory)
     }
 
-    inline fun <reified T : Any> get(url: String): T
+    inline fun <reified T : Any> get(url: String, uriVariables: Map<String, Any> = emptyMap()): T
     {
         val response = restTemplate.exchange(
             url,
             HttpMethod.GET,
             HttpEntity.EMPTY,
-            object : ParameterizedTypeReference<ServerResponse<T>>()
-            {}
+            object : ParameterizedTypeReference<ServerResponse<T>>() {},
+            uriVariables
         )
         return handleResponse(response)
     }
 
-    inline fun <reified T> post(url: String, body: Any? = null): T
+    inline fun <reified T> post(url: String, body: Any? = null, uriVariables: Map<String, Any> = emptyMap()): T
     {
         log.debug("Posting to {}", url)
 
         val response = restTemplate.exchange(
             url,
             HttpMethod.POST,
             body?.let { HttpEntity(it) } ?: HttpEntity.EMPTY,
-            object : ParameterizedTypeReference<ServerResponse<T>>()
-            {}
+            object : ParameterizedTypeReference<ServerResponse<T>>() {},
+            uriVariables
         )
 
         return handleResponse(response)

api/app/src/main/kotlin/packit/service/OrderlyRunnerClient.kt

@@ -1,50 +1,76 @@
 package packit.service
 
-import org.springframework.stereotype.Service
-import packit.AppConfig
+import packit.model.dto.GitBranches
 import packit.model.dto.OrderlyRunnerVersion
 import packit.model.dto.Parameter
+import packit.model.dto.RepositoryFetch
 import packit.model.dto.RunnerPacketGroup
-import packit.model.dto.SubmitRunInfo
+import packit.model.dto.RunnerSubmitRunInfo
 import packit.model.dto.SubmitRunResponse
 import packit.model.dto.TaskStatus
 
 interface OrderlyRunner
 {
     fun getVersion(): OrderlyRunnerVersion
-    fun getParameters(packetGroupName: String, ref: String): List<Parameter>
-    fun getPacketGroups(ref: String): List<RunnerPacketGroup>
-    fun submitRun(info: SubmitRunInfo): SubmitRunResponse
+    fun gitFetch(url: String)
+    fun getBranches(url: String): GitBranches
+    fun getParameters(url: String, ref: String, packetGroupName: String): List<Parameter>
+    fun getPacketGroups(url: String, ref: String): List<RunnerPacketGroup>
+    fun submitRun(info: RunnerSubmitRunInfo): SubmitRunResponse
     fun getTaskStatuses(taskIds: List<String>, includeLogs: Boolean): List<TaskStatus>
 }
 
-@Service
-class OrderlyRunnerClient(appConfig: AppConfig) : OrderlyRunner
+class OrderlyRunnerClient(val baseUrl: String) : OrderlyRunner
 {
-    val baseUrl: String = appConfig.orderlyRunnerUrl
     override fun getVersion(): OrderlyRunnerVersion
     {
         return GenericClient.get(constructUrl("/"))
     }
 
-    override fun getParameters(packetGroupName: String, ref: String): List<Parameter>
+    override fun gitFetch(url: String)
     {
-        return GenericClient.get(constructUrl("/report/$packetGroupName/parameters?ref=$ref"))
+        return GenericClient.post(
+            constructUrl("repository/fetch"),
+            RepositoryFetch(url = url)
+        )
     }
 
-    override fun getPacketGroups(ref: String): List<RunnerPacketGroup>
+    override fun getBranches(url: String): GitBranches
     {
-        return GenericClient.get(constructUrl("/report/list?ref=$ref"))
+        return GenericClient.get(
+            constructUrl("repository/branches?url={url}"),
+            mapOf("url" to url)
+        )
     }
 
-    override fun submitRun(info: SubmitRunInfo): SubmitRunResponse
+    override fun getPacketGroups(url: String, ref: String): List<RunnerPacketGroup>
+    {
+        return GenericClient.get(
+            constructUrl("/report/list?url={url}&ref={ref}"),
+            mapOf("url" to url, "ref" to ref)
+        )
+    }
+
+    override fun getParameters(url: String, ref: String, packetGroupName: String): List<Parameter>
+    {
+        return GenericClient.get(
+            constructUrl("report/parameters?url={url}&ref={ref}&name={name}"),
+            mapOf("url" to url, "ref" to ref, "name" to packetGroupName)
+        )
+    }
+
+    override fun submitRun(info: RunnerSubmitRunInfo): SubmitRunResponse
     {
         return GenericClient.post(constructUrl("/report/run"), info)
     }
 
     override fun getTaskStatuses(taskIds: List<String>, includeLogs: Boolean): List<TaskStatus>
     {
-        return GenericClient.post(constructUrl("/report/status?include_logs=$includeLogs"), taskIds)
+        return GenericClient.post(
+            constructUrl("/report/status?include_logs={includeLogs}"),
+            taskIds,
+            mapOf("includeLogs" to includeLogs)
+        )
     }
 
     private fun constructUrl(urlFragment: String): String

api/app/src/main/kotlin/packit/service/OutpackServerClient.kt

@@ -6,7 +6,6 @@ import org.springframework.http.HttpHeaders
 import org.springframework.stereotype.Service
 import packit.AppConfig
 import packit.model.PacketMetadata
-import packit.model.dto.GitBranches
 import packit.model.dto.OutpackMetadata
 
 interface OutpackServer
@@ -21,8 +20,6 @@ interface OutpackServer
         copyRequestBody: Boolean
     )
     fun getChecksum(): String
-    fun gitFetch()
-    fun getBranches(): GitBranches
 }
 
 @Service
@@ -55,16 +52,6 @@ class OutpackServerClient(appConfig: AppConfig) : OutpackServer
         return GenericClient.get(constructUrl("checksum"))
     }
 
-    override fun gitFetch()
-    {
-        return GenericClient.post(constructUrl("git/fetch"))
-    }
-
-    override fun getBranches(): GitBranches
-    {
-        return GenericClient.get(constructUrl("git/branches"))
-    }
-
     override fun getMetadata(from: Double?): List<OutpackMetadata>
     {
         var url = "packit/metadata"