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

Sync previous container logs #113

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

jrauh01
Copy link
Collaborator

@jrauh01 jrauh01 commented Aug 5, 2024

No description provided.

@jrauh01 jrauh01 self-assigned this Aug 5, 2024
@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Aug 5, 2024
Copy link
Member

@lippserd lippserd left a comment

Choose a reason for hiding this comment

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

There is now a lot of duplicated code for basically doing the same. I think creating the log options and making the actual request to the Kubernetes API could be moved to a function.

Also, It might make sense to only sync previous container logs if it has been restarted since the last sync. We would need Pod.ContainerStatuses[].LastTerminationState for that which is currently not reflected in our Container. Though it makes sense to also synchronize that to the database so feel free to create a PR for this one beforehand.

If I'm not mistaken, logs for terminated containers are not collected at all. There is a started.Bool check somewhere. This should be adjusted as well.

@lippserd lippserd added this to the 0.3.0 milestone Aug 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed CLA is signed by all contributors of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants