-
Notifications
You must be signed in to change notification settings - Fork 4
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
base: master
Are you sure you want to change the base?
Conversation
src/main/groovy/io/seqera/wave/service/builder/DockerBuildStrategy.groovy
Outdated
Show resolved
Hide resolved
…gs' into 451-enable-streaming-of-build-logs
@pditommaso is this required or should it refresh automatically? wavebuildpodlogs1.mov |
If only build box content is refreshed while the status is PENDING, it would be cool |
I will give it a try |
@pditommaso build logs with refresh now refreshwavebuildlogs.mov |
src/main/groovy/io/seqera/wave/service/builder/BuildStrategy.groovy
Outdated
Show resolved
Hide resolved
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 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be considered to use a streaming approach, see
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It happens because podlogs set the follow true and under the hood use readNamespacedPodLog
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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'
}
@pditommaso its ready for review |
…gs' into 451-enable-streaming-of-build-logs
I have moved the docs changes to #464 |
Suggestion from @ewels |
Please keep colour outside of this PR. We need fist makes the logs stream to work |
I will create another issue |
Signed-off-by: munishchouhan <[email protected]>
Signed-off-by: munishchouhan <[email protected]>
merged get messed up, moving to draft |
Signed-off-by: munishchouhan <[email protected]>
Signed-off-by: munishchouhan <[email protected]>
Signed-off-by: munishchouhan <[email protected]>
This PR will enalbe log streaming to get build log endpoint, while build is in process