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

Add 'uuid', 'on_reboot', 'on_restart', and 'on_crash' settings to the… #48214

Merged
merged 5 commits into from Jun 25, 2018

Conversation

Projects
None yet
3 participants
@pm17788
Copy link
Contributor

commented Jun 19, 2018

What does this PR do?

Adds 'uuid', 'on_reboot', 'on_restart', and 'on_crash' settings to the virt.vm_info and virt.full_info outputs

What issues does this PR fix or reference?

The need to parse XML to get this data.

New Behavior

Both virt.vm_info and virt.full_info outputs now include the parsed values (or Null) for uuid, on_reboot, on_restart, and on_crash. The highest utility is in mass parsing of output of things like
salt --out=json '*' virt.full_info

Tests written?

No

Commits signed with GPG?

No

Pavel May
@pm17788

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2018

The use case we have for this is matching, with a high degree of confidence, the dataset we get from a salt minion which runs on a guest and when the same VM shows up as a part of virt.vm_info or virt.full_info output produced by the salt minion running on the hypervisor.
Right now, I'm forced to use a hacky "let's match the MAC address of the NIC(s) seen by the guest minion to a MAC address reported as part of virt.vm_info", and I already have collisions (unsurprisingly).

If uuid was available, it'd be matched to the output of dmidecode --string system-uuid (just have to .lower() the string), and the match would be a lot more deterministic.

@pm17788

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2018

Re: Failing checks - sorry, not sure how to fix any of them. None of the failures seemed actual failures. I tried to follow the pattern already established by functions such as _get_nics(dom).

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2018

Hi @pm17788 - Apologies for not commenting on this sooner. It got lost in my tabs. :)

The original test run actually hit a race-condition we have in our jenkins server, that's why there weren't any test results there.

However, the lint failures will need to be fixed up for sure: https://jenkins.saltstack.com/job/PR/job/salt-pr-lint-n/22808/violations/file/salt/modules/virt.py/

Can you grab those?

Also, welcome! Thanks for contributing. :)

@pm17788

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2018

@rallytime: Thank you for catching up with this.

The linter and I are now in the battle of wits. :)

@rallytime

This comment has been minimized.

@pm17788

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2018

@rallytime : Incoming!

@pm17788

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2018

@rallytime: I think I got it this time. So many checks seem to come back angry-red, though, sadly, I'm at a loss how to fix 'em.

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

@pm17788 Thanks! Yeah, those other test failures are not related to this PR.

@rallytime rallytime requested a review from gtmanfred Jun 22, 2018

@pm17788

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2018

@rallytime : Huzzah! :)

@rallytime rallytime merged commit 3d5739f into saltstack:develop Jun 25, 2018

4 of 10 checks passed

jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #10929 — ABORTED
Details
codeclimate 5 issues to fix
Details
default Build finished.
Details
jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #5959 — FAILURE
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #23887 — FAILURE
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #20012 — FAILURE
Details
WIP ready for review
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #26163 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #18213 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #22848 — SUCCESS
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.