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

Add bond options field. #38512

Merged
merged 1 commit into from
Apr 12, 2018
Merged

Add bond options field. #38512

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
79 changes: 50 additions & 29 deletions lib/ansible/modules/cloud/ovirt/ovirt_host_networks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/python
Copy link

Choose a reason for hiding this comment

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

in your commit message, explain why this change is useful (lets users control fine details of Linux bonding)

Copy link
Contributor

Choose a reason for hiding this comment

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

And add some useful examples.

# -*- coding: utf-8 -*-
#
# Copyright (c) 2016 Red Hat, Inc.
# Copyright (c) 2016, 2018 Red Hat, Inc.
#
# This file is part of Ansible
#
Expand Down Expand Up @@ -47,6 +47,7 @@
- "Dictionary describing network bond:"
- "C(name) - Bond name."
- "C(mode) - Bonding mode."
- "C(options) - Bonding options."
- "C(interfaces) - List of interfaces to create a bond."
interface:
description:
Expand Down Expand Up @@ -96,6 +97,19 @@
gateway: 1.2.3.4
version: v4

# Create bond on eth1 and eth2 interface, specifiyng both mode and miimon:
- name: Bonds
ovirt_host_networks:
name: myhost
bond:
name: bond0
mode: 1
options:
miimon: 200
interfaces:
- eth1
- eth2

# Remove bond0 bond from host interfaces:
- ovirt_host_networks:
state: absent
Expand Down Expand Up @@ -146,6 +160,7 @@
except ImportError:
pass

from ansible.module_utils import six
from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils.ovirt import (
BaseModule,
Expand All @@ -160,48 +175,54 @@
)


def get_mode_type(mode_number):
"""
Adaptive transmit load balancing (balance-tlb): mode=1 miimon=100
Dynamic link aggregation (802.3ad): mode=2 miimon=100
Load balance (balance-xor): mode=3 miimon=100
Active-Backup: mode=4 miimon=100 xmit_hash_policy=2
"""
def get_bond_options(mode, usr_opts):
MIIMON_100 = dict(miimon='100')
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to use upper case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to hint at being a constant.

DEFAULT_MODE_OPTS = {
'1': MIIMON_100,
'2': MIIMON_100,
'3': MIIMON_100,
'4': dict(xmit_hash_policy='2', **MIIMON_100)
}

options = []
if mode_number is None:
if mode is None:
return options

def get_type_name(mode_number):
"""
We need to maintain this type strings, for the __compare_options method,
for easier comparision.
"""
return [
modes = [
'Active-Backup',
'Load balance (balance-xor)',
None,
'Dynamic link aggregation (802.3ad)',
][mode_number]
]
if (not 0 < mode_number <= len(modes) - 1):
return None
return modes[mode_number - 1]

try:
mode_number = int(mode_number)
if mode_number >= 1 and mode_number <= 4:
if mode_number == 4:
options.append(otypes.Option(name='xmit_hash_policy', value='2'))

options.append(otypes.Option(name='miimon', value='100'))
options.append(
otypes.Option(
name='mode',
type=get_type_name(mode_number - 1),
value=str(mode_number)
)
)
else:
options.append(otypes.Option(name='mode', value=str(mode_number)))
mode_number = int(mode)
except ValueError:
raise Exception("Bond mode must be a number.")
raise Exception('Bond mode must be a number.')

options.append(
otypes.Option(
name='mode',
type=get_type_name(mode_number),
value=str(mode_number)
)
)

opts_dict = DEFAULT_MODE_OPTS.get(mode, {})
opts_dict.update(**usr_opts)

options.extend(
[otypes.Option(name=opt, value=value)
Copy link
Contributor

Choose a reason for hiding this comment

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

I am afraid this won't work in compare method bceause you don't pass type, but API does fill it, can you please verify idepmtency works correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We've been only passing type for "mode", and I do that still (line#214). I verified this locally.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see :)

for opt, value in six.iteritems(opts_dict)]
)
return options


Expand Down Expand Up @@ -250,7 +271,7 @@ def has_update(self, nic_service):

# Check if bond configuration should be updated:
if bond:
update = self.__compare_options(get_mode_type(bond.get('mode')), getattr(nic.bonding, 'options', []))
update = self.__compare_options(get_bond_options(bond.get('mode'), bond.get('options')), getattr(nic.bonding, 'options', []))
update = update or not equal(
sorted(bond.get('interfaces')) if bond.get('interfaces') else None,
sorted(get_link_name(self._connection, s) for s in nic.bonding.slaves)
Expand Down Expand Up @@ -369,7 +390,7 @@ def main():
otypes.HostNic(
name=bond.get('name'),
bonding=otypes.Bonding(
options=get_mode_type(bond.get('mode')),
options=get_bond_options(bond.get('mode'), bond.get('options')),
slaves=[
otypes.HostNic(name=i) for i in bond.get('interfaces', [])
],
Expand Down