From aa278b2895e5cc1babd33304bfb84c1bd77080f2 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 29 Oct 2024 13:08:45 +0100 Subject: [PATCH 1/9] fixed conda file rendering Signed-off-by: munishchouhan --- .../io/seqera/wave/controller/ViewController.groovy | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy index d30db91d8..ec25e6fa1 100644 --- a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy +++ b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy @@ -233,7 +233,17 @@ class ViewController { } //add conda lock file when available if( buildLogService && result.condaFile ) { - binding.build_conda_lock_data = buildLogService.fetchCondaLockString(result.buildId) + def condaLock = buildLogService.fetchCondaLockString(result.buildId) + if ( condaLock && condaLock.contains('cat environment.lock')) { + condaLock = null //if there is no conda lock file, don't show the conda lock data + def builds = persistenceService.allBuilds(result.buildId.split('-')[1].split('_')[0]) + for (def build : builds) { + if (build.succeeded()){ + condaLock = buildLogService.fetchCondaLockString(build.buildId) + } + } + } + binding.build_conda_lock_data = condaLock binding.build_conda_lock_url = "$serverUrl/v1alpha1/builds/${result.buildId}/condalock" } // result the main object From 22a5af7f4b334a2f0b48a130c09c87808082a849 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 29 Oct 2024 13:21:54 +0100 Subject: [PATCH 2/9] fix lockfile download Signed-off-by: munishchouhan --- .../groovy/io/seqera/wave/controller/ViewController.groovy | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy index ec25e6fa1..c63ec1e72 100644 --- a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy +++ b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy @@ -234,17 +234,19 @@ class ViewController { //add conda lock file when available if( buildLogService && result.condaFile ) { def condaLock = buildLogService.fetchCondaLockString(result.buildId) + def buildId = result.buildId if ( condaLock && condaLock.contains('cat environment.lock')) { condaLock = null //if there is no conda lock file, don't show the conda lock data def builds = persistenceService.allBuilds(result.buildId.split('-')[1].split('_')[0]) for (def build : builds) { if (build.succeeded()){ + buildId = build.buildId condaLock = buildLogService.fetchCondaLockString(build.buildId) } } } binding.build_conda_lock_data = condaLock - binding.build_conda_lock_url = "$serverUrl/v1alpha1/builds/${result.buildId}/condalock" + binding.build_conda_lock_url = "$serverUrl/v1alpha1/builds/${buildId}/condalock" } // result the main object return binding From 7490150cfa8d77f6b28d9000d534e44bf7eb2546 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 29 Oct 2024 13:23:48 +0100 Subject: [PATCH 3/9] formatted [ci skip] Signed-off-by: munishchouhan --- .../groovy/io/seqera/wave/controller/ViewController.groovy | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy index c63ec1e72..a30caf637 100644 --- a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy +++ b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy @@ -233,8 +233,9 @@ class ViewController { } //add conda lock file when available if( buildLogService && result.condaFile ) { - def condaLock = buildLogService.fetchCondaLockString(result.buildId) + def condaLock = buildLogService.fetchCondaLockString(result.buildId) def buildId = result.buildId + if ( condaLock && condaLock.contains('cat environment.lock')) { condaLock = null //if there is no conda lock file, don't show the conda lock data def builds = persistenceService.allBuilds(result.buildId.split('-')[1].split('_')[0]) @@ -245,6 +246,7 @@ class ViewController { } } } + binding.build_conda_lock_data = condaLock binding.build_conda_lock_url = "$serverUrl/v1alpha1/builds/${buildId}/condalock" } From ac959ec4bbe9bd5bcdc75156b51f7faeccfb8aa9 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 19 Nov 2024 09:51:19 +0100 Subject: [PATCH 4/9] added comment Signed-off-by: munishchouhan --- .../groovy/io/seqera/wave/controller/ViewController.groovy | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy index a96766274..6fcd97e98 100644 --- a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy +++ b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy @@ -236,6 +236,13 @@ class ViewController { def condaLock = buildLogService.fetchCondaLockString(result.buildId) def buildId = result.buildId + /* + When a container image is cached, dockerfile does not get executed. + In that case condalock file will contain "cat environment.lock" because its not been executed. + So wave will check the previous builds of that container image + and render the condalock file from latest successful build + and replace with the current build's condalock file. + */ if ( condaLock && condaLock.contains('cat environment.lock')) { condaLock = null //if there is no conda lock file, don't show the conda lock data def builds = persistenceService.allBuilds(result.buildId.split('-')[1].split('_')[0]) From a6d4a92aed8bceff9860e828221b683da5575df0 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 19 Nov 2024 09:55:03 +0100 Subject: [PATCH 5/9] added another check Signed-off-by: munishchouhan --- .../groovy/io/seqera/wave/controller/ViewController.groovy | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy index 6fcd97e98..469f7526a 100644 --- a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy +++ b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy @@ -247,9 +247,11 @@ class ViewController { condaLock = null //if there is no conda lock file, don't show the conda lock data def builds = persistenceService.allBuilds(result.buildId.split('-')[1].split('_')[0]) for (def build : builds) { - if (build.succeeded()){ + if ( build.succeeded() ){ buildId = build.buildId condaLock = buildLogService.fetchCondaLockString(build.buildId) + if ( !condaLock.contains('cat environment.lock') ) + break } } } From c201b2504af017569f9c2556eae54ea87738837b Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 19 Nov 2024 09:57:31 +0100 Subject: [PATCH 6/9] minor change [ci skip] Signed-off-by: munishchouhan --- src/main/groovy/io/seqera/wave/controller/ViewController.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy index 469f7526a..3d3950461 100644 --- a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy +++ b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy @@ -243,7 +243,7 @@ class ViewController { and render the condalock file from latest successful build and replace with the current build's condalock file. */ - if ( condaLock && condaLock.contains('cat environment.lock')) { + if ( condaLock && condaLock.contains('cat environment.lock') ) { condaLock = null //if there is no conda lock file, don't show the conda lock data def builds = persistenceService.allBuilds(result.buildId.split('-')[1].split('_')[0]) for (def build : builds) { From 04f5acb4497b0e58f347fb59f6d63bf329486e06 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 10 Dec 2024 14:10:42 +0100 Subject: [PATCH 7/9] moved logic to storeCondaLock Signed-off-by: munishchouhan --- .../wave/controller/ViewController.groovy | 27 ++----------------- .../service/logs/BuildLogServiceImpl.groovy | 18 ++++++++++++- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy index 3d3950461..68328eb36 100644 --- a/src/main/groovy/io/seqera/wave/controller/ViewController.groovy +++ b/src/main/groovy/io/seqera/wave/controller/ViewController.groovy @@ -233,31 +233,8 @@ class ViewController { } //add conda lock file when available if( buildLogService && result.condaFile ) { - def condaLock = buildLogService.fetchCondaLockString(result.buildId) - def buildId = result.buildId - - /* - When a container image is cached, dockerfile does not get executed. - In that case condalock file will contain "cat environment.lock" because its not been executed. - So wave will check the previous builds of that container image - and render the condalock file from latest successful build - and replace with the current build's condalock file. - */ - if ( condaLock && condaLock.contains('cat environment.lock') ) { - condaLock = null //if there is no conda lock file, don't show the conda lock data - def builds = persistenceService.allBuilds(result.buildId.split('-')[1].split('_')[0]) - for (def build : builds) { - if ( build.succeeded() ){ - buildId = build.buildId - condaLock = buildLogService.fetchCondaLockString(build.buildId) - if ( !condaLock.contains('cat environment.lock') ) - break - } - } - } - - binding.build_conda_lock_data = condaLock - binding.build_conda_lock_url = "$serverUrl/v1alpha1/builds/${buildId}/condalock" + binding.build_conda_lock_data = buildLogService.fetchCondaLockString(result.buildId) + binding.build_conda_lock_url = "$serverUrl/v1alpha1/builds/${result.buildId}/condalock" } // result the main object return binding diff --git a/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy b/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy index 5fadd46c9..aa265cd43 100644 --- a/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy +++ b/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy @@ -152,8 +152,24 @@ class BuildLogServiceImpl implements BuildLogService { if( !logs ) return try { String condaLock = extractCondaLockFile(logs) - if (condaLock){ + if ( condaLock ){ log.debug "Storing conda lock for buildId: $buildId" + + /* When a container image is cached, dockerfile does not get executed. + In that case condalock file will contain "cat environment.lock" because its not been executed. + So wave will check the previous builds of that container image + and render the condalock file from latest successful build + and replace with the current build's condalock file. + */ + if( condaLock.contains('cat environment.lock') ){ + log.info "Container Image is already cached, uploading previously successful build's condalock file for buildId: $buildId" + def builds = persistenceService.allBuilds(buildId.split('-')[1].split('_')[0]) + for (def build : builds) { + if ( build.succeeded() && !condaLock.contains('cat environment.lock') ){ + condaLock = fetchCondaLockString(build.buildId) + } + } + } final uploadRequest = UploadRequest.fromBytes(condaLock.bytes, condaLockKey(buildId)) objectStorageOperations.upload(uploadRequest) } From eef1629666dca296594789ba278911d1701ebab3 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 10 Dec 2024 18:37:42 +0100 Subject: [PATCH 8/9] fixed logic Signed-off-by: munishchouhan --- .../io/seqera/wave/service/logs/BuildLogServiceImpl.groovy | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy b/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy index aa265cd43..430fe587c 100644 --- a/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy +++ b/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy @@ -165,8 +165,11 @@ class BuildLogServiceImpl implements BuildLogService { log.info "Container Image is already cached, uploading previously successful build's condalock file for buildId: $buildId" def builds = persistenceService.allBuilds(buildId.split('-')[1].split('_')[0]) for (def build : builds) { - if ( build.succeeded() && !condaLock.contains('cat environment.lock') ){ - condaLock = fetchCondaLockString(build.buildId) + if ( build.succeeded() ){ + def curCondaLock = fetchCondaLockString(build.buildId) + if( curCondaLock && !curCondaLock.contains('cat environment.lock') ){ + condaLock = curCondaLock + } } } } From d0237cbb62b637ca653feb3e36ce40d7f446f202 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Wed, 11 Dec 2024 10:27:26 +0100 Subject: [PATCH 9/9] refactored and added tests Signed-off-by: munishchouhan --- .../service/logs/BuildLogServiceImpl.groovy | 37 +++++---- .../service/logs/BuildLogsServiceTest.groovy | 76 +++++++++++++++++++ 2 files changed, 97 insertions(+), 16 deletions(-) diff --git a/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy b/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy index 430fe587c..db3cdf830 100644 --- a/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy +++ b/src/main/groovy/io/seqera/wave/service/logs/BuildLogServiceImpl.groovy @@ -152,27 +152,18 @@ class BuildLogServiceImpl implements BuildLogService { if( !logs ) return try { String condaLock = extractCondaLockFile(logs) - if ( condaLock ){ - log.debug "Storing conda lock for buildId: $buildId" - - /* When a container image is cached, dockerfile does not get executed. + /* When a container image is cached, dockerfile does not get executed. In that case condalock file will contain "cat environment.lock" because its not been executed. So wave will check the previous builds of that container image and render the condalock file from latest successful build and replace with the current build's condalock file. */ - if( condaLock.contains('cat environment.lock') ){ - log.info "Container Image is already cached, uploading previously successful build's condalock file for buildId: $buildId" - def builds = persistenceService.allBuilds(buildId.split('-')[1].split('_')[0]) - for (def build : builds) { - if ( build.succeeded() ){ - def curCondaLock = fetchCondaLockString(build.buildId) - if( curCondaLock && !curCondaLock.contains('cat environment.lock') ){ - condaLock = curCondaLock - } - } - } - } + if( condaLock && condaLock.contains('cat environment.lock') ){ + condaLock = fetchValidCondaLock(buildId) + } + + if ( condaLock ){ + log.debug "Storing conda lock for buildId: $buildId" final uploadRequest = UploadRequest.fromBytes(condaLock.bytes, condaLockKey(buildId)) objectStorageOperations.upload(uploadRequest) } @@ -217,4 +208,18 @@ class BuildLogServiceImpl implements BuildLogService { .replaceAll(/#\d+ \d+\.\d+\s*/, '') } + String fetchValidCondaLock(String buildId) { + log.info "Container Image is already cached, uploading previously successful build's condalock file for buildId: $buildId" + def builds = persistenceService.allBuilds(buildId.split('-')[1].split('_')[0]) + for (def build : builds) { + if ( build.succeeded() ){ + def curCondaLock = fetchCondaLockString(build.buildId) + if( curCondaLock && !curCondaLock.contains('cat environment.lock') ){ + return curCondaLock + } + } + } + return null + } + } diff --git a/src/test/groovy/io/seqera/wave/service/logs/BuildLogsServiceTest.groovy b/src/test/groovy/io/seqera/wave/service/logs/BuildLogsServiceTest.groovy index 0aa1ee3e5..cd7d2bd18 100644 --- a/src/test/groovy/io/seqera/wave/service/logs/BuildLogsServiceTest.groovy +++ b/src/test/groovy/io/seqera/wave/service/logs/BuildLogsServiceTest.groovy @@ -22,13 +22,19 @@ import spock.lang.Specification import spock.lang.Unroll import io.micronaut.objectstorage.InputStreamMapper +import io.micronaut.objectstorage.ObjectStorageOperations import io.micronaut.objectstorage.aws.AwsS3Configuration +import io.micronaut.objectstorage.aws.AwsS3ObjectStorageEntry import io.micronaut.objectstorage.aws.AwsS3Operations +import io.seqera.wave.service.persistence.PersistenceService +import io.seqera.wave.service.persistence.WaveBuildRecord import io.seqera.wave.test.AwsS3TestContainer import software.amazon.awssdk.auth.credentials.AwsBasicCredentials import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider +import software.amazon.awssdk.core.ResponseInputStream import software.amazon.awssdk.regions.Region import software.amazon.awssdk.services.s3.S3Client +import software.amazon.awssdk.services.s3.model.GetObjectResponse /** * @@ -167,4 +173,74 @@ class BuildLogsServiceTest extends Specification implements AwsS3TestContainer { noExceptionThrown() } + def 'should return valid conda lock from previous successful build'() { + given: + def persistenceService = Mock(PersistenceService) + def objectStorageOperations = Mock(ObjectStorageOperations) + def service = new BuildLogServiceImpl(persistenceService: persistenceService, objectStorageOperations: objectStorageOperations) + def build1 = Mock(WaveBuildRecord) { + succeeded() >> false + buildId >> 'bd-abc_1' + } + def build2 = Mock(WaveBuildRecord) { + succeeded() >> true + buildId >> 'bd-abc_2' + } + def build3 = Mock(WaveBuildRecord) { + succeeded() >> true + buildId >> 'bd-abc_3' + } + def responseMetadata = GetObjectResponse.builder() + .contentLength(1024L) + .contentType("text/plain") + .build() + def contentStream = new ByteArrayInputStream("valid conda lock".bytes); + def responseInputStream = new ResponseInputStream<>(responseMetadata, contentStream); + persistenceService.allBuilds(_) >> [build1, build2] + objectStorageOperations.retrieve(service.condaLockKey('bd-abc_2')) >> Optional.of(new AwsS3ObjectStorageEntry('bd-abc_2', responseInputStream)) + + expect: + service.fetchValidCondaLock('bd-abc_3') == 'valid conda lock' + } + + def 'should return null when no successful build has valid conda lock'() { + given: + def persistenceService = Mock(PersistenceService) + def objectStorageOperations = Mock(ObjectStorageOperations) + def service = new BuildLogServiceImpl(persistenceService: persistenceService, objectStorageOperations: objectStorageOperations) + def build1 = Mock(WaveBuildRecord) { + succeeded() >> false + buildId >> 'bd-abc_1' + } + def build2 = Mock(WaveBuildRecord) { + succeeded() >> true + buildId >> 'bd-abc_2' + } + def build3 = Mock(WaveBuildRecord) { + succeeded() >> true + buildId >> 'bd-abc_3' + } + def responseMetadata = GetObjectResponse.builder() + .contentLength(1024L) + .contentType("text/plain") + .build() + def contentStream = new ByteArrayInputStream("cat environment.lock".bytes); + def responseInputStream = new ResponseInputStream<>(responseMetadata, contentStream); + persistenceService.allBuilds(_) >> [build1, build2] + objectStorageOperations.retrieve(service.condaLockKey('bd-abc_2')) >> Optional.of(new AwsS3ObjectStorageEntry('bd-abc_2', responseInputStream)) + + expect: + service.fetchValidCondaLock('bd-abc_3') == null + } + + def 'should return null when no builds are available'() { + given: + def persistenceService = Mock(PersistenceService) + def service = new BuildLogServiceImpl(persistenceService: persistenceService) + persistenceService.allBuilds(_) >> [] + + expect: + service.fetchValidCondaLock('bd-abc_1') == null + } + }