converting libvirt::network to a type, step 1

[igalic]

this skeleton is missing all the actual code for creating the XML, since
that's tough business, especially when we're not 100% sure what the
interfaces should look like ;)

[igalic]

@msimonin, here we go. I think we should start this off discussing what the interface should look like.

[msimonin]

Hi ,

There are so much options in the xml description...
I was thinking to pass directly the raw xml in the type declaration (or the path to a file containing the description), something like :

libvirt_network { 'mynetwork'
  :ensure       => present,
  :autostart    => true,
  :active       => true,
  :description  => "<network ...>...</network>
}

To be consistent with libvirt_pool type maybe we should think to add this feature aswell.
It would be easier for someone who is familiar with libvirt xml to move to puppet.

My 2 cents

[igalic]

What we have now in libvirt::network is this

define libvirt::network (
  $ensure = 'present',
  $autostart = false,
  $bridge = undef,
  $forward_mode = undef,
  $forward_dev = undef,
  $forward_interfaces = [],
  $ip = undef,
  $ipv6 = undef,
  $mac = undef,
) {

My main problem with this is the overload of $ip and $ipv6, as they are hashes and are used to define pretty much everything.

An alternative to this would be to prefix options non-unique to ipv4 or ipv6, and extend the declaration to:

libvirt_network { 'mynetwork'
  :ensure       => present,
  :autostart    => true,
  :active       => true,
  :bridge       => undef,
  :mode         => undef,
  :ipv4_address      => undef,
  :ipv4_netmask      => undef,
  :ipv4_dhcp_start   => undef,
  :ipv4_dhcp_end     => undef,
  :dhcp_bootp_file   => undef,
  :dhcp_bootp_server => undef,
  :ipv6_address    => undef,
  :ipv6_netmask    => undef,
  :ipv6_prefix     => undef,
  :ipv6_dhcp_start => undef,
  :ipv6_dhcp_end   => undef,
}

[msimonin]

Yes we could split the options with the ipv* prefix. My concern is how concise we will be ?
How can we define this ?

<network>
  <name>ovs-net</name>
  <forward mode='bridge'/>
  <bridge name='ovsbr0'/>
  <virtualport type='openvswitch'>
    <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/>
  </virtualport>
  <vlan trunk='yes'>
    <tag id='42' nativeMode='untagged'/>
    <tag id='47'/>
  </vlan>
  <portgroup name='dontpanic'>
    <vlan>
      <tag id='42'/>
    </vlan>
  </portgroup>
</network>

my suggestion is to have something like :

libvirt_network { 'mynetwork'
  :ensure       => present,
  :autostart    => true,
  :active       => true,
  :source     =>  "puppet:///modules/libvirt/networks/mynetwork.xml"
}

M

[igalic]

ugh.. since my OSes don't support OpenSwitcch yet, I haven't even dared looking into that, but I see where you're coming from.
My issue is that when we supply an external XML, we'd have to parse (validate), normalize and compare that to the current version every run… Whereas when we generate it ourselves, we just have to generate and compare it… okay, I'm not sure where I'm going with this …

[Xylakant]

This is probably not what you want for the reasons explained in #32 which suffers from the same code. Instead use REXML::Document.new to create a REXML document.

[igalic]

i've done loads of work on puppet types & providers recently, so i might come back to this one again in the next week

[msimonin]

great :)

[jaggededgedjustice]

I've started looking at converting the network to a type recently.
Regarding the interface, I don't think asking the user to provide their own xml is the right thing to do. I think that if a user has to provide their own xml they might as well register the network themselves as well. And I worry that having the individual ip_* parameters will lead to too many parameters to be easy to use.
I don't think the current hashes are too bad, but another option would be config fragments like the way concat works. For example

libvirt::network{ 'my_net':
  bridge => vmbr0
}
libvirt::network::ip{ 'my_ip':
  network => 'my_net'
  ...
}

This is probably a good set up for the portgroup section, I'm not sure about sections like ip.

[igalic]

"next week" how time flies :(((

i absolutely agree, @jaggededgedjustice!