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 hosts facts add cluster version filtration #48664

Merged
merged 3 commits into from
Jan 17, 2019

Conversation

mnecas
Copy link
Contributor

@mnecas mnecas commented Nov 14, 2018

SUMMARY

Now you can filter ovirt_hosts_facts with cluster version

Fixes #38871

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ovirt

ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

ansibot commented Nov 14, 2018

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

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Nov 14, 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. 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 Nov 14, 2018
)
if(module.params.get('cluster_version')):
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove () around if

@@ -83,7 +100,10 @@ def main():
hosts = hosts_service.list(
search=module.params['pattern'],
all_content=module.params['all_content'],
follow='cluster'
Copy link
Contributor

Choose a reason for hiding this comment

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

please add , at the end

filtered_hosts = []
for host in hosts:
cluster = host.cluster
if(str(cluster.version.major) + '.' + str(cluster.version.minor) == module.params.get('cluster_version')):
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove () around if

def main():
argument_spec = ovirt_facts_full_argument_spec(
pattern=dict(default='', required=False),
all_content=dict(default=False, type='bool'),
cluster_version=dict(default=None, type='str'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Add that param also to documentation

Copy link
Contributor

Choose a reason for hiding this comment

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

I would make it a list so user can pass more versions

@ansibot
Copy link
Contributor

ansibot commented Nov 14, 2018

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

lib/ansible/modules/cloud/ovirt/ovirt_host_facts.py:0:0: E322 "cluster_version" is listed in the argument_spec, but not documented in the module

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. 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. needs_triage Needs a first human triage before being processed. labels Nov 14, 2018
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 14, 2018
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Nov 22, 2018
# All hosts with cluster version 4.2:
- ovirt_host_facts:
pattern: name=host*
cluster_version:
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, better would be a string.

@@ -33,6 +33,12 @@
default: False
version_added: "2.7"
type: bool
cluster_version:
description:
- "Filters all your hosts which you got and returns only those with correct version of cluster."
Copy link
Contributor

Choose a reason for hiding this comment

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

Filter the hosts based on the cluster version.

cluster_version:
description:
- "Filters all your hosts which you got and returns only those with correct version of cluster."
type: dict
Copy link
Contributor

Choose a reason for hiding this comment

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

str

@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 17, 2019
@machacekondra
Copy link
Contributor

shipit

@ansibot ansibot added automerge This PR was automatically merged by ansibot. 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 Jan 17, 2019
@ansibot ansibot merged commit 88bb555 into ansible:devel Jan 17, 2019
knumskull pushed a commit to knumskull/ansible that referenced this pull request Jan 21, 2019
* ovirt hosts facts add cluster version filtration

* update filtering of ovirt hosts and input of cluster version

* ovirt host facts change compat version to string
@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.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. 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.

Missing cluster compatibility version as a filter in ovirt_hosts_facts
4 participants