From 3d3ebf714adda9a5a3dd10e25c3ecb00679fbbd8 Mon Sep 17 00:00:00 2001 From: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com> Date: Wed, 3 Jul 2024 11:31:42 +0100 Subject: [PATCH 1/3] Azure Batch validates the region before checking available VMs Signed-off-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com> --- .../cloud/azure/batch/AzBatchService.groovy | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy b/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy index cd3e4b37f5..91d992802f 100644 --- a/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy +++ b/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy @@ -143,15 +143,23 @@ class AzBatchService implements Closeable { @Memoized private List listAllVms(String location) { + + final locationNames = listLocationNames() + if( !locationNames.contains(location) ) + throw new IllegalArgumentException("Invalid Azure location code: $location") + if( !location ) throw new IllegalArgumentException("Missing Azure location parameter") final json = AzBatchService.class.getResourceAsStream("/nextflow/cloud/azure/vm-list-size-${location}.json") - if( !json ) { - log.warn "Unable to find Azure VM names for location: $location" - return Collections.emptyList() - } + if( !json ) + throw new IllegalArgumentException("[AZURE BATCH] Unable to find Azure VM names for location: $location") - return (List) new JsonSlurper().parse(json) + final vmList = (List) new JsonSlurper().parse(json) + + if ( vmList.isEmpty() ) + throw new IllegalArgumentException("[AZURE BATCH] No VM sizes found for location: $location") + + return vmList } @Memoized @@ -189,19 +197,23 @@ class AzBatchService implements Closeable { * @return The `AzVmType` instance that best accommodate the resource requirement */ AzVmType findBestVm(String location, int cpus, MemoryUnit mem, String allFamilies) { + log.debug "[AZURE BATCH] Finding best VM given location=$location; cpus=$cpus; mem=$mem; family=$allFamilies" def all = listAllVms(location) + log.debug "[AZURE BATCH] Found ${all.size()} VM types in location $location" def scores = new TreeMap() def list = allFamilies ? allFamilies.tokenize(',') : [''] + log.debug "[AZURE BATCH] Listing VM families" for( String family : list ) { for( Map entry : all ) { if( !matchType(family, entry.name as String) ) continue def score = computeScore(cpus, mem, entry) + log.debug "[AZURE BATCH] VM family=${entry.name} score=${score}" if( score != null ) scores.put(score, entry.name as String) } } - + log.debug "[AZURE BATCH] Found ${scores.size()} VM types matching the criteria" return scores ? getVmType(location, scores.firstEntry().value) : null } From b458e6523061884491e271bf9c157ee368178d4f Mon Sep 17 00:00:00 2001 From: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com> Date: Mon, 8 Jul 2024 11:29:38 +0100 Subject: [PATCH 2/3] Reduce error to warning if region does not exist This allows users to manually configure a region not available in Nextflow's list of Azure regions Signed-off-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com> --- .../main/nextflow/cloud/azure/batch/AzBatchService.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy b/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy index 91d992802f..6158f6b113 100644 --- a/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy +++ b/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy @@ -146,18 +146,18 @@ class AzBatchService implements Closeable { final locationNames = listLocationNames() if( !locationNames.contains(location) ) - throw new IllegalArgumentException("Invalid Azure location code: $location") + log.warn("[AZURE BATCH] No location called ${location} found! Please confirm it exists and the name is correct!") if( !location ) throw new IllegalArgumentException("Missing Azure location parameter") final json = AzBatchService.class.getResourceAsStream("/nextflow/cloud/azure/vm-list-size-${location}.json") if( !json ) - throw new IllegalArgumentException("[AZURE BATCH] Unable to find Azure VM names for location: $location") + log.warn("[AZURE BATCH] Unable to find Azure VM names for location: $location") final vmList = (List) new JsonSlurper().parse(json) if ( vmList.isEmpty() ) - throw new IllegalArgumentException("[AZURE BATCH] No VM sizes found for location: $location") + log.warn("[AZURE BATCH] No VM sizes found for location: $location") return vmList } From bd60be373b519accb3d07df7b1c3f1b7033daef2 Mon Sep 17 00:00:00 2001 From: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com> Date: Mon, 8 Jul 2024 17:33:51 +0100 Subject: [PATCH 3/3] Are there VMs on Mars? Signed-off-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com> --- .../cloud/azure/batch/AzBatchService.groovy | 6 ++- .../azure/batch/AzBatchServiceTest.groovy | 44 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy b/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy index 6158f6b113..a22241275c 100644 --- a/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy +++ b/plugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchService.groovy @@ -151,14 +151,17 @@ class AzBatchService implements Closeable { if( !location ) throw new IllegalArgumentException("Missing Azure location parameter") final json = AzBatchService.class.getResourceAsStream("/nextflow/cloud/azure/vm-list-size-${location}.json") - if( !json ) + if( !json ) { log.warn("[AZURE BATCH] Unable to find Azure VM names for location: $location") + return Collections.emptyList() + } final vmList = (List) new JsonSlurper().parse(json) if ( vmList.isEmpty() ) log.warn("[AZURE BATCH] No VM sizes found for location: $location") + log.debug("[AZURE BATCH] vmList: ${vmList}") return vmList } @@ -208,7 +211,6 @@ class AzBatchService implements Closeable { if( !matchType(family, entry.name as String) ) continue def score = computeScore(cpus, mem, entry) - log.debug "[AZURE BATCH] VM family=${entry.name} score=${score}" if( score != null ) scores.put(score, entry.name as String) } diff --git a/plugins/nf-azure/src/test/nextflow/cloud/azure/batch/AzBatchServiceTest.groovy b/plugins/nf-azure/src/test/nextflow/cloud/azure/batch/AzBatchServiceTest.groovy index 660291859a..827730c86a 100644 --- a/plugins/nf-azure/src/test/nextflow/cloud/azure/batch/AzBatchServiceTest.groovy +++ b/plugins/nf-azure/src/test/nextflow/cloud/azure/batch/AzBatchServiceTest.groovy @@ -82,6 +82,50 @@ class AzBatchServiceTest extends Specification { 'Standard_D5_v2' in names } + def 'should list all VMs in region' () { + given: + def exec = Mock(AzBatchExecutor) { + getConfig() >> new AzConfig([:]) + } + def svc = new AzBatchService(exec) + + when: + def vms = svc.listAllVms('northeurope') + + then: + [ + maxDataDiskCount: 32, + memoryInMB: 28672, + name: "Standard_D4_v2", + numberOfCores: 8, + osDiskSizeInMB: 1047552, + resourceDiskSizeInMB: 409600 + ] in vms + [ + maxDataDiskCount: 8, + memoryInMB: 7168, + name: "Basic_A3", + numberOfCores: 4, + osDiskSizeInMB: 1047552, + resourceDiskSizeInMB: 122880 + ] in vms + } + + def 'should fail to list VMs in region' () { + given: + def exec = Mock(AzBatchExecutor) { + getConfig() >> new AzConfig([:]) + } + def svc = new AzBatchService(exec) + + when: + def vms = svc.listAllVms('mars') + + then: + vms instanceof List + vms.isEmpty() + } + def 'should get size for vm' () { given: def exec = Mock(AzBatchExecutor) {