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.

Sign up for GitHub

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

Jump to bottom

Get QEMU logs from virtlogd #3692

Merged
merged 2 commits into from Jul 9, 2020
Merged

Get QEMU logs from virtlogd #3692

merged 2 commits into from Jul 9, 2020

Conversation

victortoso
Copy link
Member

What this PR does / why we need it:

  • It reads the logs from /var/log/libvirt/qemu/domain.log (which is handled by virtlogd), structuralizes it and log them with kubevirt.io/client-go/log so it is reachable outside the pod

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2953

Special notes for your reviewer:
This is a WIP, I want to double check that this is the right direction. I'm just reading from the file created by virtlogd for now.

Release note:

QEMU logs can now be fetched from outside the pod

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 27, 2020
@openshift-ci-robot
Copy link
Collaborator

Hi @victortoso. Thanks for your PR.

I'm waiting for a kubevirt member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 27, 2020
@kubevirt-bot
Copy link
Contributor

Hi @victortoso. Thanks for your PR.

I'm waiting for a kubevirt member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@victortoso
Copy link
Member Author

CC @enp0s3

@victortoso
libvirt: rename libvirt -> virtlogd
9b12bde 
Failures in StartVirtlog are related to /usr/sbin/virtlogd which is
handling virtual's machine log for libvirt.

Signed-off-by: Victor Toso <victortoso@redhat.com>
rmohr
rmohr reviewed Jun 29, 2020
View reviewed changes
Copy link
Member

@rmohr rmohr left a comment

Choose a reason for hiding this comment

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

@victortoso thanks for the PR. One question mostly.

pkg/virt-launcher/virtwrap/util/libvirt_helper.go Show resolved Hide resolved
@victortoso
Export qemu logs as structured logs
9e66e80 
QEMU logs are handled by virtlogd which defaults to creating a file to
store the logs. Those logs are not that verbose but it might help in
case of unexpected crashes.

Signed-off-by: Victor Toso <victortoso@redhat.com>
@kubevirt-bot kubevirt-bot added size/L and removed size/M labels Jul 2, 2020
@victortoso
Copy link
Member Author

Besides adding a unit test, I changed the subcomponent to "qemu" as suggested by @enp0s3 and renamed the log function to reflect that.

@rmohr
Copy link
Member

rmohr commented Jul 8, 2020

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 8, 2020
@rmohr
Copy link
Member

rmohr commented Jul 8, 2020

Let's see what the tests say about the change. @victortoso thanks.

@rmohr
Copy link
Member

rmohr commented Jul 9, 2020

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rmohr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 9, 2020
@kubevirt-bot kubevirt-bot merged commit f6ec7bd into kubevirt:master Jul 9, 2020
@victortoso victortoso deleted the qemu-logs branch August 12, 2020 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmohr rmohr rmohr left review comments

@enp0s3 enp0s3 enp0s3 left review comments

@booxter booxter Awaiting requested review from booxter

@vatsalparekh vatsalparekh Awaiting requested review from vatsalparekh

Assignees

@rmohr rmohr

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Evacuate QEMU logs from virt-launcher pod
5 participants
@victortoso @openshift-ci-robot @kubevirt-bot @rmohr @enp0s3