Skip to content

Commit c50e562

Browse files
dglazerclaude
authored andcommitted
Add GCS bucket mounting logic for LogsPolicy PATH option
- Add helper methods to extract bucket names and convert GCS paths to mount paths - Ensure logs bucket is mounted as Volume in GoogleBatchScriptLauncher - Update LogsPolicy to use container mount paths instead of GCS paths - Add comprehensive tests for bucket mounting and path conversion - Addresses reviewer feedback about missing bucket mounting requirements This ensures Google Batch can write logs to the specified GCS bucket by properly mounting it before referencing it in the LogsPolicy PATH. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Emma Rogge <[email protected]>
1 parent 35bb4b8 commit c50e562

File tree

5 files changed

+63
-2
lines changed

5 files changed

+63
-2
lines changed

plugins/nf-google/src/main/nextflow/cloud/google/batch/GoogleBatchScriptLauncher.groovy

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,13 @@ class GoogleBatchScriptLauncher extends BashWrapperBuilder implements GoogleBatc
186186

187187
GoogleBatchScriptLauncher withConfig(GoogleOpts config) {
188188
this.config = config
189+
// Add logs bucket to mounted volumes if configured
190+
if( config?.batch?.logsBucket ) {
191+
final logsBucketName = config.batch.extractBucketName(config.batch.logsBucket)
192+
if( logsBucketName ) {
193+
buckets.add(logsBucketName)
194+
}
195+
}
189196
return this
190197
}
191198

plugins/nf-google/src/main/nextflow/cloud/google/batch/GoogleBatchTaskHandler.groovy

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,9 +459,10 @@ class GoogleBatchTaskHandler extends TaskHandler implements FusionAwareTask {
459459
protected LogsPolicy createLogsPolicy() {
460460
final logsBucket = executor.batchConfig.logsBucket
461461
if( logsBucket ) {
462+
final containerPath = executor.batchConfig.convertGcsPathToMountPath(logsBucket)
462463
return LogsPolicy.newBuilder()
463464
.setDestination(LogsPolicy.Destination.PATH)
464-
.setLogsPath(logsBucket)
465+
.setLogsPath(containerPath)
465466
.build()
466467
} else {
467468
return LogsPolicy.newBuilder()

plugins/nf-google/src/main/nextflow/cloud/google/batch/client/BatchConfig.groovy

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,4 +175,31 @@ class BatchConfig implements ConfigScope {
175175
return bucket
176176
}
177177

178+
/**
179+
* Extract the bucket name from a GCS path
180+
* @param gcsPath GCS path like "gs://bucket-name/path/to/logs"
181+
* @return bucket name like "bucket-name"
182+
*/
183+
static String extractBucketName(String gcsPath) {
184+
if( !gcsPath || !gcsPath.startsWith('gs://') )
185+
return null
186+
187+
final pathWithoutProtocol = gcsPath.substring(5) // Remove "gs://"
188+
final slashIndex = pathWithoutProtocol.indexOf('/')
189+
return slashIndex > 0 ? pathWithoutProtocol.substring(0, slashIndex) : pathWithoutProtocol
190+
}
191+
192+
/**
193+
* Convert a GCS path to container mount path
194+
* @param gcsPath GCS path like "gs://bucket-name/path/to/logs"
195+
* @return container path like "/mnt/disks/bucket-name/path/to/logs"
196+
*/
197+
static String convertGcsPathToMountPath(String gcsPath) {
198+
if( !gcsPath || !gcsPath.startsWith('gs://') )
199+
return gcsPath
200+
201+
final pathWithoutProtocol = gcsPath.substring(5) // Remove "gs://"
202+
return "/mnt/disks/${pathWithoutProtocol}"
203+
}
204+
178205
}

plugins/nf-google/src/test/nextflow/cloud/google/batch/GoogleBatchTaskHandlerTest.groovy

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -702,7 +702,14 @@ class GoogleBatchTaskHandlerTest extends Specification {
702702

703703
and:
704704
req.getLogsPolicy().getDestination().toString() == 'PATH'
705-
req.getLogsPolicy().getLogsPath() == LOGS_BUCKET
705+
req.getLogsPolicy().getLogsPath() == '/mnt/disks/my-logs-bucket/logs'
706+
and:
707+
def taskGroup = req.getTaskGroups(0)
708+
def volumes = taskGroup.getTaskSpec().getVolumesList()
709+
volumes.size() >= 2 // At least work dir volume and logs bucket volume
710+
def logsBucketVolume = volumes.find { it.getGcs().getRemotePath() == 'my-logs-bucket' }
711+
logsBucketVolume != null
712+
logsBucketVolume.getMountPath() == '/mnt/disks/my-logs-bucket'
706713
}
707714

708715
def makeTask(String name, TaskStatus.State state){

plugins/nf-google/src/test/nextflow/cloud/google/batch/client/BatchConfigTest.groovy

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,23 @@ class BatchConfigTest extends Specification {
101101
e.message.contains("Logs bucket path must start with 'gs://'")
102102
}
103103

104+
def 'should extract bucket name from GCS path' () {
105+
expect:
106+
BatchConfig.extractBucketName('gs://my-bucket') == 'my-bucket'
107+
BatchConfig.extractBucketName('gs://my-bucket/logs') == 'my-bucket'
108+
BatchConfig.extractBucketName('gs://my-bucket/path/to/logs') == 'my-bucket'
109+
BatchConfig.extractBucketName('gs://') == ''
110+
BatchConfig.extractBucketName('invalid-path') == null
111+
BatchConfig.extractBucketName(null) == null
112+
}
113+
114+
def 'should convert GCS path to mount path' () {
115+
expect:
116+
BatchConfig.convertGcsPathToMountPath('gs://my-bucket') == '/mnt/disks/my-bucket'
117+
BatchConfig.convertGcsPathToMountPath('gs://my-bucket/logs') == '/mnt/disks/my-bucket/logs'
118+
BatchConfig.convertGcsPathToMountPath('gs://my-bucket/path/to/logs') == '/mnt/disks/my-bucket/path/to/logs'
119+
BatchConfig.convertGcsPathToMountPath('invalid-path') == 'invalid-path'
120+
BatchConfig.convertGcsPathToMountPath(null) == null
121+
}
122+
104123
}

0 commit comments

Comments
 (0)