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

ovirt_vm: Check next_run configuration update if exist #47282

Merged
merged 1 commit into from Oct 23, 2018

Conversation

machacekondra
Copy link
Contributor

Backport of #47280

This PR fixes the update check method so it now check also the next_run
configuration of the virtual machine if it exists.

So if previously the VM was updated with new parameters, and then reset
back, the module didn't set the parameters to be set back in next_run.
This PR fixes it so the next run configuration is set back with proper
parameters.

Signed-off-by: Ondra Machacek omachace@redhat.com
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1639894
Signed-off-by: Ondra Machacek omachace@redhat.com

SUMMARY
ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
  • Feature Pull Request
  • New Module Pull Request
COMPONENT NAME
ANSIBLE VERSION

ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

ansibot commented Oct 18, 2018

Hi @machacekondra, thank you for submitting this pull-request!

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Oct 18, 2018

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 backport This PR does not target the devel branch. bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. virt Virt community (incl. QEMU, KVM, libvirt, ovirt, RHV and Proxmox) owner_pr This PR is made by the module's maintainer. support:community This issue/PR relates to code supported by the Ansible community. labels Oct 18, 2018
Copy link
Contributor

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

shipit

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. needs_triage Needs a first human triage before being processed. labels Oct 18, 2018
@mwperina
Copy link
Contributor

Ryan, could you please merge? This is a customer related bug, so we would really need to get this into Ansible 2.7.1. Thanks!

@ryansb

@abadger
Copy link
Contributor

abadger commented Oct 22, 2018

Could you add a changelog entry for this? It will be ready to merge once that is added.

@abadger
Copy link
Contributor

abadger commented Oct 22, 2018

@mwperina @machacekondra Also... today is the last day for merging for the 2.7.1 release. So if you can get it in ASAP, that would be extremely helpful for getting it into 2.7.1 as opposed to 2.7.2.

@machacekondra
Copy link
Contributor Author

@abadger Thanks for review. I've added the changelog.

Copy link
Contributor

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

shipit

@@ -0,0 +1,2 @@
bugfixes:
- ovirt_vm: Check next_run configuration update if exist (https://github.com/ansible/ansible/pull/47282/).
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem in the changelog is the ": ". In yaml, that has a special meaning, the separator between a dictionary's key and value elements.

The easiest fix is to change the ": " into a " - ".

@ansibot
Copy link
Contributor

ansibot commented Oct 23, 2018

The test ansible-test sanity --test changelog [explain] failed with the error:

Command "/usr/bin/python test/sanity/code-smell/changelog.py" returned exit status 1.
>>> Standard Error
Traceback (most recent call last):
  File "packaging/release/changelogs/changelog.py", line 814, in <module>
    main()
  File "packaging/release/changelogs/changelog.py", line 98, in main
    args.func(args)
  File "packaging/release/changelogs/changelog.py", line 109, in command_lint
    lint_fragments(fragments, exceptions)
  File "packaging/release/changelogs/changelog.py", line 227, in lint_fragments
    errors += linter.lint(fragment)
  File "packaging/release/changelogs/changelog.py", line 307, in lint
    errors += [(fragment.path, 0, 0, result[1]) for result in results]
  File "packaging/release/changelogs/changelog.py", line 307, in <listcomp>
    errors += [(fragment.path, 0, 0, result[1]) for result in results]
  File "/usr/local/lib/python3.6/dist-packages/rstcheck.py", line 169, in check
    find_ignored_languages(source)
  File "/usr/local/lib/python3.6/dist-packages/rstcheck.py", line 235, in find_ignored_languages
    for (index, line) in enumerate(source.splitlines()):
AttributeError: 'dict' object has no attribute 'splitlines'
Traceback (most recent call last):
  File "test/sanity/code-smell/changelog.py", line 14, in <module>
    main()
  File "test/sanity/code-smell/changelog.py", line 10, in main
    subprocess.check_call(cmd)
  File "/usr/lib/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['packaging/release/changelogs/changelog.py', 'lint', 'changelogs/fragments/ovirt_vm_check_next_run_configuration_update_if_exist.yaml']' returned non-zero exit status 1.

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed owner_pr This PR is made by the module's maintainer. shipit This PR is ready to be merged by Core labels Oct 23, 2018
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Oct 23, 2018
Copy link
Contributor

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

shipit

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. labels Oct 23, 2018
@abadger
Copy link
Contributor

abadger commented Oct 23, 2018

There are now conflicts in this PR (after merging other ovirt PRs) If you have a chance to resolve those that would be great. I'll take a look after I run through the backport queue at whether you've had a chance and whether it would be straightforward for me to resolve the conflict myself.

@abadger
Copy link
Contributor

abadger commented Oct 23, 2018

Okay, I'm looking at whether I can resolve the conflicts now.

@machacekondra
Copy link
Contributor Author

@abadger conflicts resolved

This PR fixes the update check method so it now check also the next_run
configuration of the virtual machine if it exists.

So if previously the VM was updated with new parameters, and then reset
back, the module didn't set the parameters to be set back in next_run.
This PR fixes it so the next run configuration is set back with proper
parameters.

Signed-off-by: Ondra Machacek <omachace@redhat.com>
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1639894
Signed-off-by: Ondra Machacek <omachace@redhat.com>
@abadger
Copy link
Contributor

abadger commented Oct 23, 2018

@machacekondra Thanks. github was still telling me there's conflicts, likely because of the merge (as opposed to rebase). But I can see how to fix that thanks to your work. I've pushed up a resolution to that. Please review to be sure I haven't done anything wrong.

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed shipit This PR is ready to be merged by Core labels Oct 23, 2018
@machacekondra
Copy link
Contributor Author

@abadger Change looks OK, thanks you!

@abadger abadger merged commit a9128b9 into ansible:stable-2.7 Oct 23, 2018
@abadger
Copy link
Contributor

abadger commented Oct 23, 2018

Merged for the 2.7.1 release.

@dagwieers dagwieers added ovirt oVirt and RHV community and removed virt Virt community (incl. QEMU, KVM, libvirt, ovirt, RHV and Proxmox) labels Feb 21, 2019
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 backport This PR does not target the devel branch. bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. ovirt oVirt and RHV community support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants