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

Adding the pod logs #49

Closed
tjungblu opened this issue Jan 13, 2022 · 8 comments
Closed

Adding the pod logs #49

tjungblu opened this issue Jan 13, 2022 · 8 comments
Assignees

Comments

@tjungblu
Copy link
Contributor

(yet another follow-up to the conversations that triggered #44 + #47 + #48 - last one for now)

Our customer would like to correlate pod logs, this is potentially easy done in OpenShift as you can take the pod info from the dump and search in the logging system. However, because our customer is a vendor the cluster logging system might not be accessed by the respective tenants very easily.

Another "easy" solution would be to include the pod log (up until the crash) into the zip file. My naive thought would include a simply cp from the configured log directory on the host. Alternatively this could be done with kubectl log going through the API server.

What's your take on the story?

@No9
Copy link
Collaborator

No9 commented Jan 13, 2022

For the cp approach there are lot of different logging variations that would have to be catered for in order for it to be generalised:

I would also be a bit hesitant on querying the container through the API as the pod may be an unknown state given that the process has SIG'ed. The permissions aspect will also need-- to be worked out as the kubeapi will be from the core composer on the host. But it may be worth investigating this as it would be more generally applicable if it works.
Looking at the options in crictl it's possible to get the logs for a container with crictl logs $containerid
The $containerid is obtained from the pod ps info so this should be relatively straight forward to implement

This is coupled with #47 (comment) makes me think they may be better with a post processor but I'm open to keep discussing options.

@No9
Copy link
Collaborator

No9 commented Jan 16, 2022

Moved the crictl code into a specific library for reuse in another project and added a log method to it.
https://docs.rs/libcrio/latest/libcrio/struct.Cli.html#method.logs
I'll integrate that in on the next release

@No9 No9 self-assigned this Jan 21, 2022
@No9
Copy link
Collaborator

No9 commented Jan 23, 2022

Initial implementation here de95956
On reflection shipping the whole log is not optimal and could potentially timeout the dump.
I'm going to switch this to be a tail command where the default size of the tail will be 500 lines (Subject to validation)
See No9/libcrio#1

@No9
Copy link
Collaborator

No9 commented Jan 23, 2022

Moving this to done but keeping this open to confirm discussion on log tail approach.

@tjungblu
Copy link
Contributor Author

I think the tailing is good enough already. Thanks so much for your work @No9 :)

@No9
Copy link
Collaborator

No9 commented Jan 26, 2022

Great - I have an initial cut of the tail feature on this branch - https://github.com/IBM/core-dump-handler/tree/pod-logs
It has your openshift.yaml work and points to an specific release candidate so you should be able to run it using the usual steps.
I've also updated the segfaulter container to generate a 1000 log lines before crashing.
Can you give it a spin when you have time and confirm you are getting 500 lines in the log file that is in the zip that would be great.
Also any feedback on the docs updates would be great.
If we are good on those bits I just need to add more hardening to the agent CI tests and I'll cut a release.

@tjungblu
Copy link
Contributor Author

just tested this again with OpenShift 4.9 on AWS. It works like a charm :) Thanks a lot once more, @No9.

Also any feedback on the docs updates would be great.

do you want to create a PR maybe? happy to review it through there

@No9
Copy link
Collaborator

No9 commented Jan 26, 2022

Thanks for looking at it @tjungblu
I've PRd the whole change set here #53
Closing this as we've validated the log collection.

@No9 No9 closed this as completed Jan 26, 2022
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

No branches or pull requests

2 participants