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.

Sign up for GitHub

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

Jump to bottom

Refactor code for VirtualFacts #18122

Merged
merged 1 commit into from
Nov 1, 2016
Merged

Refactor code for VirtualFacts #18122

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 5 additions & 24 deletions lib/ansible/module_utils/facts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2999,8 +2999,13 @@ def __new__(cls, *arguments, **keyword):
return super(cls, subclass).__new__(subclass, *arguments, **keyword)

def populate(self):
self.get_virtual_facts()
return self.facts

def get_virtual_facts(self):
self.facts['virtualization_type'] = ''
self.facts['virtualization_role'] = ''
Copy link
Contributor

Choose a reason for hiding this comment

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

bcoca> alikins: in most cases we don't add keys when they are not detected, not just virtualization (which we also need to change into lists instead of fixed values)
^ we either change the behaviour to always populate all keys or keep as is, we should not change just for one set of keys
it would be nice to be consistent one way or the other

Going by that, I'd suggest the base get_virtual_facts is

def get_virtual_facts(self):
    pass

But as you mention, that would change behavior. So pr as is LGTM to me, we can change the behavior later.


class LinuxVirtual(Virtual):
"""
This is a Linux-specific subclass of Virtual. It defines
Expand All @@ -3009,10 +3014,6 @@ class LinuxVirtual(Virtual):
"""
platform = 'Linux'

def populate(self):
self.get_virtual_facts()
return self.facts

# For more information, check: http://people.redhat.com/~rjones/virt-what/
def get_virtual_facts(self):
# lxc/docker
Expand Down Expand Up @@ -3184,14 +3185,6 @@ class FreeBSDVirtual(Virtual):
"""
platform = 'FreeBSD'

def populate(self):
self.get_virtual_facts()
return self.facts

def get_virtual_facts(self):
self.facts['virtualization_type'] = ''
self.facts['virtualization_role'] = ''

class DragonFlyVirtual(FreeBSDVirtual):
platform = 'DragonFly'

Expand All @@ -3203,10 +3196,6 @@ class OpenBSDVirtual(Virtual):
"""
platform = 'OpenBSD'

def populate(self):
self.get_virtual_facts()
return self.facts

def get_virtual_facts(self):
sysctl_path = self.module.get_bin_path('sysctl')

Expand Down Expand Up @@ -3251,10 +3240,6 @@ class HPUXVirtual(Virtual):
"""
platform = 'HP-UX'

def populate(self):
self.get_virtual_facts()
return self.facts

def get_virtual_facts(self):
if os.path.exists('/usr/sbin/vecheck'):
rc, out, err = self.module.run_command("/usr/sbin/vecheck")
Expand Down Expand Up @@ -3288,10 +3273,6 @@ class SunOSVirtual(Virtual):
"""
platform = 'SunOS'

def populate(self):
self.get_virtual_facts()
return self.facts

def get_virtual_facts(self):

# Check if it's a zone
Expand Down