Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable streaming of build logs #459

Draft
wants to merge 43 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
ff70a52
add getlogs method
munishchouhan Apr 17, 2024
6f3cd8f
fixed builderName
munishchouhan Apr 17, 2024
16481a0
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Apr 18, 2024
8af97ef
fixed DockerBuildStrategyTest
munishchouhan Apr 18, 2024
22a574e
Merge remote-tracking branch 'origin/451-enable-streaming-of-build-lo…
munishchouhan Apr 18, 2024
d228dd9
added BuildLogLocalServiceImpl
munishchouhan Apr 18, 2024
a5ba7d9
added getCurrentLogsPod(String name)
munishchouhan Apr 18, 2024
6cb5e0e
fixed logs color issue
munishchouhan Apr 18, 2024
eafcb94
added build in progress phase and dynamic logs loading
munishchouhan Apr 18, 2024
c335997
minor change
munishchouhan Apr 18, 2024
26fdb65
changed to inputstream
munishchouhan Apr 19, 2024
93491f5
strip ansi escape codes
munishchouhan Apr 19, 2024
7869323
added docs
munishchouhan Apr 19, 2024
b287a56
fixed tests
munishchouhan Apr 19, 2024
a294b64
added test
munishchouhan Apr 19, 2024
94f40e7
corrected docs
munishchouhan Apr 19, 2024
db5afde
corrected docs
munishchouhan Apr 19, 2024
f6bb71c
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Apr 22, 2024
e7313e7
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Apr 22, 2024
1def98a
updated doc [ci skip]
munishchouhan Apr 22, 2024
a43a160
Merge remote-tracking branch 'origin/451-enable-streaming-of-build-lo…
munishchouhan Apr 22, 2024
a559584
reverted doc changes
munishchouhan Apr 22, 2024
0c06d3c
Merge branch 'master' into 451-enable-streaming-of-build-logs
pditommaso Apr 23, 2024
153b0df
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Apr 23, 2024
96afbb9
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan May 4, 2024
534da81
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan May 15, 2024
4859154
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Jun 11, 2024
fd9ea32
fix tests
munishchouhan Jun 11, 2024
7dedb5f
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Jul 22, 2024
aeb386c
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Aug 14, 2024
0c53ec5
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Aug 20, 2024
59f25e8
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Aug 29, 2024
1511ddb
fixed errors
munishchouhan Aug 29, 2024
167a9ad
fixed errors
munishchouhan Aug 30, 2024
1752d55
master merged
munishchouhan Oct 17, 2024
e09c9ec
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Oct 22, 2024
d6b19e5
fixed error
munishchouhan Oct 22, 2024
ed403ee
fixed error
munishchouhan Oct 22, 2024
1c304b1
fixed tests
munishchouhan Oct 22, 2024
7b09548
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Nov 5, 2024
505c303
Merge branch 'master' into 451-enable-streaming-of-build-logs
munishchouhan Jan 15, 2025
f92a301
fixed error
munishchouhan Jan 15, 2025
5942bd8
fixed tests
munishchouhan Jan 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ class ViewController {
final binding = new HashMap(20)
binding.build_id = result.buildId
binding.build_success = result.succeeded()
binding.build_failed = result.exitStatus && result.exitStatus != 0
binding.build_in_progress = result.exitStatus == null
binding.build_exit_status = result.exitStatus
binding.build_user = (result.userName ?: '-') + " (ip: ${result.requestIp})"
binding.build_time = formatTimestamp(result.startTime, result.offsetId) ?: '-'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ abstract class BuildStrategy {

abstract BuildResult build(BuildRequest req)

abstract String getLogs(String buildId)
pditommaso marked this conversation as resolved.
Show resolved Hide resolved

void cleanup(BuildRequest req) {
req.workDir?.deleteDir()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,13 @@ class DockerBuildStrategy extends BuildStrategy {
final spack = req.isSpackBuild ? spackConfig : null

final dockerCmd = req.formatDocker()
? cmdForKaniko( req.workDir, credsFile, spack, req.platform)
: cmdForSingularity( req.workDir, credsFile, spack, req.platform)
? cmdForKaniko( req.workDir, credsFile, spack, req.platform, req.buildId)
: cmdForSingularity( req.workDir, credsFile, spack, req.platform, req.buildId)

return dockerCmd + launchCmd(req)
}

protected List<String> cmdForKaniko(Path workDir, Path credsFile, SpackConfig spackConfig, ContainerPlatform platform ) {
protected List<String> cmdForKaniko(Path workDir, Path credsFile, SpackConfig spackConfig, ContainerPlatform platform, String buildId) {
final wrapper = ['docker',
'run',
'--rm',
Expand All @@ -137,13 +137,18 @@ class DockerBuildStrategy extends BuildStrategy {
wrapper.add('--platform')
wrapper.add(platform.toString())
}

//add builder name
wrapper.add('--name')
wrapper.add(builderName(buildId))

// the container image to be used t
wrapper.add( buildConfig.kanikoImage )
// return it
return wrapper
}

protected List<String> cmdForSingularity(Path workDir, Path credsFile, SpackConfig spackConfig, ContainerPlatform platform) {
protected List<String> cmdForSingularity(Path workDir, Path credsFile, SpackConfig spackConfig, ContainerPlatform platform, String buildId) {
final wrapper = ['docker',
'run',
'--rm',
Expand All @@ -170,7 +175,26 @@ class DockerBuildStrategy extends BuildStrategy {
wrapper.add(platform.toString())
}

//add builder name
wrapper.add('--name')
wrapper.add(builderName(buildId))

wrapper.add(buildConfig.singularityImage(platform))
return wrapper
}

protected static String builderName(String buildId) {
return "build-${buildId}".toString().replace('_', '-')
}

@Override
String getLogs(String buildId) {
def logCmd = ['docker', 'logs'] + builderName(buildId)
log.info("Get build logs: ${logCmd.join(' ')}")
final proc = new ProcessBuilder()
.command(logCmd)
.redirectErrorStream(true)
.start()
return proc.inputStream.text
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ class KubeBuildStrategy extends BuildStrategy {
@Inject
private RegistryProxyService proxyService

protected String podName(BuildRequest req) {
return "build-${req.buildId}".toString().replace('_', '-')
protected String podName(String buildId) {
return "build-${buildId}".toString().replace('_', '-')
}

@Override
Expand All @@ -96,7 +96,7 @@ class KubeBuildStrategy extends BuildStrategy {
try {
final buildImage = getBuildImage(req)
final buildCmd = launchCmd(req)
final name = podName(req)
final name = podName(req.buildId)
final selector= getSelectorLabel(req.platform, nodeSelectorMap)
final spackCfg0 = req.isSpackBuild ? spackConfig : null
final pod = k8sService.buildContainer(name, buildImage, buildCmd, req.workDir, configFile, spackCfg0, selector)
Expand Down Expand Up @@ -130,7 +130,7 @@ class KubeBuildStrategy extends BuildStrategy {
@Override
void cleanup(BuildRequest req) {
super.cleanup(req)
final name = podName(req)
final name = podName(req.buildId)
try {
k8sService.deletePod(name)
}
Expand All @@ -139,4 +139,9 @@ class KubeBuildStrategy extends BuildStrategy {
}
}

@Override
String getLogs(String buildId) {
return k8sService.getCurrentLogsPod(podName(buildId))
}

}
2 changes: 2 additions & 0 deletions src/main/groovy/io/seqera/wave/service/k8s/K8sService.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,6 @@ interface K8sService {
V1Pod transferContainer(String name, String containerImage, List<String> args, BlobCacheConfig blobConfig)

V1ContainerStateTerminated waitPod(V1Pod pod, long timeout)

String getCurrentLogsPod(String name)
}
19 changes: 19 additions & 0 deletions src/main/groovy/io/seqera/wave/service/k8s/K8sServiceImpl.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package io.seqera.wave.service.k8s

import java.nio.file.Path
import java.util.stream.Collectors
import javax.annotation.PostConstruct

import groovy.transform.CompileDynamic
Expand All @@ -35,6 +36,7 @@ import io.kubernetes.client.openapi.models.V1JobBuilder
import io.kubernetes.client.openapi.models.V1PersistentVolumeClaimVolumeSource
import io.kubernetes.client.openapi.models.V1Pod
import io.kubernetes.client.openapi.models.V1PodBuilder
import io.kubernetes.client.openapi.models.V1PodStatus
import io.kubernetes.client.openapi.models.V1ResourceRequirements
import io.kubernetes.client.openapi.models.V1Volume
import io.kubernetes.client.openapi.models.V1VolumeMount
Expand Down Expand Up @@ -460,6 +462,23 @@ class K8sServiceImpl implements K8sService {
}
}

/**
* Fetch current available logs of a running pod
*
* @param name The pod name
* @return The logs as a string or when logs are not available or cannot be accessed
*/
@Override
String getCurrentLogsPod(String name) {
try {
return k8sClient.coreV1Api().readNamespacedPodLog(name, namespace, null, null, null, null, null, null, null, null, null)
} catch (Exception e) {
// logging trace here because errors are expected when the pod is not running
log.trace "Unable to fetch logs for pod: $name", e
return null
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

@munishchouhan munishchouhan Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did tried it, but streamNamespacedPodLog() waits till the pod execution is completed. I will dig more

Copy link
Member Author

@munishchouhan munishchouhan Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will move the logic to convert string to inputstream to this function so that getLogs return input stream

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this is a limitation of java client or a bug, but I'm quite sure it's possible to stream the logs of a running pod.

I've made a simple test running a pod like this

kubectl run counter --image=busybox --command -- /bin/sh -c 'n=0; while true; do echo $((n++)); sleep 1;

Then I've used the K8s client implemented in Nextflow, that's a bare simple http request over URLConnection.

This snippet just stream the pod logs until the pod completes

    def 'should get pod log' () {
        given:
        def settings = ['context': 'docker-desktop', name: 'default']
        def config = new K8sConfig(settings)
        def client = new K8sClient( config.getClient() )

        when:
        def logs = client.podLog([follow:'true'], 'counter')
        then:
        logs.text == 'hello\n'
    }


/**
* Delete a pod
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,23 @@

package io.seqera.wave.service.logs

import java.nio.charset.StandardCharsets
import java.util.concurrent.CompletableFuture
import io.micronaut.core.annotation.Nullable

import groovy.transform.CompileStatic
import groovy.util.logging.Slf4j
import io.micronaut.context.annotation.Requires
import io.micronaut.context.annotation.Value
import io.micronaut.http.MediaType
import io.micronaut.http.server.types.files.StreamedFile
import io.micronaut.objectstorage.ObjectStorageEntry
import io.micronaut.objectstorage.ObjectStorageOperations
import io.micronaut.objectstorage.request.UploadRequest
import io.micronaut.runtime.event.annotation.EventListener
import io.seqera.wave.service.builder.BuildEvent
import io.seqera.wave.service.builder.BuildRequest
import io.seqera.wave.service.builder.BuildStrategy
import io.seqera.wave.service.persistence.PersistenceService
import jakarta.annotation.PostConstruct
import jakarta.inject.Inject
Expand All @@ -56,6 +59,9 @@ class BuildLogServiceImpl implements BuildLogService {
@Inject
private PersistenceService persistenceService

@Inject
private BuildStrategy buildStrategy

@Nullable
@Value('${wave.build.logs.prefix}')
private String prefix
Expand Down Expand Up @@ -107,7 +113,18 @@ class BuildLogServiceImpl implements BuildLogService {
private StreamedFile fetchLogStream0(String buildId) {
if( !buildId ) return null
final Optional<ObjectStorageEntry<?>> result = objectStorageOperations.retrieve(logKey(buildId))
return result.isPresent() ? result.get().toStreamedFile() : null
return result.isPresent() ? result.get().toStreamedFile() : fetchLogStream1(buildId)
}

private StreamedFile fetchLogStream1(String buildId) {
def logs = buildStrategy.getLogs(buildId)
if( !logs )
return null
else
//replace all regex is removing color from log otherwise it will not be displayed correctly in browser
logs = logs.replaceAll("\u001B\\[[;\\d]*m", "")
def inputStream = new ByteArrayInputStream(logs.getBytes(StandardCharsets.UTF_8))
return inputStream ? new StreamedFile(inputStream, MediaType.TEXT_HTML_TYPE) : null
}

@Override
Expand Down
48 changes: 46 additions & 2 deletions src/main/resources/io/seqera/wave/build-view.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,18 @@
Container build completed successfully!
</h4>
</div>
{{else}}
{{else build_failed}}
<div style="color: #a94442; background-color: #f2dede; padding: 15px; border: 1px solid transparent; border-radius: 4px;">
<h4 style="margin-top:0; color: inherit;">
Container build failed
</h4>
</div>
{{else build_in_progress}}
<div style="color: #3c763d; background-color: #ffd300; padding: 15px; border: 1px solid transparent; border-radius: 4px;">
<h4 style="margin-top:0; color: inherit;">
Container build in progress
</h4>
</div>
{{/if}}

<h3>Summary</h3>
Expand Down Expand Up @@ -98,12 +104,50 @@

{{#if build_log_data}}
<h3>Build logs</h3>
<pre style="white-space: pre-wrap; overflow-x: auto; overflow-y: auto; background-color: #ededed; padding: 15px; border-radius: 4px; margin-bottom:30px;">{{build_log_data}}</pre>
<pre id="buildLogs" style="white-space: pre-wrap; overflow-x: auto; overflow-y: auto; background-color: #ededed; padding: 15px; border-radius: 4px; margin-bottom:30px;">{{build_log_data}}</pre>
{{#if build_log_truncated}}
<a href="{{build_log_url}}" download>Click here to download the complete build log</a>
{{/if}}
{{/if}}

<script>
function checkStatusAndFetchData() {
fetch("{{server_url}}/v1alpha1/builds/{{build_id}}/status", {method: 'GET'})
.then(response => {
if (!response.ok) {
clearInterval(interval)
return;
}
return response.json();
}).then(statusResponse => {
if (statusResponse.status === 'PENDING') {
fetchData();
}else{
clearInterval(interval)
}
}).catch(error => {
console.error('Error checking status:', error)
clearInterval(interval)
});
}
function fetchData() {
fetch('{{server_url}}/v1alpha1/builds/{{build_id}}/logs', {method: 'GET', cache: 'no-cache'})
.then(response => {
if (!response.ok) {
clearInterval(interval)
return;
}
return response.text();
}).then(logs => {
document.getElementById('buildLogs').innerHTML = logs;
}).catch(error => {
console.error('Error fetching build logs:', error)
clearInterval(interval)
});
}

const interval = setInterval(checkStatusAndFetchData, 2000);
</script>

<div class="footer" style="clear:both;width:100%;">
<hr class="footer-hr" style="height:0;overflow:visible;margin-top:30px;border:0;border-top:1px solid #eee;color:#999999;font-size:12px;line-height:18px;margin-bottom:30px;">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,34 +45,37 @@ class DockerBuildStrategyTest extends Specification {
and:
def work = Path.of('/work/foo')
when:
def cmd = service.cmdForKaniko(work, null, null, null)
def cmd = service.cmdForKaniko(work, null, null, null, '1234')
then:
cmd == ['docker',
'run',
'--rm',
'-v', '/work/foo:/work/foo',
'--name', 'build-1234',
'gcr.io/kaniko-project/executor:v1.19.2']

when:
cmd = service.cmdForKaniko(work, Path.of('/foo/creds.json'), null, ContainerPlatform.of('arm64'))
cmd = service.cmdForKaniko(work, Path.of('/foo/creds.json'), null, ContainerPlatform.of('arm64'), '1234')
then:
cmd == ['docker',
'run',
'--rm',
'-v', '/work/foo:/work/foo',
'-v', '/foo/creds.json:/kaniko/.docker/config.json:ro',
'--platform', 'linux/arm64',
'--name', 'build-1234',
'gcr.io/kaniko-project/executor:v1.19.2']

when:
cmd = service.cmdForKaniko(work, Path.of('/foo/creds.json'), spackConfig, null)
cmd = service.cmdForKaniko(work, Path.of('/foo/creds.json'), spackConfig, null, '1234')
then:
cmd == ['docker',
'run',
'--rm',
'-v', '/work/foo:/work/foo',
'-v', '/foo/creds.json:/kaniko/.docker/config.json:ro',
'-v', '/host/spack/key:/opt/spack/key:ro',
'--name', 'build-1234',
'gcr.io/kaniko-project/executor:v1.19.2']

cleanup:
Expand All @@ -87,6 +90,7 @@ class DockerBuildStrategyTest extends Specification {
def creds = Path.of('/work/creds.json')
and:
def req = new BuildRequest(
buildId: '1234',
workDir: Path.of('/work/foo/89fb83ce6ec8627b'),
platform: ContainerPlatform.of('linux/amd64'),
targetImage: 'repo:89fb83ce6ec8627b',
Expand All @@ -100,6 +104,7 @@ class DockerBuildStrategyTest extends Specification {
'-v', '/work/foo/89fb83ce6ec8627b:/work/foo/89fb83ce6ec8627b',
'-v', '/work/creds.json:/kaniko/.docker/config.json:ro',
'--platform', 'linux/amd64',
'--name', 'build-1234',
'gcr.io/kaniko-project/executor:v1.19.2',
'--dockerfile', '/work/foo/89fb83ce6ec8627b/Containerfile',
'--context', '/work/foo/89fb83ce6ec8627b/context',
Expand All @@ -118,6 +123,7 @@ class DockerBuildStrategyTest extends Specification {
def service = ctx.getBean(DockerBuildStrategy)
and:
def req = new BuildRequest(
buildId: '1234',
workDir: Path.of('/work/foo/89fb83ce6ec8627b'),
platform: ContainerPlatform.of('linux/amd64'),
targetImage: 'repo:89fb83ce6ec8627b',
Expand Down Expand Up @@ -151,6 +157,7 @@ class DockerBuildStrategyTest extends Specification {
def creds = Path.of('/work/creds.json')
and:
def req = new BuildRequest(
buildId: '1234',
workDir: Path.of('/work/foo/d4869cc39b8d7d55'),
platform: ContainerPlatform.of('linux/amd64'),
targetImage: 'oras://repo:d4869cc39b8d7d55',
Expand All @@ -170,6 +177,7 @@ class DockerBuildStrategyTest extends Specification {
'-v', '/work/singularity-remote.yaml:/root/.singularity/remote.yaml:ro',
'-v', '/host/spack/key:/opt/spack/key:ro',
'--platform', 'linux/amd64',
'--name', 'build-1234',
'quay.io/singularity/singularity:v3.11.4-slim',
'sh',
'-c',
Expand All @@ -193,6 +201,7 @@ class DockerBuildStrategyTest extends Specification {
def creds = Path.of('/work/creds.json')
and:
def req = new BuildRequest(
buildId: '1234',
workDir: Path.of('/work/foo/9c68af894bb2419c'),
platform: ContainerPlatform.of('linux/arm64'),
targetImage: 'oras://repo:9c68af894bb2419c',
Expand All @@ -212,6 +221,7 @@ class DockerBuildStrategyTest extends Specification {
'-v', '/work/singularity-remote.yaml:/root/.singularity/remote.yaml:ro',
'-v', '/host/spack/key:/opt/spack/key:ro',
'--platform', 'linux/arm64',
'--name', 'build-1234',
'quay.io/singularity/singularity:v3.11.4-slim-arm64',
'sh',
'-c',
Expand Down
Loading