diff --git a/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/AmalgamatedLandingZoneService.java b/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/AmalgamatedLandingZoneService.java index 9b5f74ae14..59f04b08da 100644 --- a/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/AmalgamatedLandingZoneService.java +++ b/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/AmalgamatedLandingZoneService.java @@ -101,7 +101,9 @@ public ApiAzureLandingZoneList listLandingZonesByBillingProfile( List landingZones = landingZoneService.getLandingZonesByBillingProfile(bearerToken, billingProfileId); landingZones.forEach( - landingZone -> result.addLandingzonesItem(typeAdapter.toApiAzureLandingZone(landingZone))); + landingZone -> + result.addLandingzonesItem( + typeAdapter.toApiAzureLandingZoneFromApiClient(landingZone))); return result; } diff --git a/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneApiDispatch.java b/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneApiDispatch.java index 4a4e8b938d..9362727330 100644 --- a/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneApiDispatch.java +++ b/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneApiDispatch.java @@ -3,8 +3,8 @@ import bio.terra.common.iam.BearerToken; import bio.terra.landingzone.library.landingzones.deployment.LandingZonePurpose; import bio.terra.landingzone.library.landingzones.deployment.ResourcePurpose; -import bio.terra.landingzone.service.landingzone.azure.LandingZoneService; import bio.terra.workspace.app.configuration.external.FeatureConfiguration; +import bio.terra.workspace.common.utils.Rethrow; import bio.terra.workspace.generated.model.ApiAzureLandingZone; import bio.terra.workspace.generated.model.ApiAzureLandingZoneDefinitionList; import bio.terra.workspace.generated.model.ApiAzureLandingZoneDeployedResource; @@ -47,10 +47,10 @@ public class LandingZoneApiDispatch { private final WorkspaceLandingZoneService amalgamated; public LandingZoneApiDispatch( - LandingZoneService landingZoneService, FeatureConfiguration features, SamService samService) { + FeatureConfiguration features, SamService samService, LandingZoneServiceFactory lzsFactory) { this.features = features; this.samService = samService; - this.amalgamated = new AmalgamatedLandingZoneService(landingZoneService); + this.amalgamated = lzsFactory.getLandingZoneService(); } public ApiCreateLandingZoneResult createAzureLandingZone( @@ -66,15 +66,18 @@ public ApiCreateLandingZoneResult createAzureLandingZone( // Prevent deploying more than 1 landing zone per billing profile verifyLandingZoneDoesNotExistForBillingProfile(bearerToken, body); - return amalgamated.startLandingZoneCreationJob( - bearerToken, - body.getJobControl().getId(), - body.getLandingZoneId(), - body.getDefinition(), - body.getVersion(), - body.getParameters(), - body.getBillingProfileId(), - asyncResultEndpoint); + return Rethrow.onInterrupted( + () -> + amalgamated.startLandingZoneCreationJob( + bearerToken, + body.getJobControl().getId(), + body.getLandingZoneId(), + body.getDefinition(), + body.getVersion(), + body.getParameters(), + body.getBillingProfileId(), + asyncResultEndpoint), + "startLandingZoneCreationJob"); } private void verifyLandingZoneDoesNotExistForBillingProfile( @@ -82,16 +85,19 @@ private void verifyLandingZoneDoesNotExistForBillingProfile( // TODO: Catching the exception is a temp solution. // A better approach would be to return an empty list instead of throwing an exception try { - amalgamated - .listLandingZonesByBillingProfile(bearerToken, body.getBillingProfileId()) - .getLandingzones() - .stream() - .findFirst() - .ifPresent( - t -> { - throw new LandingZoneInvalidInputException( - "A Landing Zone already exists in the requested billing profile"); - }); + Rethrow.onInterrupted( + () -> + amalgamated + .listLandingZonesByBillingProfile(bearerToken, body.getBillingProfileId()) + .getLandingzones() + .stream() + .findFirst() + .ifPresent( + t -> { + throw new LandingZoneInvalidInputException( + "A Landing Zone already exists in the requested billing profile"); + }), + "listLandingZonesByBillingProfile"); } catch (bio.terra.landingzone.db.exception.LandingZoneNotFoundException ex) { logger.info("The billing profile does not have a landing zone. ", ex); } @@ -100,13 +106,15 @@ private void verifyLandingZoneDoesNotExistForBillingProfile( public ApiAzureLandingZoneResult getCreateAzureLandingZoneResult( BearerToken bearerToken, String jobId) { features.azureEnabledCheck(); - return amalgamated.getAsyncJobResult(bearerToken, jobId); + return Rethrow.onInterrupted( + () -> amalgamated.getAsyncJobResult(bearerToken, jobId), "getAsyncJobResult"); } public ApiAzureLandingZoneDefinitionList listAzureLandingZonesDefinitions( BearerToken bearerToken) { features.azureEnabledCheck(); - return amalgamated.listLandingZoneDefinitions(bearerToken); + return Rethrow.onInterrupted( + () -> amalgamated.listLandingZoneDefinitions(bearerToken), "listLandingZoneDefinitions"); } public ApiDeleteAzureLandingZoneResult deleteLandingZone( @@ -115,14 +123,19 @@ public ApiDeleteAzureLandingZoneResult deleteLandingZone( ApiDeleteAzureLandingZoneRequestBody body, String resultEndpoint) { features.azureEnabledCheck(); - return amalgamated.startLandingZoneDeletionJob( - bearerToken, body.getJobControl().getId(), landingZoneId, resultEndpoint); + return Rethrow.onInterrupted( + () -> + amalgamated.startLandingZoneDeletionJob( + bearerToken, body.getJobControl().getId(), landingZoneId, resultEndpoint), + "startLandingZoneDeletionJob"); } public ApiAzureLandingZoneResourcesList listAzureLandingZoneResources( BearerToken bearerToken, UUID landingZoneId) { features.azureEnabledCheck(); - return amalgamated.listResourcesWithPurposes(bearerToken, landingZoneId); + return Rethrow.onInterrupted( + () -> amalgamated.listResourcesWithPurposes(bearerToken, landingZoneId), + "listResourcesWithPurpose"); } public Optional getSharedStorageAccount( @@ -155,7 +168,9 @@ public Optional getSharedDatabaseAdminIdent public ApiAzureLandingZoneResourcesList listAzureLandingZoneResourcesByPurpose( BearerToken bearerToken, UUID landingZoneId, LandingZonePurpose resourcePurpose) { features.azureEnabledCheck(); - return amalgamated.listResourcesMatchingPurpose(bearerToken, landingZoneId, resourcePurpose); + return Rethrow.onInterrupted( + () -> amalgamated.listResourcesMatchingPurpose(bearerToken, landingZoneId, resourcePurpose), + "listResourcesMatchingPurpose"); } public UUID getLandingZoneId(BearerToken token, Workspace workspace) { @@ -169,10 +184,11 @@ public UUID getLandingZoneId(BearerToken token, Workspace workspace) { } // getLandingZonesByBillingProfile returns a list. But it always contains only one item - return amalgamated - .listLandingZonesByBillingProfile(token, profileId.get()) - .getLandingzones() - .stream() + var response = + Rethrow.onInterrupted( + () -> amalgamated.listLandingZonesByBillingProfile(token, profileId.get()), + "listLandingZonesByBillingProfile"); + return response.getLandingzones().stream() .findFirst() .map(ApiAzureLandingZone::getLandingZoneId) .orElseThrow( @@ -188,21 +204,27 @@ public UUID getLandingZoneId(BearerToken token, Workspace workspace) { public ApiDeleteAzureLandingZoneJobResult getDeleteAzureLandingZoneResult( BearerToken token, UUID landingZoneId, String jobId) { features.azureEnabledCheck(); - return amalgamated.getDeleteLandingZoneResult(token, landingZoneId, jobId); + return Rethrow.onInterrupted( + () -> amalgamated.getDeleteLandingZoneResult(token, landingZoneId, jobId), + "getDeleteLandingZoneResult"); } public ApiAzureLandingZone getAzureLandingZone(BearerToken bearerToken, UUID landingZoneId) { features.azureEnabledCheck(); - return amalgamated.getAzureLandingZone(bearerToken, landingZoneId); + return Rethrow.onInterrupted( + () -> amalgamated.getAzureLandingZone(bearerToken, landingZoneId), "getAzureLandingZone"); } public ApiAzureLandingZoneList listAzureLandingZones( BearerToken bearerToken, UUID billingProfileId) { features.azureEnabledCheck(); if (billingProfileId != null) { - return amalgamated.listLandingZonesByBillingProfile(bearerToken, billingProfileId); + return Rethrow.onInterrupted( + () -> amalgamated.listLandingZonesByBillingProfile(bearerToken, billingProfileId), + "listLandingZonesByBillingProfile"); } - return amalgamated.listLandingZones(bearerToken); + return Rethrow.onInterrupted( + () -> amalgamated.listLandingZones(bearerToken), "listLandingZones"); } public ApiResourceQuota getResourceQuota( @@ -255,6 +277,7 @@ public String getLandingZoneRegionForWorkspaceUsingWsmToken(Workspace workspace) public String getLandingZoneRegionUsingWsmToken(UUID landingZoneId) { features.azureEnabledCheck(); var token = new BearerToken(samService.getWsmServiceAccountToken()); - return amalgamated.getLandingZoneRegion(token, landingZoneId); + return Rethrow.onInterrupted( + () -> amalgamated.getLandingZoneRegion(token, landingZoneId), "getLandingZoneRegion"); } } diff --git a/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneServiceApiException.java b/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneServiceApiException.java index 76bec1c20a..b2ed08520d 100644 --- a/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneServiceApiException.java +++ b/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneServiceApiException.java @@ -40,7 +40,7 @@ public LandingZoneServiceApiException( super(message, cause, causes, statusCode); } - /** Get the HTTP status code of the underlying response from Policy Service. */ + /** Get the HTTP status code of the underlying response from LZS. */ public int getApiExceptionStatus() { return apiException.getCode(); } diff --git a/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneServiceFactory.java b/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneServiceFactory.java new file mode 100644 index 0000000000..4f04f337db --- /dev/null +++ b/service/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneServiceFactory.java @@ -0,0 +1,33 @@ +package bio.terra.workspace.amalgam.landingzone.azure; + +import bio.terra.landingzone.service.landingzone.azure.LandingZoneService; +import bio.terra.workspace.app.configuration.external.FeatureConfiguration; +import io.opentelemetry.api.OpenTelemetry; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +@Component +public class LandingZoneServiceFactory { + + private final LandingZoneService amalgamatedLandingZoneService; + private FeatureConfiguration configuration; + private final OpenTelemetry openTelemetry; + + @Autowired + public LandingZoneServiceFactory( + FeatureConfiguration configuration, + OpenTelemetry openTelemetry, + LandingZoneService landingZoneService) { + this.configuration = configuration; + this.openTelemetry = openTelemetry; + this.amalgamatedLandingZoneService = landingZoneService; + } + + public WorkspaceLandingZoneService getLandingZoneService() { + if (this.configuration.isLzsEnabled()) { + return new HttpLandingZoneService(openTelemetry); + } else { + return new AmalgamatedLandingZoneService(amalgamatedLandingZoneService); + } + } +} diff --git a/service/src/main/java/bio/terra/workspace/app/configuration/external/FeatureConfiguration.java b/service/src/main/java/bio/terra/workspace/app/configuration/external/FeatureConfiguration.java index 5ef363d5bb..5f2ff956fc 100644 --- a/service/src/main/java/bio/terra/workspace/app/configuration/external/FeatureConfiguration.java +++ b/service/src/main/java/bio/terra/workspace/app/configuration/external/FeatureConfiguration.java @@ -20,6 +20,7 @@ public class FeatureConfiguration { private boolean tpsEnabled; private boolean bpmGcpEnabled; private boolean temporaryGrantEnabled; + private boolean lzsEnabled; private WsmResourceStateRule stateRule; public boolean isAzureEnabled() { @@ -62,6 +63,14 @@ public void setBpmGcpEnabled(boolean bpmGcpEnabled) { this.bpmGcpEnabled = bpmGcpEnabled; } + public boolean isLzsEnabled() { + return lzsEnabled; + } + + public void setLzsEnabled(boolean lzsEnabled) { + this.lzsEnabled = lzsEnabled; + } + public boolean isTemporaryGrantEnabled() { return temporaryGrantEnabled; } @@ -110,5 +119,7 @@ public void logFeatures() { logger.info("Feature: bpm-gcp-enabled: {}", isBpmGcpEnabled()); logger.info("Feature: temporary-grant-enabled: {}", isTemporaryGrantEnabled()); logger.info("Feature: state-rule: {}", getStateRule()); + logger.info("Feature: lzs-enabled: {}", isLzsEnabled()); + ; } } diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/LzsRetry.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/LzsRetry.java index ec9aaf0108..52b317eae2 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/LzsRetry.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/LzsRetry.java @@ -25,7 +25,7 @@ public class LzsRetry { private static final Duration INITIAL_WAIT = Duration.ofSeconds(10); private static final Duration OPERATION_TIMEOUT = Duration.ofSeconds(300); - // Tps calls which timeout will throw ApiExceptions wrapping SocketTimeoutExceptions and will have + // LZS calls which timeout will throw ApiExceptions wrapping SocketTimeoutExceptions and will have // an errorCode 0. This isn't a real HTTP status code, but we can check for it anyway. private static final int TIMEOUT_STATUS_CODE = 0; @@ -55,7 +55,7 @@ public interface LzsFunction { } /** - * Requests made through the Tps client library sometimes fail with timeouts, generally due to + * Requests made through the LZS client library sometimes fail with timeouts, generally due to * transient network or connection issues. When this happens, the client library will throw an API * exceptions with status code 0 wrapping a SocketTimeoutException. These errors should always be * retried. @@ -127,20 +127,20 @@ private void performVoid(LzsRetry.LzsVoidFunction function) } /** - * Given an exception from Tps, either timeout and rethrow the error from Tps or sleep for + * Given an exception from LZS, either timeout and rethrow the error from LZS or sleep for * retryDuration. If the thread times out while sleeping, throw the initial exception. * *

With the current values of INITIAL_WAIT and MAXIMUM_WAIT, this will sleep with the pattern * 10, 20, 30, 30, 30... seconds. * - * @param previousException The error Tps threw + * @param previousException The error LZS threw * @throws E, InterruptedException */ private void sleepOrTimeoutBeforeRetrying(E previousException) throws E, InterruptedException { if (operationTimeout.minus(retryDuration).isBefore(now())) { logger.error("LzsRetry: operation timed out after " + operationTimeout); - // If we timed out, throw the error from Tps that caused us to need to retry. + // If we timed out, throw the error from LZS that caused us to need to retry. throw previousException; } logger.info("LzsRetry: sleeping " + retryDuration.getSeconds() + " seconds"); diff --git a/service/src/test/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneApiDispatchTest.java b/service/src/test/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneApiDispatchTest.java index 9ae454bd35..b430411abb 100644 --- a/service/src/test/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneApiDispatchTest.java +++ b/service/src/test/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneApiDispatchTest.java @@ -64,12 +64,14 @@ public class LandingZoneApiDispatchTest extends BaseAzureSpringBootUnitTest { @Mock private LandingZoneService landingZoneService; @Mock private FeatureConfiguration featureConfiguration; @Mock private WorkspaceService workspaceService; + @Mock private LandingZoneServiceFactory landingZoneServiceFactory; @BeforeEach void setupLandingZoneTests() { when(featureConfiguration.isAzureEnabled()).thenReturn(true); landingZoneApiDispatch = - new LandingZoneApiDispatch(landingZoneService, featureConfiguration, mockSamService()); + new LandingZoneApiDispatch( + featureConfiguration, mockSamService(), landingZoneServiceFactory); } @Test @@ -91,7 +93,8 @@ void createAzureLandingZone_ReturnJobRunning_Success() { eq(BEARER_TOKEN), eq(JOB_ID), any(LandingZoneRequest.class), any())) .thenReturn(createJobResult); landingZoneApiDispatch = - new LandingZoneApiDispatch(landingZoneService, featureConfiguration, mockSamService()); + new LandingZoneApiDispatch( + featureConfiguration, mockSamService(), landingZoneServiceFactory); ApiCreateLandingZoneResult response = landingZoneApiDispatch.createAzureLandingZone(BEARER_TOKEN, request, resultEndpoint); @@ -124,7 +127,8 @@ void createAzureLandingZone_LandingZoneExistsForBillingProfileThrows() .thenReturn(landingZoneList); landingZoneApiDispatch = - new LandingZoneApiDispatch(landingZoneService, featureConfiguration, mockSamService()); + new LandingZoneApiDispatch( + featureConfiguration, mockSamService(), landingZoneServiceFactory); assertThrows( LandingZoneInvalidInputException.class, @@ -150,7 +154,8 @@ void createAzureLandingZone_LandingZoneDoesNotExistsExceptionIsHandledAndStartsJ .thenReturn(createJobResult); landingZoneApiDispatch = - new LandingZoneApiDispatch(landingZoneService, featureConfiguration, mockSamService()); + new LandingZoneApiDispatch( + featureConfiguration, mockSamService(), landingZoneServiceFactory); ApiCreateLandingZoneResult result = landingZoneApiDispatch.createAzureLandingZone(BEARER_TOKEN, request, resultEndpoint); @@ -234,7 +239,8 @@ void listAzureLandingZoneResourcesByPurpose_Success_NoResults() { BEARER_TOKEN, LANDING_ZONE_ID, SubnetResourcePurpose.WORKSPACE_BATCH_SUBNET)) .thenReturn(Collections.emptyList()); landingZoneApiDispatch = - new LandingZoneApiDispatch(landingZoneService, featureConfiguration, mockSamService()); + new LandingZoneApiDispatch( + featureConfiguration, mockSamService(), landingZoneServiceFactory); ApiAzureLandingZoneResourcesList response = landingZoneApiDispatch.listAzureLandingZoneResourcesByPurpose( @@ -265,7 +271,8 @@ void listAzureLandingZonesByBillingProfile_Success() { .createdDate(CREATED_DATE) .build())); landingZoneApiDispatch = - new LandingZoneApiDispatch(landingZoneService, featureConfiguration, mockSamService()); + new LandingZoneApiDispatch( + featureConfiguration, mockSamService(), landingZoneServiceFactory); ApiAzureLandingZoneList response = landingZoneApiDispatch.listAzureLandingZones(BEARER_TOKEN, BILLING_PROFILE_ID); @@ -304,7 +311,8 @@ void listAzureLandingZones_Success() { .createdDate(CREATED_DATE) .build())); landingZoneApiDispatch = - new LandingZoneApiDispatch(landingZoneService, featureConfiguration, mockSamService()); + new LandingZoneApiDispatch( + featureConfiguration, mockSamService(), landingZoneServiceFactory); ApiAzureLandingZoneList response = landingZoneApiDispatch.listAzureLandingZones(BEARER_TOKEN, null); @@ -371,7 +379,8 @@ private void setupLandingZoneResources() { SubnetResourcePurpose.WORKSPACE_STORAGE_SUBNET, listSubnets1, ResourcePurpose.SHARED_RESOURCE, listResources1))); landingZoneApiDispatch = - new LandingZoneApiDispatch(landingZoneService, featureConfiguration, mockSamService()); + new LandingZoneApiDispatch( + featureConfiguration, mockSamService(), landingZoneServiceFactory); } @Test