From 2d7895e0e0a33fdda9710a523930a950882386b5 Mon Sep 17 00:00:00 2001 From: jdcanas Date: Mon, 19 Aug 2024 13:55:46 -0700 Subject: [PATCH 1/6] delete disk done --- .../workbench/leonardo/dao/HttpWsmDao.scala | 26 --------- .../dsde/workbench/leonardo/dao/WsmDao.scala | 10 +--- .../leonardo/util/AzurePubsubHandler.scala | 8 ++- .../workbench/leonardo/dao/MockWsmDAO.scala | 53 ------------------- .../dao/WsmApiClientProviderSpec.scala | 1 - .../leonardo/monitor/MonitorAtBootSpec.scala | 2 +- .../util/AzurePubsubHandlerSpec.scala | 3 +- 7 files changed, 7 insertions(+), 96 deletions(-) diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala index 8f3333a3ca..9b3f94f9f7 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala @@ -252,12 +252,6 @@ class HttpWsmDao[F[_]](httpClient: Client[F], config: HttpWsmDaoConfig)(implicit )(onError) } yield resOpt.fold(List.empty[LandingZoneResourcesByPurpose])(res => res.resources) - override def deleteDisk(request: DeleteWsmResourceRequest, authorization: Authorization)(implicit - ev: Ask[F, AppContext] - ): F[Option[DeleteWsmResourceResult]] = - deleteHelper(request, authorization, "disks") - - // TODO: Next up for removal, IA-4175 override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit ev: Ask[F, AppContext] ): F[GetCreateVmJobResult] = @@ -278,26 +272,6 @@ class HttpWsmDao[F[_]](httpClient: Client[F], config: HttpWsmDaoConfig)(implicit )(onError) } yield res - override def getDeleteDiskJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[F, AppContext] - ): F[GetDeleteJobResult] = - for { - ctx <- ev.ask - res <- httpClient.expectOr[GetDeleteJobResult]( - Request[F]( - method = Method.GET, - uri = config.uri - .withPath( - Uri.Path - .unsafeFromString( - s"/api/workspaces/v1/${request.workspaceId.value.toString}/resources/controlled/azure/disks/delete-result/${request.jobId.value}" - ) - ), - headers = headers(authorization, ctx.traceId, false) - ) - )(onError) - } yield res - override def getWorkspaceStorageContainer(workspaceId: WorkspaceId, authorization: Authorization)(implicit ev: Ask[F, AppContext] ): F[Option[StorageContainerResponse]] = for { diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala index 0d47a21dc8..2cd73a259a 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala @@ -34,23 +34,15 @@ import java.time.ZonedDateTime import java.util.UUID trait WsmDao[F[_]] { - def deleteDisk(request: DeleteWsmResourceRequest, authorization: Authorization)(implicit - ev: Ask[F, AppContext] - ): F[Option[DeleteWsmResourceResult]] - - // TODO: IA-4175 next up def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit ev: Ask[F, AppContext] ): F[GetCreateVmJobResult] - def getDeleteDiskJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[F, AppContext] - ): F[GetDeleteJobResult] - def getLandingZoneResources(billingProfileId: BillingProfileId, userToken: Authorization)(implicit ev: Ask[F, AppContext] ): F[LandingZoneResources] + // TODO: pt2 def getWorkspaceStorageContainer(workspaceId: WorkspaceId, authorization: Authorization)(implicit ev: Ask[F, AppContext] ): F[Option[StorageContainerResponse]] diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala index 2a278c675b..cb39da739b 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala @@ -1304,12 +1304,11 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( )(implicit ev: Ask[F, AppContext]): F[Unit] = for { ctx <- ev.ask - jobId = getWsmJobId("delete-disk", wsmResourceId) _ <- logger.info(ctx.loggingCtx)(s"Sending WSM delete message for disk resource ${wsmResourceId.value}") wsmControlledResourceClient <- buildWsmControlledResourceApiClient - deleteDiskBody = getDeleteControlledResourceRequest(jobId.value) + deleteDiskBody = getDeleteControlledResourceRequest(jobId) _ <- F .delay(wsmControlledResourceClient.deleteAzureDisk(deleteDiskBody, workspaceId.value, wsmResourceId.value)) .void @@ -1321,7 +1320,6 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( s"${ctx.traceId.asString} | WSM call to delete disk failed due to ${e.getMessage}. Please retry delete again" ) ) - getDeleteJobResult = F.delay(wsmControlledResourceClient.getDeleteAzureDiskResult(workspaceId.value, jobId.value)) // We need to wait until WSM deletion job to be done to update the database @@ -1417,10 +1415,10 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( } yield wsmControlledResourceClient private def getDeleteControlledResourceRequest( - jobId: String = UUID.randomUUID().toString + jobId: WsmJobId = WsmJobId(UUID.randomUUID().toString) ): bio.terra.workspace.model.DeleteControlledAzureResourceRequest = { val jobControl = new JobControl() - .id(jobId) + .id(jobId.value) val deleteControlledResource = new bio.terra.workspace.model.DeleteControlledAzureResourceRequest() .jobControl(jobControl) diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/MockWsmDAO.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/MockWsmDAO.scala index a6aa7c6044..20b2874f11 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/MockWsmDAO.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/MockWsmDAO.scala @@ -51,59 +51,6 @@ class MockWsmDAO(jobStatus: WsmJobStatus = WsmJobStatus.Succeeded) extends WsmDa CommonTestData.landingZoneResources ) - override def deleteDisk(request: DeleteWsmResourceRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[Option[DeleteWsmResourceResult]] = - IO.pure( - Some( - DeleteWsmResourceResult( - WsmJobReport( - request.deleteRequest.jobControl.id, - "desc", - jobStatus, - 200, - ZonedDateTime.parse("2022-03-18T15:02:29.264756Z"), - Some(ZonedDateTime.parse("2022-03-18T15:02:29.264756Z")), - "resultUrl" - ), - if (jobStatus.equals(WsmJobStatus.Failed)) - Some( - WsmErrorReport( - "error", - 500, - List.empty - ) - ) - else None - ) - ) - ) - - override def getDeleteDiskJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[GetDeleteJobResult] = IO.pure( - GetDeleteJobResult( - WsmJobReport( - request.jobId, - "desc", - jobStatus, - 200, - ZonedDateTime.parse("2022-03-18T15:02:29.264756Z"), - Some(ZonedDateTime.parse("2022-03-18T15:02:29.264756Z")), - "resultUrl" - ), - if (jobStatus.equals(WsmJobStatus.Failed)) - Some( - WsmErrorReport( - "error", - 500, - List.empty - ) - ) - else None - ) - ) - override def getWorkspaceStorageContainer(workspaceId: WorkspaceId, authorization: Authorization)(implicit ev: Ask[IO, AppContext] ): IO[Option[StorageContainerResponse]] = diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmApiClientProviderSpec.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmApiClientProviderSpec.scala index 6a450aa2a0..a79dffacbe 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmApiClientProviderSpec.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmApiClientProviderSpec.scala @@ -24,7 +24,6 @@ import org.broadinstitute.dsde.workbench.leonardo.CommonTestData.{ } import org.broadinstitute.dsde.workbench.leonardo.TestUtils.appContext import org.broadinstitute.dsde.workbench.leonardo.db.WsmResourceType -import org.broadinstitute.dsde.workbench.leonardo.util.AzureTestUtils import org.broadinstitute.dsde.workbench.leonardo.{AppContext, LeonardoTestSuite} import org.http4s._ import org.mockito.Mockito.{times, verify, when} diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/MonitorAtBootSpec.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/MonitorAtBootSpec.scala index bf34290ad5..e2fe40b6f6 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/MonitorAtBootSpec.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/MonitorAtBootSpec.scala @@ -7,7 +7,7 @@ import cats.syntax.all._ import org.broadinstitute.dsde.workbench.google2.mock.FakeGoogleComputeService import org.broadinstitute.dsde.workbench.leonardo.CommonTestData._ import org.broadinstitute.dsde.workbench.leonardo.KubernetesTestData._ -import org.broadinstitute.dsde.workbench.leonardo.dao.{MockSamDAO, MockWsmClientProvider, MockWsmDAO} +import org.broadinstitute.dsde.workbench.leonardo.dao.{MockSamDAO, MockWsmClientProvider} import org.broadinstitute.dsde.workbench.leonardo.db.TestComponent import org.broadinstitute.dsde.workbench.leonardo.monitor.ClusterNodepoolAction.{ CreateClusterAndNodepool, diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala index e382177b87..e03280c35e 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala @@ -1639,10 +1639,12 @@ class AzurePubsubHandlerSpec it should "delete azure disk properly" in isolatedDbTest { val queue = QueueFactory.asyncTaskQueue() + val (mockWsm, mockControlledResourceApi, _, _) = AzureTestUtils.setUpMockWsmApiClientProvider(storageContainerJobStatus = JobReport.StatusEnum.SUCCEEDED) val azureInterp = makeAzurePubsubHandler(asyncTaskQueue = queue, wsmClient = mockWsm) + val resourceId = WsmControlledResourceId(UUID.randomUUID()) val res = @@ -1672,7 +1674,6 @@ class AzurePubsubHandlerSpec mockitoEq(workspaceId.value), mockitoEq(disk.wsmResourceId.get.value) ) - diskStatus shouldBe DiskStatus.Deleted } msg = DeleteDiskV2Message(disk.id, workspaceId, cloudContextAzure, disk.wsmResourceId, None) From 2ff709d4ac2532b93fddfcea3abdf3cc50cd6bf5 Mon Sep 17 00:00:00 2001 From: jdcanas Date: Tue, 20 Aug 2024 09:21:59 -0700 Subject: [PATCH 2/6] compiling but unlikely tests work with current mocks --- .../workbench/leonardo/dao/HttpWsmDao.scala | 43 --- .../dsde/workbench/leonardo/dao/WsmDao.scala | 121 ------ .../leonardo/util/AzurePubsubHandler.scala | 29 +- .../workbench/leonardo/dao/MockWsmDAO.scala | 35 -- .../workbench/leonardo/dao/WsmCodecSpec.scala | 365 ------------------ .../LeoPubsubMessageSubscriberSpec.scala | 14 +- .../util/AzurePubsubHandlerSpec.scala | 130 +------ .../leonardo/util/AzureTestUtils.scala | 32 +- 8 files changed, 75 insertions(+), 694 deletions(-) delete mode 100644 http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmCodecSpec.scala diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala index 9b3f94f9f7..5e1cbf53d7 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala @@ -15,14 +15,12 @@ import org.broadinstitute.dsde.workbench.leonardo.dao.LandingZoneResourcePurpose WORKSPACE_BATCH_SUBNET } import org.broadinstitute.dsde.workbench.leonardo.dao.WsmDecoders._ -import org.broadinstitute.dsde.workbench.leonardo.dao.WsmEncoders._ import org.broadinstitute.dsde.workbench.leonardo.db.WsmResourceType import org.broadinstitute.dsde.workbench.leonardo.util.AppCreationException import org.broadinstitute.dsde.workbench.model.TraceId import org.broadinstitute.dsde.workbench.openTelemetry.OpenTelemetryMetrics import org.http4s._ import org.http4s.circe.CirceEntityDecoder._ -import org.http4s.circe.CirceEntityEncoder._ import org.http4s.client.Client import org.http4s.client.dsl.Http4sClientDsl import org.http4s.headers.{`Content-Type`, Authorization} @@ -252,26 +250,6 @@ class HttpWsmDao[F[_]](httpClient: Client[F], config: HttpWsmDaoConfig)(implicit )(onError) } yield resOpt.fold(List.empty[LandingZoneResourcesByPurpose])(res => res.resources) - override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[F, AppContext] - ): F[GetCreateVmJobResult] = - for { - ctx <- ev.ask - res <- httpClient.expectOr[GetCreateVmJobResult]( - Request[F]( - method = Method.GET, - uri = config.uri - .withPath( - Uri.Path - .unsafeFromString( - s"/api/workspaces/v1/${request.workspaceId.value.toString}/resources/controlled/azure/vm/create-result/${request.jobId.value}" - ) - ), - headers = headers(authorization, ctx.traceId, false) - ) - )(onError) - } yield res - override def getWorkspaceStorageContainer(workspaceId: WorkspaceId, authorization: Authorization)(implicit ev: Ask[F, AppContext] ): F[Option[StorageContainerResponse]] = for { @@ -319,27 +297,6 @@ class HttpWsmDao[F[_]](httpClient: Client[F], config: HttpWsmDaoConfig)(implicit )(onError) } yield resp - private def deleteHelper(req: DeleteWsmResourceRequest, authorization: Authorization, resource: String)(implicit - ev: Ask[F, AppContext] - ): F[Option[DeleteWsmResourceResult]] = - for { - ctx <- ev.ask - res <- httpClient.expectOptionOr[DeleteWsmResourceResult]( - Request[F]( - method = Method.POST, - uri = config.uri - .withPath( - Uri.Path - .unsafeFromString( - s"/api/workspaces/v1/${req.workspaceId.value.toString}/resources/controlled/azure/${resource}/${req.resourceId.value.toString}" - ) - ), - entity = req.deleteRequest, - headers = headers(authorization, ctx.traceId, true) - ) - )(onError) - } yield res - private def onError(response: Response[F])(implicit ev: Ask[F, AppContext]): F[Throwable] = for { context <- ev.ask diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala index 2cd73a259a..fd5fb2d1d4 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala @@ -5,22 +5,16 @@ import _root_.io.circe._ import _root_.io.circe.syntax._ import ca.mrvisser.sealerate import cats.mtl.Ask -import com.azure.resourcemanager.compute.models.VirtualMachineSizeTypes import org.broadinstitute.dsde.workbench.azure._ import org.broadinstitute.dsde.workbench.google2.RegionName //TODO: IA-4175 prune import org.broadinstitute.dsde.workbench.leonardo.JsonCodec.{ - azureImageEncoder, - azureMachineTypeEncoder, googleProjectDecoder, regionDecoder, - runtimeNameEncoder, storageContainerNameDecoder, storageContainerNameEncoder, - workspaceIdDecoder, wsmControlledResourceIdDecoder, wsmControlledResourceIdEncoder, - wsmJobIdDecoder, wsmJobIdEncoder } import org.broadinstitute.dsde.workbench.leonardo.dao.LandingZoneResourcePurpose.LandingZoneResourcePurpose @@ -30,13 +24,9 @@ import org.broadinstitute.dsde.workbench.model.google.GoogleProject import org.broadinstitute.dsde.workbench.model.{TraceId, WorkbenchEmail} import org.http4s.headers.Authorization -import java.time.ZonedDateTime import java.util.UUID trait WsmDao[F[_]] { - def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[F, AppContext] - ): F[GetCreateVmJobResult] def getLandingZoneResources(billingProfileId: BillingProfileId, userToken: Authorization)(implicit ev: Ask[F, AppContext] @@ -91,13 +81,6 @@ final case class LandingZoneResourcesByPurpose(purpose: LandingZoneResourcePurpo ) final case class ListLandingZoneResourcesResult(id: UUID, resources: List[LandingZoneResourcesByPurpose]) -//Azure Vm Models -final case class CreateVmRequest(workspaceId: WorkspaceId, - common: InternalDaoControlledResourceCommonFields, - vmData: CreateVmRequestData, - jobControl: WsmJobControl -) - final case class ProtectedSettings(fileUris: List[String], commandToExecute: String) final case class CustomScriptExtension(name: String, publisher: String, @@ -107,28 +90,11 @@ final case class CustomScriptExtension(name: String, protectedSettings: ProtectedSettings ) final case class StorageContainerResponse(name: ContainerName, resourceId: WsmControlledResourceId) -final case class CreateVmRequestData(name: RuntimeName, - vmSize: VirtualMachineSizeTypes, - vmImage: AzureImage, - customScriptExtension: CustomScriptExtension, - vmUserCredential: VMCredential, - diskId: WsmControlledResourceId -) final case class WsmVMMetadata(resourceId: WsmControlledResourceId) final case class WsmVMAttributes(region: RegionName) final case class WsmVm(metadata: WsmVMMetadata, attributes: WsmVMAttributes) -final case class DeleteWsmResourceRequest(workspaceId: WorkspaceId, - resourceId: WsmControlledResourceId, - deleteRequest: WsmDaoDeleteControlledAzureResourceRequest -) -final case class CreateVmResult(jobReport: WsmJobReport, errorReport: Option[WsmErrorReport]) - -final case class GetCreateVmJobResult(vm: Option[WsmVm], jobReport: WsmJobReport, errorReport: Option[WsmErrorReport]) - -final case class GetDeleteJobResult(jobReport: WsmJobReport, errorReport: Option[WsmErrorReport]) - sealed trait ResourceAttributes extends Serializable with Product object ResourceAttributes { final case class StorageContainerResourceAttributes(name: ContainerName) extends ResourceAttributes @@ -140,14 +106,6 @@ final case class GetWsmResourceResponse(resources: List[WsmResource]) final case class GetJobResultRequest(workspaceId: WorkspaceId, jobId: WsmJobId) // Azure Disk models -final case class CreateDiskRequest(workspaceId: WorkspaceId, - common: InternalDaoControlledResourceCommonFields, - diskData: CreateDiskRequestData -) - -final case class CreateDiskRequestData(name: AzureDiskName, size: DiskSize) - -final case class CreateDiskResponse(resourceId: WsmControlledResourceId) final case class CreateDiskForRuntimeResult(resourceId: WsmControlledResourceId, pollParams: Option[PollDiskParams]) @@ -165,20 +123,7 @@ final case class ControlledResourceName(value: String) extends AnyVal final case class ControlledResourceDescription(value: String) extends AnyVal final case class PrivateResourceUser(userName: WorkbenchEmail, privateResourceIamRoles: ControlledResourceIamRole) -final case class WsmErrorReport(message: String, statusCode: Int, causes: List[String]) -final case class WsmJobReport(id: WsmJobId, - description: String, - status: WsmJobStatus, - statusCode: Int, - submitted: ZonedDateTime, - completed: Option[ZonedDateTime], - resultUrl: String -) - final case class WsmJobControl(id: WsmJobId) -final case class WsmDaoDeleteControlledAzureResourceRequest(jobControl: WsmJobControl) - -final case class DeleteWsmResourceResult(jobReport: WsmJobReport, errorReport: Option[WsmErrorReport]) final case class WsmGcpContext(projectId: GoogleProject) @@ -270,11 +215,6 @@ object ManagedBy { // End Common Controlled resource models object WsmDecoders { - implicit val createDiskResponseDecoder: Decoder[CreateDiskResponse] = Decoder.instance { c => - for { - id <- c.downField("resourceId").as[UUID] - } yield CreateDiskResponse(WsmControlledResourceId(id)) - } implicit val metadataDecoder: Decoder[WsmVMMetadata] = Decoder.instance { c => for { @@ -288,13 +228,6 @@ object WsmDecoders { } yield WsmVMAttributes(region) } - implicit val createVmResponseDecoder: Decoder[WsmVm] = Decoder.instance { c => - for { - m <- c.downField("metadata").as[WsmVMMetadata] - a <- c.downField("attributes").as[WsmVMAttributes] - } yield WsmVm(m, a) - } - implicit val createStorageContainerResultDecoder: Decoder[CreateStorageContainerResult] = Decoder.forProduct1("resourceId")(CreateStorageContainerResult.apply) @@ -331,44 +264,9 @@ object WsmDecoders { implicit val wsmGcpContextDecoder: Decoder[WsmGcpContext] = Decoder.forProduct1("gcpContext")(WsmGcpContext.apply) - implicit val getWorkspaceResponseDecoder: Decoder[WorkspaceDescription] = Decoder.instance { c => - for { - id <- c.downField("id").as[WorkspaceId] - displayName <- c.downField("displayName").as[String] - spendProfile <- c.downField("spendProfile").as[String] - azureContext <- c.downField("azureContext").as[Option[AzureCloudContext]] - gcpContext <- c.downField("gcpContext").as[Option[WsmGcpContext]] - } yield WorkspaceDescription(id, displayName, spendProfile, azureContext, gcpContext.map(_.projectId)) - } - implicit val wsmJobStatusDecoder: Decoder[WsmJobStatus] = Decoder.decodeString.emap(s => WsmJobStatus.stringToObject.get(s).toRight(s"Invalid WsmJobStatus found: $s")) - implicit val wsmJobReportDecoder: Decoder[WsmJobReport] = Decoder.instance { c => - for { - id <- c.downField("id").as[WsmJobId] - description <- c.downField("description").as[String] - status <- c.downField("status").as[WsmJobStatus] - statusCode <- c.downField("statusCode").as[Int] - submitted <- c.downField("submitted").as[ZonedDateTime] - completed <- c.downField("completed").as[Option[ZonedDateTime]] - resultUrl <- c.downField("resultURL").as[String] - } yield WsmJobReport(id, description, status, statusCode, submitted, completed, resultUrl) - } - - implicit val wsmErrorReportDecoder: Decoder[WsmErrorReport] = - Decoder.forProduct3("message", "statusCode", "causes")(WsmErrorReport.apply) - - implicit val deleteControlledAzureResourceResponseDecoder: Decoder[DeleteWsmResourceResult] = Decoder.instance { c => - for { - jobReport <- c.downField("jobReport").as[WsmJobReport] - errorReport <- c.downField("errorReport").as[Option[WsmErrorReport]] - } yield DeleteWsmResourceResult(jobReport, errorReport) - } - - implicit val createVmResultDecoder: Decoder[CreateVmResult] = - Decoder.forProduct2("jobReport", "errorReport")(CreateVmResult.apply) - implicit val storageContainerResourceAttributesDecoder : Decoder[ResourceAttributes.StorageContainerResourceAttributes] = Decoder.forProduct1("storageContainerName")(ResourceAttributes.StorageContainerResourceAttributes.apply) @@ -383,10 +281,6 @@ object WsmDecoders { implicit val getRelayNamespaceDecoder: Decoder[GetWsmResourceResponse] = Decoder.forProduct1("resources")(GetWsmResourceResponse.apply) - implicit val getCreateVmResultDecoder: Decoder[GetCreateVmJobResult] = - Decoder.forProduct3("azureVm", "jobReport", "errorReport")(GetCreateVmJobResult.apply) - implicit val getDeleteVmResultDecoder: Decoder[GetDeleteJobResult] = - Decoder.forProduct2("jobReport", "errorReport")(GetDeleteJobResult.apply) } object WsmEncoders { @@ -413,11 +307,6 @@ object WsmEncoders { ) ) - implicit val diskRequestDataEncoder: Encoder[CreateDiskRequestData] = - Encoder.forProduct2("name", "size")(x => (x.name.value, x.size.gb)) - implicit val createDiskRequestEncoder: Encoder[CreateDiskRequest] = - Encoder.forProduct2("common", "azureDisk")(x => (x.common, x.diskData)) - implicit val protectedSettingsEncoder: Encoder[ProtectedSettings] = Encoder.instance { x => val fileUrisMap = Map( "key" -> "fileUris".asJson, @@ -439,23 +328,13 @@ object WsmEncoders { implicit val vmCrendentialnEncoder: Encoder[VMCredential] = Encoder.forProduct2("name", "password")(x => (x.username, x.password)) - implicit val vmRequestDataEncoder: Encoder[CreateVmRequestData] = - Encoder.forProduct6("name", "vmSize", "vmImage", "customScriptExtension", "vmUser", "diskId")(x => - (x.name, x.vmSize, x.vmImage, x.customScriptExtension, x.vmUserCredential, x.diskId) - ) implicit val wsmJobControlEncoder: Encoder[WsmJobControl] = Encoder.forProduct1("id")(x => x.id) - implicit val createVmRequestEncoder: Encoder[CreateVmRequest] = - Encoder.forProduct3("common", "azureVm", "jobControl")(x => (x.common, x.vmData, x.jobControl)) - implicit val storageContainerRequestEncoder: Encoder[StorageContainerRequest] = Encoder.forProduct1("storageContainerName")(x => x.storageContainerName) implicit val createStorageContainerRequestEncoder: Encoder[CreateStorageContainerRequest] = Encoder.forProduct2("common", "azureStorageContainer")(x => (x.commonFields, x.storageContainerReq)) - - implicit val deleteControlledAzureResourceRequestEncoder: Encoder[WsmDaoDeleteControlledAzureResourceRequest] = - Encoder.forProduct1("jobControl")(x => x.jobControl) } final case class WsmException(traceId: TraceId, message: String) extends Exception(message) diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala index cb39da739b..8bd9b288ca 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala @@ -17,6 +17,7 @@ import bio.terra.workspace.model.{ CreateControlledAzureResourceResult, CreateControlledAzureStorageContainerRequestBody, CreateControlledAzureVmRequestBody, + CreatedControlledAzureVmResult, DeleteControlledAzureResourceResult, JobControl, JobReport @@ -78,10 +79,6 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( // implicits necessary to poll on the status of external jobs implicit private def isJupyterUpDoneCheckable: DoneCheckable[Boolean] = (v: Boolean) => v - implicit private def wsmCreateVmDoneCheckable: DoneCheckable[GetCreateVmJobResult] = (v: GetCreateVmJobResult) => - v.jobReport.status.equals(WsmJobStatus.Succeeded) || v.jobReport.status == WsmJobStatus.Failed - implicit private def wsmDeleteDoneCheckable: DoneCheckable[GetDeleteJobResult] = (v: GetDeleteJobResult) => - v.jobReport.status.equals(WsmJobStatus.Succeeded) || v.jobReport.status == WsmJobStatus.Failed implicit private def wsmDeleteDoneControlledAzureResourceDoneCheckable : DoneCheckable[DeleteControlledAzureResourceResult] = (v: DeleteControlledAzureResourceResult) => @@ -93,6 +90,11 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( v.getJobReport.getStatus.equals(JobReport.StatusEnum.SUCCEEDED) || v.getJobReport.getStatus .equals(JobReport.StatusEnum.FAILED) + implicit private def wsmCreateAzureVmResultDoneCheckable: DoneCheckable[CreatedControlledAzureVmResult] = + (v: CreatedControlledAzureVmResult) => + v.getJobReport.getStatus.equals(JobReport.StatusEnum.SUCCEEDED) || v.getJobReport.getStatus + .equals(JobReport.StatusEnum.FAILED) + implicit private def vmStopDoneCheckable: DoneCheckable[Option[VirtualMachine]] = (v: Option[VirtualMachine]) => v.get.powerState() == PowerState.DEALLOCATED @@ -857,7 +859,10 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( ctx <- ev.ask auth <- samDAO.getLeoAuthToken createVmJobId = WsmJobId(s"create-vm-${params.runtime.id.toString.take(10)}") - getWsmVmJobResult = wsmDao.getCreateVmJobResult(GetJobResultRequest(params.workspaceId, createVmJobId), auth) + wsmControlledResourceClient <- buildWsmControlledResourceApiClient + getWsmVmJobResult = F.delay( + wsmControlledResourceClient.getCreateAzureVmResult(params.workspaceId.value, createVmJobId.value) + ) wsmApi <- buildWsmControlledResourceApiClient @@ -895,17 +900,17 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( config.createVmPollConfig.maxAttempts, config.createVmPollConfig.interval ).compile.lastOrError - _ <- resp.jobReport.status match { - case WsmJobStatus.Failed => + _ <- resp.getJobReport.getStatus match { + case JobReport.StatusEnum.FAILED => F.raiseError[Unit]( AzureRuntimeCreationError( params.runtime.id, params.workspaceId, - s"Wsm createVm job failed due to ${resp.errorReport.map(_.message).getOrElse("unknown")}", + s"Wsm createVm job failed due to ${resp.getErrorReport.getMessage}", params.useExistingDisk ) ) - case WsmJobStatus.Running => + case JobReport.StatusEnum.RUNNING => F.raiseError[Unit]( AzureRuntimeCreationError( params.runtime.id, @@ -914,7 +919,7 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( params.useExistingDisk ) ) - case WsmJobStatus.Succeeded => + case JobReport.StatusEnum.SUCCEEDED => val hostIp = s"${params.landingZoneResources.relayNamespace.value}.servicebus.windows.net" for { now <- nowInstant @@ -933,9 +938,11 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( s"Welder was not running within ${config.createVmPollConfig.maxAttempts} attempts with ${config.createVmPollConfig.interval} delay" ) _ <- clusterQuery.setToRunning(params.runtime.id, IP(hostIp), now).transaction + unsanitizedRegion = resp.getAzureVm.getAttributes.getRegion + region = if (unsanitizedRegion == null) None else Some(RegionName(unsanitizedRegion)) // Update runtime region to the VM region _ <- RuntimeConfigQueries - .updateRegion(params.runtime.runtimeConfigId, resp.vm.map(_.attributes.region)) + .updateRegion(params.runtime.runtimeConfigId, region) .transaction _ <- logger.info(ctx.loggingCtx)("runtime is ready") } yield () diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/MockWsmDAO.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/MockWsmDAO.scala index 20b2874f11..16da5bb075 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/MockWsmDAO.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/MockWsmDAO.scala @@ -4,46 +4,11 @@ package dao import cats.effect.IO import cats.mtl.Ask import org.broadinstitute.dsde.workbench.azure._ -import org.broadinstitute.dsde.workbench.google2.RegionName import org.http4s.headers.Authorization -import java.time.ZonedDateTime import java.util.UUID class MockWsmDAO(jobStatus: WsmJobStatus = WsmJobStatus.Succeeded) extends WsmDao[IO] { - - override def getCreateVmJobResult( - request: GetJobResultRequest, - authorization: Authorization - )(implicit ev: Ask[IO, AppContext]): IO[GetCreateVmJobResult] = - IO.pure( - GetCreateVmJobResult( - Some( - WsmVm(WsmVMMetadata(WsmControlledResourceId(UUID.randomUUID())), - WsmVMAttributes(RegionName("southcentralus")) - ) - ), - WsmJobReport( - request.jobId, - "desc", - jobStatus, - 200, - ZonedDateTime.parse("2022-03-18T15:02:29.264756Z"), - Some(ZonedDateTime.parse("2022-03-18T15:02:29.264756Z")), - "resultUrl" - ), - if (jobStatus.equals(WsmJobStatus.Failed)) - Some( - WsmErrorReport( - "error", - 500, - List.empty - ) - ) - else None - ) - ) - override def getLandingZoneResources(billingProfileId: BillingProfileId, userToken: Authorization)(implicit ev: Ask[IO, AppContext] ): IO[LandingZoneResources] = diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmCodecSpec.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmCodecSpec.scala deleted file mode 100644 index 1421b5679d..0000000000 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmCodecSpec.scala +++ /dev/null @@ -1,365 +0,0 @@ -package org.broadinstitute.dsde.workbench.leonardo -package dao - -import _root_.io.circe.syntax._ -import com.azure.resourcemanager.compute.models.VirtualMachineSizeTypes -import io.circe.parser._ -import org.broadinstitute.dsde.workbench.google2.RegionName -import org.broadinstitute.dsde.workbench.leonardo.CommonTestData._ -import org.broadinstitute.dsde.workbench.leonardo.dao.WsmDecoders._ -import org.broadinstitute.dsde.workbench.leonardo.dao.WsmEncoders._ -import org.broadinstitute.dsde.workbench.leonardo.http.ConfigReader -import org.broadinstitute.dsde.workbench.leonardo.http.service.VMCredential -import org.scalatest.flatspec.AnyFlatSpec -import org.scalatest.matchers.should.Matchers - -import java.time.ZonedDateTime -import java.util.UUID - -class WsmCodecSpec extends AnyFlatSpec with Matchers { - it should "encode CreateDiskRequest" in { - val req = CreateDiskRequest( - workspaceId, - testCommonControlledResourceFields, - CreateDiskRequestData( - AzureDiskName("disk"), - DiskSize(50) - ) - ).asJson.deepDropNullValues.noSpaces - - req shouldBe - """ - |{ - | "common" : { - | "name" : "name", - | "description" : "desc", - | "cloningInstructions" : "COPY_NOTHING", - | "accessScope" : "PRIVATE_ACCESS", - | "managedBy" : "USER", - | "privateResourceUser" : { - | "userName" : "user1@example.com", - | "privateResourceIamRole" : "EDITOR" - | } - | }, - | "azureDisk" : { - | "name" : "disk", - | "size": 50 - | } - |} - |""".stripMargin.replaceAll("\\s", "") - } - - it should "encode CreateVmRequest" in { - val fixedUUID = UUID.randomUUID() - - val req = CreateVmRequest( - workspaceId, - testCommonControlledResourceFields, - CreateVmRequestData( - RuntimeName("runtime"), - VirtualMachineSizeTypes.STANDARD_A2, // Standard_A2 - ConfigReader.appConfig.azure.pubsubHandler.runtimeDefaults.image, - CustomScriptExtension( - name = ConfigReader.appConfig.azure.pubsubHandler.runtimeDefaults.customScriptExtension.name, - publisher = ConfigReader.appConfig.azure.pubsubHandler.runtimeDefaults.customScriptExtension.publisher, - `type` = ConfigReader.appConfig.azure.pubsubHandler.runtimeDefaults.customScriptExtension.`type`, - version = ConfigReader.appConfig.azure.pubsubHandler.runtimeDefaults.customScriptExtension.version, - minorVersionAutoUpgrade = - ConfigReader.appConfig.azure.pubsubHandler.runtimeDefaults.customScriptExtension.minorVersionAutoUpgrade, - protectedSettings = ProtectedSettings( - ConfigReader.appConfig.azure.pubsubHandler.runtimeDefaults.customScriptExtension.fileUris, - "" - ) - ), - VMCredential("username", "password"), - WsmControlledResourceId(fixedUUID) - ), - WsmJobControl(WsmJobId("job1")) - ).asJson.deepDropNullValues.noSpaces - - req shouldBe - s""" - |{ - | "common" : { - | "name" : "name", - | "description" : "desc", - | "cloningInstructions" : "COPY_NOTHING", - | "accessScope" : "PRIVATE_ACCESS", - | "managedBy" : "USER", - | "privateResourceUser" : { - | "userName" : "user1@example.com", - | "privateResourceIamRole" : "EDITOR" - | } - | }, - | "azureVm" : { - | "name" : "runtime", - | "vmSize": "Standard_A2", - | "vmImage": { - | "publisher": "microsoft-dsvm", - | "offer": "ubuntu-2004", - | "sku": "2004-gen2", - | "version": "23.04.24" - | }, - | "customScriptExtension": { - | "name": "vm-custom-script-extension", - | "publisher": "Microsoft.Azure.Extensions", - | "type": "CustomScript", - | "version": "2.1", - | "minorVersionAutoUpgrade": true, - | "protectedSettings": [{ - | "key": "fileUris", - | "value": ["https://raw.githubusercontent.com/DataBiosphere/leonardo/8390d25ccd761fb206cf388560a571be77a42bbd/http/src/main/resources/init-resources/azure_vm_init_script.sh"] - | }, - | { - | "key": "commandToExecute", - | "value": "" - | } - | ] - | }, - | "vmUser":{"name":"username","password":"password"}, - | "diskId": "${fixedUUID.toString}" - | }, - | "jobControl": { - | "id": "job1" - | } - |} - |""".stripMargin.replaceAll("\\s", "") - } - - it should "encode DeleteVmRequest" in { - val fixedUUID = UUID.randomUUID().toString - val req = WsmDaoDeleteControlledAzureResourceRequest(WsmJobControl(WsmJobId(fixedUUID))) - - req.asJson.deepDropNullValues.noSpaces shouldBe - s""" - |{ - | "jobControl": { - | "id": "${fixedUUID.toString}" - | } - |} - |""".stripMargin.replaceAll("\\s", "") - - } - - it should "decode CreateDiskResponse" in { - val fixedUUID = UUID.randomUUID() - val expected = CreateDiskResponse(WsmControlledResourceId(fixedUUID)) - - val decodedResp = decode[CreateDiskResponse]( - s""" - |{ - | "resourceId": "${fixedUUID.toString}", - | "azureNetwork": { - | "fillerFieldsThatAreNotDecoded": "filler" - | } - |} - |""".stripMargin.replaceAll("\\s", "") - ) - - decodedResp shouldBe Right(expected) - } - - it should "decode CreateVmResult" in { - val jobId = WsmJobId("job1") - val expected = CreateVmResult( - WsmJobReport( - jobId, - "desc", - WsmJobStatus.Running, - 200, - ZonedDateTime.parse("2022-03-18T15:02:29.264756Z"), - Some(ZonedDateTime.parse("2022-03-18T15:02:29.264756Z")), - "resultUrl" - ), - Some( - WsmErrorReport( - "error", - 500, - List("testCause") - ) - ) - ) - - val decodedResp = decode[CreateVmResult]( - s""" - |{ - | "jobReport": { - | "id": "${jobId.value}", - | "description": "desc", - | "status": "RUNNING", - | "statusCode": 200, - | "submitted": "2022-03-18T15:02:29.264756Z", - | "completed": "2022-03-18T15:02:29.264756Z", - | "resultURL": "resultUrl" - | }, - | "errorReport": { - | "message": "error", - | "statusCode": 500, - | "causes": ["testCause"] - | } - |} - |""".stripMargin.replaceAll("\\s", "") - ) - - decodedResp shouldBe Right(expected) - - val decodedResp2 = decode[CreateVmResult]( - s""" - |{ - | "jobReport": - | { - | "id": "job2", - | "description": "Create controlled resource CONTROLLED_AZURE_VM; id 635e25e1-c793-4ca9-b9fe-9055cdae2f26; name automation-test-aswsimhjz", - | "status": "RUNNING", - | "statusCode": 202, - | "submitted": "2022-03-18T15:02:29.264756Z", - | "resultURL": "https://workspace.dsde-dev.broadinstitute.org/api/workspaces/v1/e1aaf25b-b298-46eb-891b-e4c326f29b0c/resources/controlled/azure/vm/create-result/1bf4d89f-53ac-4ad4-ab8e-0131c6494a69" - | } - |} - |""".stripMargin - ) - - val expected2 = CreateVmResult( - WsmJobReport( - WsmJobId("job2"), - "Create controlled resource CONTROLLED_AZURE_VM; id 635e25e1-c793-4ca9-b9fe-9055cdae2f26; name automation-test-aswsimhjz", - WsmJobStatus.Running, - 202, - ZonedDateTime.parse("2022-03-18T15:02:29.264756Z"), - None, - "https://workspace.dsde-dev.broadinstitute.org/api/workspaces/v1/e1aaf25b-b298-46eb-891b-e4c326f29b0c/resources/controlled/azure/vm/create-result/1bf4d89f-53ac-4ad4-ab8e-0131c6494a69" - ), - None - ) - decodedResp2 shouldBe Right(expected2) - } - - it should "decode getCreateVmResult" in { - val jobId = WsmJobId("job1") - val expected = GetCreateVmJobResult( - Some( - WsmVm( - WsmVMMetadata(WsmControlledResourceId(UUID.fromString("dcfa6fa4-ab46-465e-a8dd-76705cbdb4ec"))), - WsmVMAttributes(RegionName("westcentralus")) - ) - ), - WsmJobReport( - jobId, - "desc", - WsmJobStatus.Running, - 200, - ZonedDateTime.parse("2022-03-18T15:02:29.264756Z"), - Some(ZonedDateTime.parse("2022-03-18T15:02:29.264756Z")), - "resultUrl" - ), - Some( - WsmErrorReport( - "error", - 500, - List("testCause") - ) - ) - ) - - val decodedResp = decode[GetCreateVmJobResult]( - s""" - |{ - | "azureVm": { - | "metadata": - | { - | "workspaceId": "e1aaf25b-b298-46eb-891b-e4c326f29b0c", - | "resourceId": "dcfa6fa4-ab46-465e-a8dd-76705cbdb4ec", - | "name": "automation-test-afalskknz", - | "description": "Azure Vm", - | "resourceType": "AZURE_VM", - | "stewardshipType": "CONTROLLED", - | "cloningInstructions": "COPY_NOTHING", - | "controlledResourceMetadata": - | { - | "accessScope": "PRIVATE_ACCESS", - | "managedBy": "APPLICATION", - | "privateResourceUser": - | { - | "userName": "ron.weasley@test.firecloud.org" - | }, - | "privateResourceState": "ACTIVE" - | } - | }, - | "attributes": - | { - | "vmName": "automation-test-afalskknz", - | "region": "westcentralus", - | "vmSize": "Standard_D1_v2", - | "vmImageUri": "/subscriptions/3efc5bdf-be0e-44e7-b1d7-c08931e3c16c/resourceGroups/mrg-qi-1-preview-20210517084351/providers/Microsoft.Compute/galleries/msdsvm/images/customized_ms_dsvm/versions/0.1.0", - | "ipId": "62e6dec2-94c5-4806-8594-eb6020344cbe", - | "diskId": "2eddd6aa-bb94-4027-aeca-0de34a583808", - | "networkId": "b414a42b-a27d-4072-8a03-44283f4c07f6" - | } - | }, - | "jobReport": { - | "id": "${jobId.value}", - | "description": "desc", - | "status": "RUNNING", - | "statusCode": 200, - | "submitted": "2022-03-18T15:02:29.264756Z", - | "completed": "2022-03-18T15:02:29.264756Z", - | "resultURL": "resultUrl" - | }, - | "errorReport": { - | "message": "error", - | "statusCode": 500, - | "causes": ["testCause"] - | } - |} - |""".stripMargin.replaceAll("\\s", "") - ) - - decodedResp shouldBe Right(expected) - } - - it should "decode DeleteVmResult" in { - val fixedUUID = UUID.randomUUID().toString - val now = ZonedDateTime.now() - val expected = DeleteWsmResourceResult( - WsmJobReport( - WsmJobId(fixedUUID), - "desc", - WsmJobStatus.Succeeded, - 200, - now, - Some(now), - "resultUrl" - ), - Some( - WsmErrorReport( - "error", - 500, - List("testCause") - ) - ) - ) - - val decodedResp = decode[DeleteWsmResourceResult]( - s""" - |{ - | "jobReport": { - | "id": "${fixedUUID.toString}", - | "description": "desc", - | "status": "SUCCEEDED", - | "statusCode": 200, - | "submitted": "${now.toString}", - | "completed": "${now.toString}", - | "resultURL": "resultUrl" - | }, - | "errorReport": { - | "message": "error", - | "statusCode": 500, - | "causes": ["testCause"] - | } - |} - |""".stripMargin.replaceAll("\\s", "") - ) - - decodedResp shouldBe Right(expected) - } -} diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/LeoPubsubMessageSubscriberSpec.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/LeoPubsubMessageSubscriberSpec.scala index f08454c303..c29b4cfc40 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/LeoPubsubMessageSubscriberSpec.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/LeoPubsubMessageSubscriberSpec.scala @@ -1929,15 +1929,17 @@ class LeoPubsubMessageSubscriberSpec it should "handle top-level error in create azure vm properly" in isolatedDbTest { val exceptionMsg = "test exception" - val mockWsmDao = new MockWsmDAO { - override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[GetCreateVmJobResult] = - IO.raiseError(new Exception(exceptionMsg)) + + val (mockWsm, controlledResourceApi, _, _) = AzureTestUtils.setUpMockWsmApiClientProvider() + when { + controlledResourceApi.getCreateAzureVmResult(any, any) + } thenAnswer { + throw new Exception(exceptionMsg) } val mockAckConsumer = mock[AckHandler] val queue = makeTaskQueue() - val leoSubscriber = makeLeoSubscriber(azureInterp = makeAzureInterp(asyncTaskQueue = queue, wsmDAO = mockWsmDao), + val leoSubscriber = makeLeoSubscriber(azureInterp = + makeAzureInterp(asyncTaskQueue = queue, mockWsmClient = mockWsm), asyncTaskQueue = queue ) diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala index e03280c35e..d2adc437e2 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala @@ -83,20 +83,8 @@ class AzurePubsubHandlerSpec val queue = QueueFactory.asyncTaskQueue() val resourceId = WsmControlledResourceId(UUID.randomUUID()) - val mockWsmDAO = new MockWsmDAO { - override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[GetCreateVmJobResult] = - IO.pure( - GetCreateVmJobResult( - Some(WsmVm(WsmVMMetadata(resourceId), WsmVMAttributes(RegionName("southcentralus")))), - WsmJobReport(WsmJobId("job1"), "", WsmJobStatus.Succeeded, 200, ZonedDateTime.now(), None, "url"), - None - ) - ) - } val azurePubsubHandler = - makeAzurePubsubHandler(asyncTaskQueue = queue, wsmDAO = mockWsmDAO) + makeAzurePubsubHandler(asyncTaskQueue = queue) val res = for { @@ -153,22 +141,8 @@ class AzurePubsubHandlerSpec val resourceId = WsmControlledResourceId(UUID.randomUUID()) val startTime: Instant = Instant.now val mockLatencyMillis = 5000 - val mockWsmDAO = new MockWsmDAO { - override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[GetCreateVmJobResult] = { - Thread.sleep(mockLatencyMillis) - IO.pure( - GetCreateVmJobResult( - Some(WsmVm(WsmVMMetadata(resourceId), WsmVMAttributes(RegionName("southcentralus")))), - WsmJobReport(WsmJobId("job1"), "", WsmJobStatus.Succeeded, 200, ZonedDateTime.now(), None, "url"), - None - ) - ) - } - } val azurePubsubHandler = - makeAzurePubsubHandler(asyncTaskQueue = queue, wsmDAO = mockWsmDAO) + makeAzurePubsubHandler(asyncTaskQueue = queue) val res = for { @@ -224,20 +198,8 @@ class AzurePubsubHandlerSpec val queue = QueueFactory.asyncTaskQueue() val resourceId = WsmControlledResourceId(UUID.randomUUID()) - val mockWsmDAO = new MockWsmDAO { - override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[GetCreateVmJobResult] = - IO.pure( - GetCreateVmJobResult( - Some(WsmVm(WsmVMMetadata(resourceId), WsmVMAttributes(RegionName("southcentralus")))), - WsmJobReport(WsmJobId("job1"), "", WsmJobStatus.Succeeded, 200, ZonedDateTime.now(), None, "url"), - None - ) - ) - } val azurePubsubHandler = - makeAzurePubsubHandler(asyncTaskQueue = queue, wsmDAO = mockWsmDAO) + makeAzurePubsubHandler(asyncTaskQueue = queue) val res = for { @@ -296,18 +258,6 @@ class AzurePubsubHandlerSpec it should "create an azure vm properly with an action managed identity" in isolatedDbTest { val queue = QueueFactory.asyncTaskQueue() val resourceId = WsmControlledResourceId(UUID.randomUUID()) - val mockWsmDAO = new MockWsmDAO { - override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[GetCreateVmJobResult] = - IO.pure( - GetCreateVmJobResult( - Some(WsmVm(WsmVMMetadata(resourceId), WsmVMAttributes(RegionName("southcentralus")))), - WsmJobReport(WsmJobId("job1"), "", WsmJobStatus.Succeeded, 200, ZonedDateTime.now(), None, "url"), - None - ) - ) - } val mockSamDAO = new MockSamDAO { override def getAzureActionManagedIdentity(authHeader: Authorization, resource: PrivateAzureStorageAccountSamResourceId, @@ -315,7 +265,7 @@ class AzurePubsubHandlerSpec )(implicit ev: Ask[IO, TraceId]): IO[Option[String]] = IO(Some("awesome-identity")) } val azurePubsubHandler = - makeAzurePubsubHandler(asyncTaskQueue = queue, wsmDAO = mockWsmDAO, samDAO = mockSamDAO) + makeAzurePubsubHandler(asyncTaskQueue = queue, samDAO = mockSamDAO) val res = for { @@ -370,20 +320,10 @@ class AzurePubsubHandlerSpec val queue = QueueFactory.asyncTaskQueue() val resourceId = WsmControlledResourceId(UUID.randomUUID()) - val mockWsmDAO = new MockWsmDAO { - override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[GetCreateVmJobResult] = - IO.pure( - GetCreateVmJobResult( - Some(WsmVm(WsmVMMetadata(resourceId), WsmVMAttributes(RegionName("southcentralus")))), - WsmJobReport(WsmJobId("job1"), "", WsmJobStatus.Failed, 200, ZonedDateTime.now(), None, "url"), - None - ) - ) - } + val (mockWsm, _, _, _) = AzureTestUtils.setUpMockWsmApiClientProvider(vmJobStatus = JobReport.StatusEnum.FAILED) + val azurePubsubHandler = - makeAzurePubsubHandler(asyncTaskQueue = queue, wsmDAO = mockWsmDAO) + makeAzurePubsubHandler(asyncTaskQueue = queue, wsmClient = mockWsm) val res = for { @@ -473,23 +413,11 @@ class AzurePubsubHandlerSpec val queue = QueueFactory.asyncTaskQueue() val resourceId = WsmControlledResourceId(UUID.randomUUID()) - val mockWsmDAO = new MockWsmDAO { - override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[GetCreateVmJobResult] = - IO.pure( - GetCreateVmJobResult( - Some(WsmVm(WsmVMMetadata(resourceId), WsmVMAttributes(RegionName("southcentralus")))), - WsmJobReport(WsmJobId("job1"), "", WsmJobStatus.Succeeded, 200, ZonedDateTime.now(), None, "url"), - None - ) - ) - } val fakeWelderDao = new MockWelderDAO() { override def isProxyAvailable(cloudContext: CloudContext, clusterName: RuntimeName): IO[Boolean] = IO.pure(false) } val azurePubsubHandler = - makeAzurePubsubHandler(asyncTaskQueue = queue, wsmDAO = mockWsmDAO, welderDao = fakeWelderDao) + makeAzurePubsubHandler(asyncTaskQueue = queue, welderDao = fakeWelderDao) val res = for { @@ -778,12 +706,15 @@ class AzurePubsubHandlerSpec it should "handle vm creation error in async task properly" in isolatedDbTest { val queue = QueueFactory.asyncTaskQueue() val exceptionMsg = "test exception" - val mockWsmDao = new MockWsmDAO { - override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[GetCreateVmJobResult] = IO.raiseError(new Exception(exceptionMsg)) + val (mockWsm, controlledResourceApi, _, _) = + AzureTestUtils.setUpMockWsmApiClientProvider(JobReport.StatusEnum.FAILED) + + when { + controlledResourceApi.getCreateAzureVmResult(any, any) + } thenAnswer { + throw new Exception(exceptionMsg) } - val azureInterp = makeAzurePubsubHandler(asyncTaskQueue = queue, wsmDAO = mockWsmDao) + val azureInterp = makeAzurePubsubHandler(asyncTaskQueue = queue, wsmClient = mockWsm) val res = for { @@ -826,19 +757,7 @@ class AzurePubsubHandlerSpec it should "fail to create runtime with persistent disk if no resourceId" in isolatedDbTest { val queue = QueueFactory.asyncTaskQueue() val resourceId = WsmControlledResourceId(UUID.randomUUID()) - val mockWsmDAO = new MockWsmDAO { - override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[GetCreateVmJobResult] = - IO.pure( - GetCreateVmJobResult( - Some(WsmVm(WsmVMMetadata(resourceId), WsmVMAttributes(RegionName("southcentralus")))), - WsmJobReport(WsmJobId("job1"), "", WsmJobStatus.Succeeded, 200, ZonedDateTime.now(), None, "url"), - None - ) - ) - } - val azureInterp = makeAzurePubsubHandler(asyncTaskQueue = queue, wsmDAO = mockWsmDAO) + val azureInterp = makeAzurePubsubHandler(asyncTaskQueue = queue) val res = for { @@ -873,19 +792,8 @@ class AzurePubsubHandlerSpec it should "fail to create runtime with persistent disk if WSMresource not found" in isolatedDbTest { val queue = QueueFactory.asyncTaskQueue() val resourceId = WsmControlledResourceId(UUID.randomUUID()) - val mockWsmDAO = new MockWsmDAO { - override def getCreateVmJobResult(request: GetJobResultRequest, authorization: Authorization)(implicit - ev: Ask[IO, AppContext] - ): IO[GetCreateVmJobResult] = - IO.pure( - GetCreateVmJobResult( - Some(WsmVm(WsmVMMetadata(resourceId), WsmVMAttributes(RegionName("southcentralus")))), - WsmJobReport(WsmJobId("job1"), "", WsmJobStatus.Succeeded, 200, ZonedDateTime.now(), None, "url"), - None - ) - ) - } - val azureInterp = makeAzurePubsubHandler(asyncTaskQueue = queue, wsmDAO = mockWsmDAO) + + val azureInterp = makeAzurePubsubHandler(asyncTaskQueue = queue) val res = for { diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala index 0f277d0049..7179172b04 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala @@ -6,19 +6,23 @@ import org.mockito.Mockito.when import org.mockito.ArgumentMatchers.any import bio.terra.workspace.api.{ControlledAzureResourceApi, ResourceApi, WorkspaceApi} import bio.terra.workspace.model.{ + AzureVmAttributes, + AzureVmResource, CreateControlledAzureDiskRequestV2Body, CreateControlledAzureResourceResult, CreatedControlledAzureStorageContainer, + CreatedControlledAzureVmResult, DeleteControlledAzureResourceResult, ErrorReport, IamRole, - JobReport + JobReport, + ResourceMetadata } import cats.mtl.Ask import com.azure.resourcemanager.compute.models.{PowerState, VirtualMachine} import org.broadinstitute.dsde.workbench.azure.{AzureCloudContext, ManagedResourceGroupName, SubscriptionId, TenantId} import org.broadinstitute.dsde.workbench.azure.mock.FakeAzureVmService -import org.broadinstitute.dsde.workbench.leonardo.CommonTestData.wsmWorkspaceDesc +import org.broadinstitute.dsde.workbench.leonardo.CommonTestData.{azureRegion, wsmWorkspaceDesc} import org.broadinstitute.dsde.workbench.leonardo.{AppContext, WorkspaceId} import org.broadinstitute.dsde.workbench.leonardo.dao.{ MockWsmClientProvider, @@ -107,6 +111,30 @@ object AzureTestUtils extends MockitoSugar { .errorReport(new ErrorReport()) } + // create vm result + when { + api.createAzureVm(any, any) + } thenAnswer { _ => + new CreatedControlledAzureVmResult() + .jobReport( + new JobReport().status(vmJobStatus) + ) + .azureVm(new AzureVmResource().attributes(new AzureVmAttributes().region("southcentralus"))) + .errorReport(new ErrorReport()) + } + + // create vm result + when { + api.getCreateAzureVmResult(any, any) + } thenAnswer { _ => + new CreatedControlledAzureVmResult() + .jobReport( + new JobReport().status(vmJobStatus) + ) + .azureVm(new AzureVmResource().attributes(new AzureVmAttributes().region("southcentralus"))) + .errorReport(new ErrorReport()) + } + // delete vm when { api.deleteAzureVm(any, any, any) From 7d641815230e93cf2eacdb9736775f02c07032aa Mon Sep 17 00:00:00 2001 From: jdcanas Date: Tue, 20 Aug 2024 10:45:46 -0700 Subject: [PATCH 3/6] test fixes --- .../LeoPubsubMessageSubscriberSpec.scala | 6 ++--- .../util/AzurePubsubHandlerSpec.scala | 5 ++-- .../leonardo/util/AzureTestUtils.scala | 23 +++++++++++++++---- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/LeoPubsubMessageSubscriberSpec.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/LeoPubsubMessageSubscriberSpec.scala index c29b4cfc40..ae50737ae0 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/LeoPubsubMessageSubscriberSpec.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/monitor/LeoPubsubMessageSubscriberSpec.scala @@ -3,6 +3,7 @@ package monitor import akka.actor.ActorSystem import akka.testkit.TestKit +import bio.terra.workspace.client.ApiException import bio.terra.workspace.model.JobReport import cats.data.Kleisli import cats.effect.IO @@ -58,7 +59,6 @@ import org.broadinstitute.dsde.workbench.openTelemetry.OpenTelemetryMetrics import org.broadinstitute.dsde.workbench.util2.messaging.{AckHandler, CloudSubscriber, ReceivedMessage} import org.broadinstitute.dsp._ import org.broadinstitute.dsp.mocks.MockHelm -import org.http4s.headers.Authorization import org.mockito.ArgumentMatchers.{any, anyBoolean, startsWith} import org.mockito.Mockito._ import org.scalatest.BeforeAndAfterEach @@ -1933,8 +1933,8 @@ class LeoPubsubMessageSubscriberSpec val (mockWsm, controlledResourceApi, _, _) = AzureTestUtils.setUpMockWsmApiClientProvider() when { controlledResourceApi.getCreateAzureVmResult(any, any) - } thenAnswer { - throw new Exception(exceptionMsg) + } thenThrow { + new ApiException(exceptionMsg) } val mockAckConsumer = mock[AckHandler] val queue = makeTaskQueue() diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala index d2adc437e2..28a373d3f9 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala @@ -3,6 +3,7 @@ package util import akka.actor.ActorSystem import akka.testkit.TestKit +import bio.terra.workspace.client.ApiException import bio.terra.workspace.model.{DeleteControlledAzureResourceRequest, JobReport} import cats.effect.IO import cats.effect.std.Queue @@ -711,8 +712,8 @@ class AzurePubsubHandlerSpec when { controlledResourceApi.getCreateAzureVmResult(any, any) - } thenAnswer { - throw new Exception(exceptionMsg) + } thenThrow { + new ApiException(exceptionMsg) } val azureInterp = makeAzurePubsubHandler(asyncTaskQueue = queue, wsmClient = mockWsm) diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala index 7179172b04..b9ceb03b37 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala @@ -8,6 +8,7 @@ import bio.terra.workspace.api.{ControlledAzureResourceApi, ResourceApi, Workspa import bio.terra.workspace.model.{ AzureVmAttributes, AzureVmResource, + CreateControlledAzureDiskRequestBody, CreateControlledAzureDiskRequestV2Body, CreateControlledAzureResourceResult, CreatedControlledAzureStorageContainer, @@ -53,7 +54,7 @@ object AzureTestUtils extends MockitoSugar { val resourceApi = mock[ResourceApi] val disksByJob = mutable.Map.empty[String, CreateControlledAzureDiskRequestV2Body] - // Create disk + // Create disk v2 when { api.createAzureDiskV2(any, any) } thenAnswer { invocation => @@ -64,7 +65,19 @@ object AzureTestUtils extends MockitoSugar { .jobReport( new JobReport().status(diskJobStatus) ) - .errorReport(new ErrorReport()) + .errorReport(new ErrorReport().message("test exception")) + } + + // Create disk + when { + api.createAzureDisk(any, any) + } thenAnswer { invocation => + val requestBody = invocation.getArgument[CreateControlledAzureDiskRequestBody](0) + new CreateControlledAzureResourceResult() + .jobReport( + new JobReport().status(diskJobStatus) + ) + .errorReport(new ErrorReport().message("test exception")) } // Get disk result @@ -77,7 +90,7 @@ object AzureTestUtils extends MockitoSugar { .jobReport( new JobReport().status(diskJobStatus) ) - .errorReport(new ErrorReport()) + .errorReport(new ErrorReport().message("test exception")) } // Create storage container @@ -120,7 +133,7 @@ object AzureTestUtils extends MockitoSugar { new JobReport().status(vmJobStatus) ) .azureVm(new AzureVmResource().attributes(new AzureVmAttributes().region("southcentralus"))) - .errorReport(new ErrorReport()) + .errorReport(new ErrorReport().message("test exception")) } // create vm result @@ -132,7 +145,7 @@ object AzureTestUtils extends MockitoSugar { new JobReport().status(vmJobStatus) ) .azureVm(new AzureVmResource().attributes(new AzureVmAttributes().region("southcentralus"))) - .errorReport(new ErrorReport()) + .errorReport(new ErrorReport().message("test exception")) } // delete vm From 1225d9448078357756b204bcc195df2aa6e5ffd6 Mon Sep 17 00:00:00 2001 From: jdcanas Date: Tue, 20 Aug 2024 12:41:08 -0700 Subject: [PATCH 4/6] final test fixes --- .../dsde/workbench/leonardo/dao/WsmDao.scala | 97 +------------------ .../leonardo/util/AKSInterpreter.scala | 1 - .../leonardo/util/AzurePubsubHandler.scala | 24 ----- .../http/service/AdminServiceInterpSpec.scala | 2 - .../util/AzurePubsubHandlerSpec.scala | 38 +++++--- .../leonardo/util/AzureTestUtils.scala | 12 +-- 6 files changed, 27 insertions(+), 147 deletions(-) diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala index fd5fb2d1d4..798adf2f6c 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala @@ -2,23 +2,15 @@ package org.broadinstitute.dsde.workbench.leonardo package dao import _root_.io.circe._ -import _root_.io.circe.syntax._ import ca.mrvisser.sealerate import cats.mtl.Ask import org.broadinstitute.dsde.workbench.azure._ -import org.broadinstitute.dsde.workbench.google2.RegionName -//TODO: IA-4175 prune import org.broadinstitute.dsde.workbench.leonardo.JsonCodec.{ googleProjectDecoder, - regionDecoder, storageContainerNameDecoder, - storageContainerNameEncoder, - wsmControlledResourceIdDecoder, - wsmControlledResourceIdEncoder, - wsmJobIdEncoder + wsmControlledResourceIdDecoder } import org.broadinstitute.dsde.workbench.leonardo.dao.LandingZoneResourcePurpose.LandingZoneResourcePurpose -import org.broadinstitute.dsde.workbench.leonardo.http.service.VMCredential import org.broadinstitute.dsde.workbench.leonardo.util.PollDiskParams import org.broadinstitute.dsde.workbench.model.google.GoogleProject import org.broadinstitute.dsde.workbench.model.{TraceId, WorkbenchEmail} @@ -39,10 +31,6 @@ trait WsmDao[F[_]] { } final case class StorageContainerRequest(storageContainerName: ContainerName) -final case class CreateStorageContainerRequest(workspaceId: WorkspaceId, - commonFields: InternalDaoControlledResourceCommonFields, - storageContainerReq: StorageContainerRequest -) final case class CreateStorageContainerResult(resourceId: WsmControlledResourceId) final case class WorkspaceDescription(id: WorkspaceId, displayName: String, @@ -80,21 +68,8 @@ final case class LandingZoneResourcesByPurpose(purpose: LandingZoneResourcePurpo deployedResources: List[LandingZoneResource] ) final case class ListLandingZoneResourcesResult(id: UUID, resources: List[LandingZoneResourcesByPurpose]) - -final case class ProtectedSettings(fileUris: List[String], commandToExecute: String) -final case class CustomScriptExtension(name: String, - publisher: String, - `type`: String, - version: String, - minorVersionAutoUpgrade: Boolean, - protectedSettings: ProtectedSettings -) final case class StorageContainerResponse(name: ContainerName, resourceId: WsmControlledResourceId) -final case class WsmVMMetadata(resourceId: WsmControlledResourceId) -final case class WsmVMAttributes(region: RegionName) -final case class WsmVm(metadata: WsmVMMetadata, attributes: WsmVMAttributes) - sealed trait ResourceAttributes extends Serializable with Product object ResourceAttributes { final case class StorageContainerResourceAttributes(name: ContainerName) extends ResourceAttributes @@ -103,7 +78,6 @@ object ResourceAttributes { final case class WsmResourceMetadata(resourceId: WsmControlledResourceId) final case class WsmResource(metadata: WsmResourceMetadata, resourceAttributes: ResourceAttributes) final case class GetWsmResourceResponse(resources: List[WsmResource]) -final case class GetJobResultRequest(workspaceId: WorkspaceId, jobId: WsmJobId) // Azure Disk models @@ -123,8 +97,6 @@ final case class ControlledResourceName(value: String) extends AnyVal final case class ControlledResourceDescription(value: String) extends AnyVal final case class PrivateResourceUser(userName: WorkbenchEmail, privateResourceIamRoles: ControlledResourceIamRole) -final case class WsmJobControl(id: WsmJobId) - final case class WsmGcpContext(projectId: GoogleProject) sealed abstract class WsmJobStatus @@ -216,18 +188,6 @@ object ManagedBy { object WsmDecoders { - implicit val metadataDecoder: Decoder[WsmVMMetadata] = Decoder.instance { c => - for { - id <- c.downField("resourceId").as[UUID] - } yield WsmVMMetadata(WsmControlledResourceId(id)) - } - - implicit val vmAttributesDecoder: Decoder[WsmVMAttributes] = Decoder.instance { a => - for { - region <- a.downField("region").as[RegionName] - } yield WsmVMAttributes(region) - } - implicit val createStorageContainerResultDecoder: Decoder[CreateStorageContainerResult] = Decoder.forProduct1("resourceId")(CreateStorageContainerResult.apply) @@ -280,61 +240,6 @@ object WsmDecoders { Decoder.forProduct2("metadata", "resourceAttributes")(WsmResource.apply) implicit val getRelayNamespaceDecoder: Decoder[GetWsmResourceResponse] = Decoder.forProduct1("resources")(GetWsmResourceResponse.apply) - -} - -object WsmEncoders { - implicit val controlledResourceIamRoleEncoder: Encoder[ControlledResourceIamRole] = - Encoder.encodeString.contramap(x => x.toString) - implicit val privateResourceUserEncoder: Encoder[PrivateResourceUser] = - Encoder.forProduct2("userName", "privateResourceIamRole")(x => (x.userName.value, x.privateResourceIamRoles)) - implicit val wsmCommonFieldsEncoder: Encoder[InternalDaoControlledResourceCommonFields] = - Encoder.forProduct7("name", - "description", - "cloningInstructions", - "accessScope", - "managedBy", - "privateResourceUser", - "resourceId" - )(x => - (x.name.value, - x.description.value, - x.cloningInstructions.toString, - x.accessScope.toString, - x.managedBy.toString, - x.privateResourceUser, - x.resourceId - ) - ) - - implicit val protectedSettingsEncoder: Encoder[ProtectedSettings] = Encoder.instance { x => - val fileUrisMap = Map( - "key" -> "fileUris".asJson, - "value" -> x.fileUris.asJson - ) - val cmdToExecuteMap = Map( - "key" -> "commandToExecute".asJson, - "value" -> x.commandToExecute.asJson - ) - List( - fileUrisMap, - cmdToExecuteMap - ).asJson - } - implicit val customScriptExtensionEncoder: Encoder[CustomScriptExtension] = - Encoder.forProduct6("name", "publisher", "type", "version", "minorVersionAutoUpgrade", "protectedSettings")(x => - (x.name, x.publisher, x.`type`, x.version, x.minorVersionAutoUpgrade, x.protectedSettings) - ) - implicit val vmCrendentialnEncoder: Encoder[VMCredential] = - Encoder.forProduct2("name", "password")(x => (x.username, x.password)) - - implicit val wsmJobControlEncoder: Encoder[WsmJobControl] = Encoder.forProduct1("id")(x => x.id) - - implicit val storageContainerRequestEncoder: Encoder[StorageContainerRequest] = - Encoder.forProduct1("storageContainerName")(x => x.storageContainerName) - - implicit val createStorageContainerRequestEncoder: Encoder[CreateStorageContainerRequest] = - Encoder.forProduct2("common", "azureStorageContainer")(x => (x.commonFields, x.storageContainerReq)) } final case class WsmException(traceId: TraceId, message: String) extends Exception(message) diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AKSInterpreter.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AKSInterpreter.scala index da8088416c..a639a24be2 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AKSInterpreter.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AKSInterpreter.scala @@ -897,7 +897,6 @@ class AKSInterpreter[F[_]](config: AKSInterpreterConfig, _ <- F.raiseWhen(landingZoneResources.postgresServer.isEmpty)( AppCreationException("Postgres server not found in landing zone", Some(ctx.traceId)) ) - wsmApi <- buildWsmControlledResourceApiClient // get a list of database types required for this app controlledDbsRequiredForApp = appInstall.databases.collect { case d @ ControlledDatabase(_, _, _) => d } diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala index 8bd9b288ca..323e410009 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandler.scala @@ -814,27 +814,6 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( ) } } yield () - - private def getCommonFields(name: ControlledResourceName, - resourceDesc: String, - userEmail: WorkbenchEmail, - resourceId: Option[WsmControlledResourceId] - ) = - InternalDaoControlledResourceCommonFields( - name, - ControlledResourceDescription(resourceDesc), - CloningInstructions.Nothing, - AccessScope.PrivateAccess, - ManagedBy.Application, - Some( - PrivateResourceUser( - userEmail, - ControlledResourceIamRole.Writer - ) - ), - resourceId - ) - private def getCommonFieldsForWsmGeneratedClient(name: ControlledResourceName, resourceDesc: String, userEmail: WorkbenchEmail @@ -857,7 +836,6 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( for { ctx <- ev.ask - auth <- samDAO.getLeoAuthToken createVmJobId = WsmJobId(s"create-vm-${params.runtime.id.toString.take(10)}") wsmControlledResourceClient <- buildWsmControlledResourceApiClient getWsmVmJobResult = F.delay( @@ -1197,7 +1175,6 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( .transaction _ <- clusterQuery.updateClusterStatus(e.runtimeId, RuntimeStatus.Error, now).transaction - auth <- samDAO.getLeoAuthToken diskIdOpt <- clusterQuery.getDiskId(e.runtimeId).transaction _ <- (e.useExistingDisk, diskIdOpt) match { @@ -1385,7 +1362,6 @@ class AzurePubsubHandlerInterp[F[_]: Parallel]( override def deleteDisk(msg: DeleteDiskV2Message)(implicit ev: Ask[F, AppContext]): F[Unit] = for { ctx <- ev.ask - auth <- samDAO.getLeoAuthToken _ <- msg.wsmResourceId match { case Some(wsmResourceId) => diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/http/service/AdminServiceInterpSpec.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/http/service/AdminServiceInterpSpec.scala index 95c805d66e..0dff293868 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/http/service/AdminServiceInterpSpec.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/http/service/AdminServiceInterpSpec.scala @@ -76,8 +76,6 @@ final class AdminServiceInterpSpec extends AnyFlatSpec with LeonardoTestSuite wi val savedNodepool = makeNodepool(1, cluster1.id).save() val app1 = makeApp(1, savedNodepool.id, status = AppStatus.Running, appType = AppType.Cromwell, chart = v1Chart).save() - val app2 = - makeApp(2, savedNodepool.id, status = AppStatus.Running, appType = AppType.Cromwell, chart = v2Chart).save() val publisherQueue = QueueFactory.makePublisherQueue() val interp = new AdminServiceInterp[IO]( diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala index 28a373d3f9..d6c2c2b03f 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzurePubsubHandlerSpec.scala @@ -4,7 +4,13 @@ package util import akka.actor.ActorSystem import akka.testkit.TestKit import bio.terra.workspace.client.ApiException -import bio.terra.workspace.model.{DeleteControlledAzureResourceRequest, JobReport} +import bio.terra.workspace.model.{ + AzureVmAttributes, + AzureVmResource, + CreatedControlledAzureVmResult, + DeleteControlledAzureResourceRequest, + JobReport +} import cats.effect.IO import cats.effect.std.Queue import cats.implicits._ @@ -83,7 +89,6 @@ class AzurePubsubHandlerSpec when(ipReturn.ipAddress()).thenReturn(stubIp) val queue = QueueFactory.asyncTaskQueue() - val resourceId = WsmControlledResourceId(UUID.randomUUID()) val azurePubsubHandler = makeAzurePubsubHandler(asyncTaskQueue = queue) @@ -133,17 +138,25 @@ class AzurePubsubHandlerSpec val ipReturn: PublicIpAddress = mock[PublicIpAddress] when(vmReturn.powerState()).thenReturn(PowerState.RUNNING) + val (mockWsm, controlledResourceApi, _, _) = + AzureTestUtils.setUpMockWsmApiClientProvider() - val stubIp = "0.0.0.0" - when(vmReturn.getPrimaryPublicIPAddress()).thenReturn(ipReturn) - when(ipReturn.ipAddress()).thenReturn(stubIp) + val now = ZonedDateTime.now().toString + + // create vm result + when { + controlledResourceApi.getCreateAzureVmResult(any, any) + } thenAnswer { _ => + new CreatedControlledAzureVmResult() + .jobReport( + new JobReport().status(JobReport.StatusEnum.SUCCEEDED).completed(now) + ) + .azureVm(new AzureVmResource().attributes(new AzureVmAttributes().region("southcentralus"))) + } val queue = QueueFactory.asyncTaskQueue() - val resourceId = WsmControlledResourceId(UUID.randomUUID()) - val startTime: Instant = Instant.now - val mockLatencyMillis = 5000 val azurePubsubHandler = - makeAzurePubsubHandler(asyncTaskQueue = queue) + makeAzurePubsubHandler(asyncTaskQueue = queue, wsmClient = mockWsm) val res = for { @@ -166,7 +179,6 @@ class AzurePubsubHandlerSpec } yield { getRuntime.asyncRuntimeFields.flatMap(_.hostIp).isDefined shouldBe true getRuntime.status shouldBe RuntimeStatus.Running - getRuntime.auditInfo.dateAccessed.isAfter(startTime.plusMillis(mockLatencyMillis)) shouldBe true getRuntimeConfig shouldBe azureRuntimeConfig.copy(region = Some(RegionName("southcentralus"))) } @@ -198,7 +210,6 @@ class AzurePubsubHandlerSpec when(ipReturn.ipAddress()).thenReturn(stubIp) val queue = QueueFactory.asyncTaskQueue() - val resourceId = WsmControlledResourceId(UUID.randomUUID()) val azurePubsubHandler = makeAzurePubsubHandler(asyncTaskQueue = queue) @@ -258,7 +269,6 @@ class AzurePubsubHandlerSpec it should "create an azure vm properly with an action managed identity" in isolatedDbTest { val queue = QueueFactory.asyncTaskQueue() - val resourceId = WsmControlledResourceId(UUID.randomUUID()) val mockSamDAO = new MockSamDAO { override def getAzureActionManagedIdentity(authHeader: Authorization, resource: PrivateAzureStorageAccountSamResourceId, @@ -320,7 +330,6 @@ class AzurePubsubHandlerSpec when(ipReturn.ipAddress()).thenReturn(stubIp) val queue = QueueFactory.asyncTaskQueue() - val resourceId = WsmControlledResourceId(UUID.randomUUID()) val (mockWsm, _, _, _) = AzureTestUtils.setUpMockWsmApiClientProvider(vmJobStatus = JobReport.StatusEnum.FAILED) val azurePubsubHandler = @@ -413,7 +422,6 @@ class AzurePubsubHandlerSpec when(ipReturn.ipAddress()).thenReturn(stubIp) val queue = QueueFactory.asyncTaskQueue() - val resourceId = WsmControlledResourceId(UUID.randomUUID()) val fakeWelderDao = new MockWelderDAO() { override def isProxyAvailable(cloudContext: CloudContext, clusterName: RuntimeName): IO[Boolean] = IO.pure(false) } @@ -757,7 +765,6 @@ class AzurePubsubHandlerSpec it should "fail to create runtime with persistent disk if no resourceId" in isolatedDbTest { val queue = QueueFactory.asyncTaskQueue() - val resourceId = WsmControlledResourceId(UUID.randomUUID()) val azureInterp = makeAzurePubsubHandler(asyncTaskQueue = queue) val res = @@ -1322,7 +1329,6 @@ class AzurePubsubHandlerSpec makeAzurePubsubHandler(asyncTaskQueue = queue, azureVmService = spyAzureVmService) val res = for { - ctx <- appContext.ask[AppContext] disk <- makePersistentDisk().copy(status = DiskStatus.Ready).save() azureRuntimeConfig = RuntimeConfig.AzureConfig(MachineTypeName(VirtualMachineSizeTypes.STANDARD_A1.toString), Some(disk.id), diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala index b9ceb03b37..93d0008a86 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/util/AzureTestUtils.scala @@ -16,14 +16,13 @@ import bio.terra.workspace.model.{ DeleteControlledAzureResourceResult, ErrorReport, IamRole, - JobReport, - ResourceMetadata + JobReport } import cats.mtl.Ask import com.azure.resourcemanager.compute.models.{PowerState, VirtualMachine} import org.broadinstitute.dsde.workbench.azure.{AzureCloudContext, ManagedResourceGroupName, SubscriptionId, TenantId} import org.broadinstitute.dsde.workbench.azure.mock.FakeAzureVmService -import org.broadinstitute.dsde.workbench.leonardo.CommonTestData.{azureRegion, wsmWorkspaceDesc} +import org.broadinstitute.dsde.workbench.leonardo.CommonTestData.wsmWorkspaceDesc import org.broadinstitute.dsde.workbench.leonardo.{AppContext, WorkspaceId} import org.broadinstitute.dsde.workbench.leonardo.dao.{ MockWsmClientProvider, @@ -71,8 +70,7 @@ object AzureTestUtils extends MockitoSugar { // Create disk when { api.createAzureDisk(any, any) - } thenAnswer { invocation => - val requestBody = invocation.getArgument[CreateControlledAzureDiskRequestBody](0) + } thenAnswer { _ => new CreateControlledAzureResourceResult() .jobReport( new JobReport().status(diskJobStatus) @@ -83,9 +81,7 @@ object AzureTestUtils extends MockitoSugar { // Get disk result when { api.getCreateAzureDiskResult(any, any) - } thenAnswer { invocation => - val jobId = invocation.getArgument[String](1) - val requestBody = disksByJob(jobId) + } thenAnswer { _ => new CreateControlledAzureResourceResult() .jobReport( new JobReport().status(diskJobStatus) From ae75d769ff0ed907eef4901956de76bb46b7e592 Mon Sep 17 00:00:00 2001 From: jdcanas Date: Tue, 20 Aug 2024 13:42:42 -0700 Subject: [PATCH 5/6] cleanup --- .../org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala index 798adf2f6c..a87572050a 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala @@ -24,7 +24,6 @@ trait WsmDao[F[_]] { ev: Ask[F, AppContext] ): F[LandingZoneResources] - // TODO: pt2 def getWorkspaceStorageContainer(workspaceId: WorkspaceId, authorization: Authorization)(implicit ev: Ask[F, AppContext] ): F[Option[StorageContainerResponse]] From 630f0264758143d2da7a4c6f09746984863a31ac Mon Sep 17 00:00:00 2001 From: jdcanas Date: Thu, 22 Aug 2024 11:39:09 -0700 Subject: [PATCH 6/6] feedback --- .../dsde/workbench/leonardo/dao/HttpWsmDao.scala | 8 ++++++++ .../dsde/workbench/leonardo/dao/WsmDao.scala | 10 ++++------ .../leonardo/http/service/AdminServiceInterpSpec.scala | 1 - 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala index 5e1cbf53d7..bc1e1f55fb 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpWsmDao.scala @@ -29,6 +29,10 @@ import org.typelevel.log4cats.StructuredLogger import java.util.UUID +/** + * This is the legacy WsmDAO. It remains because there is some specific logic around models retrieved from WSM + * It SHOULD NOT be added to. Favor usage of WsmClientProvider, the auto-generated client. + */ class HttpWsmDao[F[_]](httpClient: Client[F], config: HttpWsmDaoConfig)(implicit logger: StructuredLogger[F], F: Async[F], @@ -38,6 +42,8 @@ class HttpWsmDao[F[_]](httpClient: Client[F], config: HttpWsmDaoConfig)(implicit val defaultMediaType = `Content-Type`(MediaType.application.json) + // This remains in the legacy dao because of the custom logic around landing zones + // We should migrate to the generated client when possible override def getLandingZoneResources(billingProfileId: BillingProfileId, userToken: Authorization)(implicit ev: Ask[F, AppContext] ): F[LandingZoneResources] = @@ -250,6 +256,8 @@ class HttpWsmDao[F[_]](httpClient: Client[F], config: HttpWsmDaoConfig)(implicit )(onError) } yield resOpt.fold(List.empty[LandingZoneResourcesByPurpose])(res => res.resources) + // This remains in the legacy dao because of the custom logic around storage containers + // We should migrate to the generated client when possible override def getWorkspaceStorageContainer(workspaceId: WorkspaceId, authorization: Authorization)(implicit ev: Ask[F, AppContext] ): F[Option[StorageContainerResponse]] = for { diff --git a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala index a87572050a..0ae7accf16 100644 --- a/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala +++ b/http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/WsmDao.scala @@ -18,6 +18,10 @@ import org.http4s.headers.Authorization import java.util.UUID +/** + * This is the legacy WsmDAO. It remains because there is some specific logic around models retrieved from WSM + * It SHOULD NOT be added to. Favor usage of WsmClientProvider, the auto-generated client. + */ trait WsmDao[F[_]] { def getLandingZoneResources(billingProfileId: BillingProfileId, userToken: Authorization)(implicit @@ -28,9 +32,6 @@ trait WsmDao[F[_]] { ev: Ask[F, AppContext] ): F[Option[StorageContainerResponse]] } - -final case class StorageContainerRequest(storageContainerName: ContainerName) -final case class CreateStorageContainerResult(resourceId: WsmControlledResourceId) final case class WorkspaceDescription(id: WorkspaceId, displayName: String, spendProfile: String, @@ -187,9 +188,6 @@ object ManagedBy { object WsmDecoders { - implicit val createStorageContainerResultDecoder: Decoder[CreateStorageContainerResult] = - Decoder.forProduct1("resourceId")(CreateStorageContainerResult.apply) - implicit val azureContextDecoder: Decoder[AzureCloudContext] = Decoder.instance { c => for { tenantId <- c.downField("tenantId").as[String] diff --git a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/http/service/AdminServiceInterpSpec.scala b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/http/service/AdminServiceInterpSpec.scala index 0dff293868..219a03dde5 100644 --- a/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/http/service/AdminServiceInterpSpec.scala +++ b/http/src/test/scala/org/broadinstitute/dsde/workbench/leonardo/http/service/AdminServiceInterpSpec.scala @@ -70,7 +70,6 @@ final class AdminServiceInterpSpec extends AnyFlatSpec with LeonardoTestSuite wi it should "properly queue a message when doing an update" in isolatedDbTest { val v1Chart = Config.gkeCromwellAppConfig.chart.copy(version = ChartVersion("0.1.0")) - val v2Chart = Config.gkeCromwellAppConfig.chart val cluster1 = makeKubeCluster(1).save() val savedNodepool = makeNodepool(1, cluster1.id).save()