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

virt state fixes #54196

Merged
merged 6 commits into from
Dec 14, 2019
Merged

virt state fixes #54196

merged 6 commits into from
Dec 14, 2019

Conversation

cbosdo
Copy link
Contributor

@cbosdo cbosdo commented Aug 13, 2019

What does this PR do?

Fixes a number of tiny annoying issues preventing the virt.pool_running and virt.network_running states to be used.

Also fixes idempotency of virt.running state.

What issues does this PR fix or reference?

One of the commits fixes issue #53107.
I was too lazy to write an issue for each of the other issues I fixed. Hopefully the commits messages are helpful enough to explain those.

Tests written?

Yes

Commits signed with GPG?

Yes

@cbosdo
Copy link
Contributor Author

cbosdo commented Aug 13, 2019

@rallytime could you have a look at this PR? Not sure if it's still OK for 2019.2.1

@joesusecom
Copy link

Does this also fix the virt.stopped state? I've tested with two paused VMs, and the result should have been that they are stopped. But I get a result "False" with comment "No changes had happened" and Changes details that have an issue "Requested operation is not valid: domain is not running". Completely bogus and not idempotent at all. :-(

@joesusecom
Copy link

Same result if the VMs are fully stopped. This should just report green (nothing to do).

@cbosdo
Copy link
Contributor Author

cbosdo commented Sep 29, 2019

@joesusecom no that one is not fixed. I'll need to go over all the existing virt states to fix that and add support for the test parameter.

@cbosdo cbosdo requested a review from a team as a code owner October 25, 2019 10:17
@ghost ghost requested a review from DmitryKuzmenko October 25, 2019 10:17
@cbosdo cbosdo changed the base branch from 2019.2.1 to master October 25, 2019 10:18
@cbosdo
Copy link
Contributor Author

cbosdo commented Oct 25, 2019

Same result if the VMs are fully stopped. This should just report green (nothing to do).

Fixed in this PR now. The test parameter is not yet handled though.
I also rebased that PR against master branch.

@cbosdo
Copy link
Contributor Author

cbosdo commented Oct 29, 2019

Strange, there are a number of failing tests that aren't related to the PR.

virt.network_running state calls virt.network_define with vport as a
positional argument resulting in an error at runtime. Fix the state to
use the vport named argument instead.
virt.pool_running needs the source to be a dictionary, which the
documentation was not reflecting. Along the same lines the source hosts
need to be a list, adjust the example to show it.
Commit 25b9681 is wrong in assuming the pool build also starts it. The
pool needs to be stopped before building it, but we still need to start
it after the build: libvirt won't do it automagically for us.
virt.network_infos and virt.pool_infos return the infos as a dictionary
with the network or pool name as a key even when there is only one
value. Adapt the network_running and pool_running states to this.
vm_state return a dictionary with the VM name as a key. Fix virt.running
state and its tests to match this. See issue saltstack#53107.
The virt.stopped and virt.powered_off states need to do nothing and
not fail if one of the targeted VMs is already in shutdown state.
@dwoz dwoz merged commit 882a832 into saltstack:master Dec 14, 2019
@cbosdo cbosdo deleted the 2019.2.1-virt-state-fixes branch December 16, 2019 09:30
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

Successfully merging this pull request may close these issues.

5 participants