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

enable dhcp as optional parameter (typelist) #385

Merged
merged 7 commits into from
Sep 3, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 92 additions & 54 deletions libvirt/resource_libvirt_network.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,22 @@ func resourceLibvirtNetwork() *schema.Resource {
},
},
},
"dhcp": {
Type: schema.TypeList,
Optional: true,
ForceNew: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"enabled": {
Type: schema.TypeBool,
Default: true,
Optional: true,
Required: false,
},
},
},
},
},
}
}
Expand Down Expand Up @@ -131,28 +147,28 @@ func resourceLibvirtNetworkUpdate(d *schema.ResourceData, meta interface{}) erro
}
network, err := virConn.LookupNetworkByUUIDString(d.Id())
if err != nil {
return err
return fmt.Errorf("Error by retrieving network ID during update: %s", err)
Copy link
Owner

Choose a reason for hiding this comment

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

Here you are not retrieving a network ID, but looking a network by its ID. I would suggest:

fmt.Errorf("Can't retrieve network with UUID '%s' during update: %s", uuid, err)

}
defer network.Free()

d.Partial(true)

active, err := network.IsActive()
if err != nil {
return err
return fmt.Errorf("Error by getting network's status during update: %s", err)
}

if !active {
log.Printf("[DEBUG] Activating network")
if err := network.Create(); err != nil {
return err
return fmt.Errorf("Error by activating network during update: %s", err)
}
}

if d.HasChange("autostart") {
err = network.SetAutostart(d.Get("autostart").(bool))
if err != nil {
return fmt.Errorf("Error setting autostart for network: %s", err)
return fmt.Errorf("Error updating autostart for network: %s", err)
}
d.SetPartial("autostart")
}
Expand Down Expand Up @@ -196,57 +212,11 @@ func resourceLibvirtNetworkCreate(d *schema.ResourceData, meta interface{}) erro
// there is no NAT when using a routed network
networkDef.Forward.NAT = nil
}

// some network modes require a DHCP/DNS server
// set the addresses for DHCP
if addresses, ok := d.GetOk("addresses"); ok {
ipsPtrsLst := []libvirtxml.NetworkIP{}
for _, addressI := range addresses.([]interface{}) {
address := addressI.(string)
_, ipNet, err := net.ParseCIDR(address)
if err != nil {
return fmt.Errorf("Error parsing addresses definition '%s': %s", address, err)
}
ones, bits := ipNet.Mask.Size()
family := "ipv4"
if bits == (net.IPv6len * 8) {
family = "ipv6"
}
ipsRange := 2 ^ bits - 2 ^ ones
if ipsRange < 4 {
return fmt.Errorf("Netmask seems to be too strict: only %d IPs available (%s)", ipsRange-3, family)
}

// we should calculate the range served by DHCP. For example, for
// 192.168.121.0/24 we will serve 192.168.121.2 - 192.168.121.254
start, end := networkRange(ipNet)

// skip the .0, (for the network),
start[len(start)-1]++

// assign the .1 to the host interface
dni := libvirtxml.NetworkIP{
Address: start.String(),
Prefix: uint(ones),
Family: family,
}

start[len(start)-1]++ // then skip the .1
end[len(end)-1]-- // and skip the .255 (for broadcast)

dni.DHCP = &libvirtxml.NetworkDHCP{
Ranges: []libvirtxml.NetworkDHCPRange{
{
Start: start.String(),
End: end.String(),
},
},
}
ipsPtrsLst = append(ipsPtrsLst, dni)
}
networkDef.IPs = ipsPtrsLst
// if addresses are given set dhcp for these
err := setDhcpByCIDRAdressesSubnets(d, &networkDef)
if err != nil {
return fmt.Errorf("Could not set DHCP from adresses '%s'", err)
}

if dnsForwardCount, ok := d.GetOk("dns_forwarder.#"); ok {
dns := libvirtxml.NetworkDNS{
Forwarders: []libvirtxml.NetworkDNSForwarder{},
Expand Down Expand Up @@ -470,3 +440,71 @@ func waitForNetworkDestroyed(virConn *libvirt.Connect, uuid string) resource.Sta
return virConn, "ACTIVE", err
}
}

func setDhcpByCIDRAdressesSubnets(d *schema.ResourceData, networkDef *libvirtxml.Network) error {
if addresses, ok := d.GetOk("addresses"); ok {
ipsPtrsLst := []libvirtxml.NetworkIP{}
for _, addressI := range addresses.([]interface{}) {
// get the IP address entry for this subnet (with a guessed DHCP range)
dni, dhcp, err := setNetworkIP(addressI.(string))
if err != nil {
return err
}
if d.Get("dhcp.0.enabled").(bool) {
dni.DHCP = dhcp
} else {
// if a network exist with enabled but an user want to disable
// dhcp, we need to set dhcp struct to nil.
dni.DHCP = nil
}

ipsPtrsLst = append(ipsPtrsLst, *dni)
}
networkDef.IPs = ipsPtrsLst
}
return nil
}

func setNetworkIP(address string) (*libvirtxml.NetworkIP, *libvirtxml.NetworkDHCP, error) {
_, ipNet, err := net.ParseCIDR(address)
if err != nil {
return nil, nil, fmt.Errorf("Error parsing addresses definition '%s': %s", address, err)
}
ones, bits := ipNet.Mask.Size()
family := "ipv4"
if bits == (net.IPv6len * 8) {
family = "ipv6"
}
ipsRange := 2 ^ bits - 2 ^ ones
if ipsRange < 4 {
return nil, nil, fmt.Errorf("Netmask seems to be too strict: only %d IPs available (%s)", ipsRange-3, family)
}

// we should calculate the range served by DHCP. For example, for
// 192.168.121.0/24 we will serve 192.168.121.2 - 192.168.121.254
start, end := networkRange(ipNet)

// skip the .0, (for the network),
start[len(start)-1]++

// assign the .1 to the host interface
dni := &libvirtxml.NetworkIP{
Address: start.String(),
Prefix: uint(ones),
Family: family,
}

start[len(start)-1]++ // then skip the .1
end[len(end)-1]-- // and skip the .255 (for broadcast)

dhcp := &libvirtxml.NetworkDHCP{
Ranges: []libvirtxml.NetworkDHCPRange{
{
Start: start.String(),
End: end.String(),
},
},
}

return dni, dhcp, nil
}
174 changes: 135 additions & 39 deletions libvirt/resource_libvirt_network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,45 +9,6 @@ import (
"github.com/libvirt/libvirt-go"
)

func TestNetworkAutostart(t *testing.T) {
var network libvirt.Network
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckLibvirtNetworkDestroy,
Steps: []resource.TestStep{
{
Config: fmt.Sprintf(`
resource "libvirt_network" "test_net" {
name = "networktest"
mode = "nat"
domain = "k8s.local"
addresses = ["10.17.3.0/24"]
autostart = true
}`),
Check: resource.ComposeTestCheckFunc(
networkExists("libvirt_network.test_net", &network),
resource.TestCheckResourceAttr("libvirt_network.test_net", "autostart", "true"),
),
},
{
Config: fmt.Sprintf(`
resource "libvirt_network" "test_net" {
name = "networktest"
mode = "nat"
domain = "k8s.local"
addresses = ["10.17.3.0/24"]
autostart = false
}`),
Check: resource.ComposeTestCheckFunc(
networkExists("libvirt_network.test_net", &network),
resource.TestCheckResourceAttr("libvirt_network.test_net", "autostart", "false"),
),
},
},
})
}

func networkExists(n string, network *libvirt.Network) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
Expand Down Expand Up @@ -81,6 +42,48 @@ func networkExists(n string, network *libvirt.Network) resource.TestCheckFunc {
}
}

func testAccCheckLibvirtNetworkDhcpStatus(name string, network *libvirt.Network, DhcpStatus string) resource.TestCheckFunc {
Copy link
Owner

Choose a reason for hiding this comment

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

Why is DhcpStatus camel-cased? (and wondering... why go format or vet do not complain?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Speaking of this... In the comments, "dhcp" should be "DHCP" 😃

return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[name]
if !ok {
return fmt.Errorf("Not found: %s", name)
}

if rs.Primary.ID == "" {
return fmt.Errorf("No libvirt network ID is set")
}

virConn := testAccProvider.Meta().(*Client).libvirt

network, err := virConn.LookupNetworkByUUIDString(rs.Primary.ID)
if err != nil {
return err
}

networkDef, err := newDefNetworkfromLibvirt(network)
if err != nil {
return fmt.Errorf("Error reading libvirt network XML description: %s", err)
}
if DhcpStatus == "disabled" {
for _, ips := range networkDef.IPs {
// &libvirtxml.NetworkDHCP{..} should be nil when dhcp is disabled
if ips.DHCP != nil {
fmt.Printf("%#v", ips.DHCP)
return fmt.Errorf("the network should have DHCP disabled")
}
}
}
if DhcpStatus == "enabled" {
for _, ips := range networkDef.IPs {
if ips.DHCP == nil {
return fmt.Errorf("the network should have DHCP enabled")
}
}
}
return nil
}
}

func testAccCheckLibvirtNetworkDestroy(s *terraform.State) error {
virtConn := testAccProvider.Meta().(*Client).libvirt

Expand Down Expand Up @@ -130,3 +133,96 @@ func TestAccLibvirtNetwork_Import(t *testing.T) {
},
})
}

func TestAccLibvirtNetwork_EnableDhcpAndDisableItAfter(t *testing.T) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

rename this

Copy link
Contributor

Choose a reason for hiding this comment

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

To whom are you speaking to? 😸

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i just reviewed my code myself 🚀 🤖 was a sidenote 🤣

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't get angry with your reviewer 😋 . Enjoy your week-end too 🎆 .

var network1 libvirt.Network
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckLibvirtNetworkDestroy,
Steps: []resource.TestStep{
{
// dhcp is enabled true by default.
Config: fmt.Sprintf(`
resource "libvirt_network" "test_net" {
name = "networktest"
mode = "nat"
domain = "k8s.local"
addresses = ["10.17.3.0/24"]
dhcp {
enabled = true
}
}`),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("libvirt_network.test_net", "dhcp.0.enabled", "false"),
testAccCheckLibvirtNetworkDhcpStatus("libvirt_network.test_net", &network1, "enabled"),
),
},
},
})
}

func TestAccLibvirtNetwork_dhcpDisabled(t *testing.T) {
var network1 libvirt.Network
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckLibvirtNetworkDestroy,
Steps: []resource.TestStep{
{
Config: fmt.Sprintf(`
resource "libvirt_network" "test_net" {
name = "networktest"
mode = "nat"
domain = "k8s.local"
addresses = ["10.17.3.0/24"]
dhcp {
enabled = false
}
}`),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("libvirt_network.test_net", "dhcp.0.enabled", "false"),
testAccCheckLibvirtNetworkDhcpStatus("libvirt_network.test_net", &network1, "disabled"),
),
},
},
})
}
func TestAccLibvirtNetwork_Autostart(t *testing.T) {
var network libvirt.Network
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckLibvirtNetworkDestroy,
Steps: []resource.TestStep{
{
Config: fmt.Sprintf(`
resource "libvirt_network" "test_net" {
name = "networktest"
mode = "nat"
domain = "k8s.local"
addresses = ["10.17.3.0/24"]
autostart = true
}`),
Check: resource.ComposeTestCheckFunc(
networkExists("libvirt_network.test_net", &network),
resource.TestCheckResourceAttr("libvirt_network.test_net", "autostart", "true"),
),
},
{
Config: fmt.Sprintf(`
resource "libvirt_network" "test_net" {
name = "networktest"
mode = "nat"
domain = "k8s.local"
addresses = ["10.17.3.0/24"]
autostart = false
}`),
Check: resource.ComposeTestCheckFunc(
networkExists("libvirt_network.test_net", &network),
resource.TestCheckResourceAttr("libvirt_network.test_net", "autostart", "false"),
),
},
},
})
}
Loading