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

agent: Add logging to libvirt qemu hook and cleanup #2554

Merged
merged 1 commit into from Apr 16, 2018

Conversation

Projects
None yet
5 participants
@rhtyd
Member

rhtyd commented Apr 9, 2018

This allows logging to the default libvirt qemu hook and cleans up the script.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

Installed the hook script on centos6 and xenial/16.04 based kvm hosts, changes the logging scope to DEBUG, performed VM migration, which caused the following logs:

16:57:25,61 qemu-hook DEBUG Executing qemu-hook with args: ['/etc/libvirt/hooks/qemu', 'i-2-39-VM', 'stopped', 'end', '-']
16:57:25,275 qemu-hook DEBUG Executing qemu-hook with args: ['/etc/libvirt/hooks/qemu', 'i-2-39-VM', 'release', 'end', '-']
16:57:34,685 qemu-hook DEBUG Executing qemu-hook with args: ['/etc/libvirt/hooks/qemu', 'i-2-39-VM', 'migrate', 'begin', '-']
16:57:34,904 qemu-hook DEBUG Executing qemu-hook with args: ['/etc/libvirt/hooks/qemu', 'i-2-39-VM', 'prepare', 'begin', '-']
16:57:36,880 qemu-hook DEBUG Executing qemu-hook with args: ['/etc/libvirt/hooks/qemu', 'i-2-39-VM', 'start', 'begin', '-']
16:57:37,388 qemu-hook DEBUG Executing qemu-hook with args: ['/etc/libvirt/hooks/qemu', 'i-2-39-VM', 'started', 'begin', '-']

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
    Testing
  • I have added tests to cover my changes.
  • All relevant new and existing integration tests have passed.
  • A full integration testsuite with all test that can run on my environment has passed.

@blueorangutan package

@rhtyd rhtyd changed the base branch from master to 4.11 Apr 9, 2018

@blueorangutan

This comment has been minimized.

Show comment
Hide comment
@blueorangutan

blueorangutan Apr 9, 2018

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan commented Apr 9, 2018

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@rhtyd rhtyd added the kvm label Apr 9, 2018

@rhtyd rhtyd added this to the 4.11.1.0 milestone Apr 9, 2018

@rhtyd rhtyd requested review from DaanHoogland and rafaelweingartner Apr 9, 2018

from xml.dom.minidom import parse
from cloudutils.configFileOps import configFileOps
from cloudutils.networkConfig import networkConfig
logging.basicConfig(filename='/var/log/libvirt/qemu-hook.log',

This comment has been minimized.

@rhtyd

rhtyd Apr 9, 2018

Member

FYI - the directory /var/log/libvirt/ exists if libvirt is installed.

@rhtyd

rhtyd Apr 9, 2018

Member

FYI - the directory /var/log/libvirt/ exists if libvirt is installed.

This comment has been minimized.

@DaanHoogland

DaanHoogland Apr 9, 2018

Contributor

so, you mean no safe guard needed?

@DaanHoogland

DaanHoogland Apr 9, 2018

Contributor

so, you mean no safe guard needed?

This comment has been minimized.

@rhtyd

rhtyd Apr 10, 2018

Member

Yes.

@rhtyd

rhtyd Apr 10, 2018

Member

Yes.

from xml.dom.minidom import parse
from cloudutils.configFileOps import configFileOps
from cloudutils.networkConfig import networkConfig
logging.basicConfig(filename='/var/log/libvirt/qemu-hook.log',

This comment has been minimized.

@DaanHoogland

DaanHoogland Apr 9, 2018

Contributor

so, you mean no safe guard needed?

@DaanHoogland

DaanHoogland Apr 9, 2018

Contributor

so, you mean no safe guard needed?

Show outdated Hide outdated agent/bindir/libvirtqemuhook.in Outdated
@blueorangutan

This comment has been minimized.

Show comment
Hide comment
@blueorangutan

blueorangutan Apr 9, 2018

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1890

blueorangutan commented Apr 9, 2018

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1890

agent: Add logging to libvirt qemu hook
This allows logging to the default libvirt qemu hook

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

@rhtyd rhtyd requested a review from nvazquez Apr 11, 2018

handleMigrateBegin()
# For docs refer https://libvirt.org/hooks.html#qemu
logger.debug("Executing qemu hook with args: %s" % sys.argv)
action, status = sys.argv[2:4]

This comment has been minimized.

@rafaelweingartner

rafaelweingartner Apr 12, 2018

Member

Just to make sure I understood this notation here. I was reading in the Internet about the notation to deal with arrays, and it seems that it is not inclusive (with respect of the first index used to define the range). I mean, if I want everything after the first parameter, which is the script name I can do the following:
user_args = sys.argv[1:]

Therefore, the code that we have here would assign parameter 3 to variable action, and parameter 4 to variable status. Is this true? I was under the impression that the variable action should be parameter 2, and status was expected to be parameter 3.

@rafaelweingartner

rafaelweingartner Apr 12, 2018

Member

Just to make sure I understood this notation here. I was reading in the Internet about the notation to deal with arrays, and it seems that it is not inclusive (with respect of the first index used to define the range). I mean, if I want everything after the first parameter, which is the script name I can do the following:
user_args = sys.argv[1:]

Therefore, the code that we have here would assign parameter 3 to variable action, and parameter 4 to variable status. Is this true? I was under the impression that the variable action should be parameter 2, and status was expected to be parameter 3.

This comment has been minimized.

@rafaelweingartner

rafaelweingartner Apr 12, 2018

Member

Ah.. never mind. After reading some more time the same explanation I think I understood now.
It is not inclusive with the upper bound parameter. Moreover, the arrays are indexed from 0. That is why if we want to ignore the script name, we start getting parameters that are in the first position.

@rafaelweingartner

rafaelweingartner Apr 12, 2018

Member

Ah.. never mind. After reading some more time the same explanation I think I understood now.
It is not inclusive with the upper bound parameter. Moreover, the arrays are indexed from 0. That is why if we want to ignore the script name, we start getting parameters that are in the first position.

@rhtyd

This comment has been minimized.

Show comment
Hide comment
@rhtyd
Member

rhtyd commented Apr 12, 2018

@blueorangutan

This comment has been minimized.

Show comment
Hide comment
@blueorangutan

blueorangutan Apr 12, 2018

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan commented Apr 12, 2018

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan

This comment has been minimized.

Show comment
Hide comment
@blueorangutan

blueorangutan Apr 13, 2018

Trillian test result (tid-2499)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 93870 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2554-t2499-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermitten failure detected: /marvin/tests/smoke/test_routers.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Intermitten failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 66 look OK, 1 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_04_restart_network_wo_cleanup Failure 2.78 test_routers.py

blueorangutan commented Apr 13, 2018

Trillian test result (tid-2499)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 93870 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2554-t2499-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermitten failure detected: /marvin/tests/smoke/test_routers.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Intermitten failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 66 look OK, 1 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_04_restart_network_wo_cleanup Failure 2.78 test_routers.py
@borisstoyanov

LGTM

@DaanHoogland DaanHoogland merged commit 156dbf4 into apache:4.11 Apr 16, 2018

2 checks passed

Jenkins This pull request looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@blueorangutan

This comment has been minimized.

Show comment
Hide comment
@blueorangutan

blueorangutan Apr 16, 2018

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan commented Apr 16, 2018

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

DaanHoogland added a commit that referenced this pull request Apr 16, 2018

Merge release branch 4.11 to master
* 4.11:
  agent: Add logging to libvirt qemu hook (#2554)
@blueorangutan

This comment has been minimized.

Show comment
Hide comment
@blueorangutan

blueorangutan Apr 16, 2018

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1930

blueorangutan commented Apr 16, 2018

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1930

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment