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

Bond improvements #95

Merged
merged 3 commits into from
Mar 10, 2015
Merged

Bond improvements #95

merged 3 commits into from
Mar 10, 2015

Conversation

vholer
Copy link
Contributor

@vholer vholer commented Feb 18, 2015

Following simple patches introduce 3 options for defined resource network::bond{ }.

  • hotplug which is propagated to bond interface (user usually wants to set to non-default false)
  • options which is merged with bonding options (user can use it to set additional options on bond interface, e.g. disable network manager or set bridge interface for more advanced configuration)
  • slave_options which is merged with MASTER/SLAVE options on bond slave interfaces

For example on Red Hat I can now easily disable Network Manager on all interfaces and set bridge over bond and also disable hotplug, which doesn't have sense for these interfaces:

network::bond { 'bond0':
  ensure  => present,
  onboot  => 'true',
  hotplug => 'false',
  slaves  => ['ens1f0','ens1f1'],
  mode    => 'active-backup',
  options => {
    'BRIDGE'        => 'br0',
    'NM_CONTROLLED' => 'no',
  },
  slave_options => {
    'NM_CONTROLLED' => 'no',
  },
}

@adrienthebo
Copy link
Member

Looks sane to me. @jhoblitt @ffrank @igalic ?

@jhoblitt
Copy link
Member

@adrienthebo At first glance, this looks good to me too. Someone should still test it before merging... we really need acceptance tests in-tree.

adrienthebo added a commit that referenced this pull request Mar 10, 2015
@adrienthebo adrienthebo merged commit c4ea625 into voxpupuli:master Mar 10, 2015
@igalic
Copy link
Contributor

igalic commented Mar 11, 2015

@nibalizer ping. do you have acceptance tests infra somewhere?

@nibalizer
Copy link
Member

@igalic I do. (We do.) But this module doesn't have acceptance tests.

@igalic
Copy link
Contributor

igalic commented Mar 12, 2015

oy _

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants