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

Update existing VMs in virt.running #48736

Merged
merged 6 commits into from Jul 27, 2018
Merged

Conversation

@cbosdo
Copy link
Collaborator

@cbosdo cbosdo commented Jul 24, 2018

What does this PR do?

This PR makes virt.running able to update existing VMs, running or not. If the VM is already running, the new definition will be applied for the next start and as many live updates as possible will be attempted.

A new virt.update function has been introduced in the virt execution module.

The PR also removes the use of minidom in the virt execution module for more consistency... and ElementTree helps removing some XML helper functions.

What issues does this PR fix or reference?

None

Previous Behavior

  • virt.running on a running VM did nothing
  • virt.running on a defined, but stopped VM started it

New Behavior

  • virt.running on a running VM tries hard to update it without restarting it
  • virt.running on a stopped VM changes the definition and starts it.

Tests written?

Yes

Commits signed with GPG?

Yes

@cbosdo
Copy link
Collaborator Author

@cbosdo cbosdo commented Jul 24, 2018

@rallytime @gtmanfred yet another feature (+ cleanup) PR for the virt modules that you surely want to look into.

@rallytime rallytime requested a review from gtmanfred Jul 24, 2018
Copy link
Contributor

@gtmanfred gtmanfred left a comment

@cbosdo
Copy link
Collaborator Author

@cbosdo cbosdo commented Jul 25, 2018

@gtmanfred oops... Two of the failures are indeed mine and I'll fix them, but the 3 others ones are completely unrelated to this PR.

@cbosdo
Copy link
Collaborator Author

@cbosdo cbosdo commented Jul 25, 2018

The two unit tests are now fixed, thanks for the heads up

@rallytime rallytime requested a review from gtmanfred Jul 26, 2018
@cbosdo
Copy link
Collaborator Author

@cbosdo cbosdo commented Jul 27, 2018

fixed a remaining lint error and rebased on the develop branch

cbosdo added 6 commits Jul 27, 2018
The target element is not mandatory, introduce a test that we properly
handle this situation
ElementTree provides a more convenient API than minidom. Switching to
ElementTree will reduce the number of dependencies for the virt module
and simplify the code a little.
Graphics type 'none' will help to distinguish between no graphics data
and removal of graphics device in a future virt.update function.
User need to be able to update an existing virtual machine definition.
This function changes the definition for the next start of the VM and
tries hard to live update the virtual machine.
So far virt.running does nothing if the corresponding domain is already
defined. Use the new virt.update function to change the domain
configuration.
Modules test files are named after the module itself, the state virt
module needs to comply with that rule too.
@rallytime rallytime merged commit 5afd64e into saltstack:develop Jul 27, 2018
4 of 9 checks passed
@cbosdo cbosdo deleted the virt-running branch Aug 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants