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: default save true in setup host networks #49983

Merged
merged 6 commits into from
Mar 13, 2019
Merged

Conversation

erav
Copy link
Contributor

@erav erav commented Dec 16, 2018

Default value for save updated network configuration
during setup host networks is set to true.

SUMMARY
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ovirt_host_network

ADDITIONAL INFORMATION

needs update of the api-model and regeneration of ovirt-sdk4


@ansibot
Copy link
Contributor

ansibot commented Dec 16, 2018

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 cloud community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. virt Virt community (incl. QEMU, KVM, libvirt, ovirt, RHV and Proxmox) support:community This issue/PR relates to code supported by the Ansible community. labels Dec 16, 2018
@ansibot
Copy link
Contributor

ansibot commented Dec 16, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/ovirt/ovirt_host_network.py:0:0: E324 Value for "default" from the argument_spec (True) for "save" does not match the documentation (False)

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 community_review In order to be merged, this PR must follow the community review workflow. labels Dec 16, 2018
@ansibot
Copy link
Contributor

ansibot commented Dec 16, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/ovirt/ovirt_host_network.py:0:0: E324 Value for "default" from the argument_spec (True) for "save" does not match the documentation (False)

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Dec 16, 2018
@erav erav closed this Dec 16, 2018
@erav erav reopened this Dec 16, 2018
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Dec 16, 2018
@ansibot
Copy link
Contributor

ansibot commented Dec 16, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/ovirt/ovirt_host_network.py:0:0: E324 Value for "default" from the argument_spec (True) for "save" does not match the documentation (False)

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Dec 16, 2018
@dominikholler
Copy link
Contributor

Please check the "temporarily" in the doc section.
What happens, if this is module is executed against an old version of the ovirt-sdk, which does not expect the 'save' parameter?

@machacekondra
Copy link
Contributor

Dominik is right, can you please just change the default value of the save parameter and change it's documentation. The save is implemneted in _action_save_configuration method. So it will work correctly but will be less effecient as it will do two API calls, but keep the compatibility with 4.2 version of the SDK/API.

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Dec 17, 2018
@erav
Copy link
Contributor Author

erav commented Dec 18, 2018

posted fix

@@ -74,7 +74,7 @@
type: bool
save:
description:
- "If I(true) network configuration will be persistent, by default they are temporarily."
- "If I(false) network configuration will be temporary until next boot, by default they are persistent."
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please write that: Since Ansible 2.8 they are persisted by default? So it's clear.

@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Dec 18, 2018
@ansibot
Copy link
Contributor

ansibot commented Dec 18, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/ovirt/ovirt_host_network.py:0:0: E324 Value for "default" from the argument_spec (True) for "save" does not match the documentation (False)

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. and removed ci_verified Changes made in this PR are causing tests to fail. labels Dec 18, 2018
@ansibot
Copy link
Contributor

ansibot commented Dec 18, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/ovirt/ovirt_host_network.py:0:0: E324 Value for "default" from the argument_spec (True) for "save" does not match the documentation (False)

click here for bot help

erav and others added 3 commits February 27, 2019 14:39
Default value for save updated network configuration
during setup host networks is set to true.
Default value for save updated network configuration
during setup host networks is set to true.
@ansibot ansibot removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Feb 27, 2019
@erav
Copy link
Contributor Author

erav commented Feb 27, 2019 via email

@machacekondra
Copy link
Contributor

shipit

2 similar comments
@mwperina
Copy link
Contributor

shipit

@machacekondra
Copy link
Contributor

shipit

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

@mwperina
Copy link
Contributor

Dominik, you have requested changed in previous review, so will probably need to approve that PR to get it merged automatically

@dominikholler

@mwperina
Copy link
Contributor

bot_status

@ansibot
Copy link
Contributor

ansibot commented Feb 28, 2019

Components

lib/ansible/modules/cloud/ovirt/ovirt_host_network.py
support: community
maintainers: machacekondra mwperina

Metadata

waiting_on: maintainer
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 2
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainers or core team members): machacekondra mwperina
shipit_actors_other: []
automerge: automerge tests passed

click here for bot help

@ansibot ansibot added automerge This PR was automatically merged by ansibot. shipit This PR is ready to be merged by Core and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 28, 2019
@ansibot
Copy link
Contributor

ansibot commented Mar 8, 2019

cc @mnecas
click here for bot help

@ansibot ansibot added automerge This PR was automatically merged by ansibot. and removed automerge This PR was automatically merged by ansibot. labels Mar 8, 2019
@machacekondra
Copy link
Contributor

shipit

1 similar comment
@mnecas
Copy link
Contributor

mnecas commented Mar 13, 2019

shipit

@machacekondra
Copy link
Contributor

bot_status

@ansibot
Copy link
Contributor

ansibot commented Mar 13, 2019

Components

lib/ansible/modules/cloud/ovirt/ovirt_host_network.py
support: community
maintainers: machacekondra mnecas mwperina

Metadata

waiting_on: maintainer
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 3
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainers or core team members): machacekondra mwperina mnecas
shipit_actors_other: []
automerge: automerge tests passed

click here for bot help

@mwperina
Copy link
Contributor

shipit

@gundalow gundalow merged commit 1b48e47 into ansible:devel Mar 13, 2019
@ansible ansible locked and limited conversation to collaborators Jul 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 automerge This PR was automatically merged by ansibot. cloud feature This issue/PR relates to a feature request. module This issue/PR relates to a module. new_contributor This PR is the first contribution by a new community member. ovirt oVirt and RHV community shipit This PR is ready to be merged by Core 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

8 participants