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

Fix #35 'kubectl logs -f' implemented #332

Merged

Conversation

antoinetran
Copy link
Contributor

Summary

Implement kubectl logs -f in interlink VK and API side. In case of following parameter set, it will reads and write continuously with a buffer from API to VK.
Also improved InterLink internal log, so that a sessionContext (eg: GetLogs#12345) is created and added in HTTP header, so that we can also follow the HTTP request from VK to API to Plugin with the same id.

Related issue :
interTwin-eu/interlink-slurm-plugin#35

@dciangot dciangot self-requested a review December 3, 2024 14:32
Copy link
Collaborator

@dciangot dciangot left a comment

Choose a reason for hiding this comment

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

@antoinetran thank you for this huge contribution. It looks very good to me, just find one comment where I need a clarification on the design. Otherwise I think it's good to go

pkg/virtualkubelet/execute.go Show resolved Hide resolved
@antoinetran
Copy link
Contributor Author

I should have put some info in PullRequest instead of issue35:

TODO for later: when doing Ctrl+C to kubectl logs -f, stops following logs for that session. But I don't know how to detect that and react.

@antoinetran antoinetran force-pushed the fix_issue35_kubectllogsfollow_rebased branch from b0eb432 to 81a4a12 Compare December 12, 2024 16:30
@dciangot
Copy link
Collaborator

@antoinetran looks like the last commit broke the compilation, can you take a look?

@dciangot
Copy link
Collaborator

mmm weird... repeating the tests locally shows no problem... maybe they broke on the force push.

So, nvm. I'm push this through.

@dciangot dciangot self-requested a review December 13, 2024 08:18
Copy link
Collaborator

@dciangot dciangot left a comment

Choose a reason for hiding this comment

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

LGTM

@dciangot dciangot merged commit 12f12a7 into interTwin-eu:main Dec 13, 2024
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants