Skip to content

Commit

Permalink
libvirt/resource_libvirt_network: Support updates for dns.hosts
Browse files Browse the repository at this point in the history
Recreating the nework detaches associated interfaces [1]:

  Sometimes, one needs to edit the network definition and apply the
  changes on the fly.  The most common scenario for this is adding new
  static MAC+IP mappings for the network's DHCP server.  If you edit
  the network with "virsh net-edit", any changes you make won't take
  effect until the network is destroyed and re-started, which
  unfortunately will cause a all guests to lose network connectivity
  with the host until their network interfaces are explicitly
  re-attached.

With this commit, we update the network on dns.hosts changes instead
of forcing a new network.  This allows DNS host changes without
clearing domain network interfaces.

I'm removing stale entries by IP alone, because libvirt allows
multiple IP addresses to share a hostname [2,3].  I have a patch in
flight that may update that logic to make IP matching explicitly the
only consideration [4].

The lowercase error messages break with the existing pattern for at
least some of this package, but they comply with [5]:

  Error strings should not be capitalized (unless beginning with
  proper nouns or acronyms) or end with punctuation, since they are
  usually printed following other context.

Putting the new helpers in their own network.go is Dario's suggestion
[6].

Update unit tests are documented in [7].

[1]: https://wiki.libvirt.org/page/Networking#Applying_modifications_to_the_network
[2]: https://libvirt.org/formatnetwork.html#elementsAddress
[3]: https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/conf/network_conf.c;h=39a13b433dbbf91ccb080036df754fdeb3adc905;hb=7a10a6a598ab4e8599c867060a7232a06c663e51#l3336
[4]: https://www.redhat.com/archives/libvir-list/2018-November/msg00231.html
[5]: https://github.com/golang/go/wiki/CodeReviewComments#error-strings
[6]: #469 (comment)
[7]: https://www.terraform.io/docs/extend/best-practices/testing.html#update-test-verify-configuration-changes
  • Loading branch information
wking committed Nov 6, 2018
1 parent afd5c1a commit ef44231
Show file tree
Hide file tree
Showing 3 changed files with 214 additions and 3 deletions.
138 changes: 138 additions & 0 deletions libvirt/network.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package libvirt

import (
"errors"
"fmt"
"reflect"
"sort"

"github.com/hashicorp/terraform/helper/schema"
libvirt "github.com/libvirt/libvirt-go"
"github.com/libvirt/libvirt-go-xml"
)

func resourceLibvirtNetworkUpdateDNSHosts(d *schema.ResourceData, network *libvirt.Network) error {
hostsKey := dnsPrefix + ".hosts"
if d.HasChange(hostsKey) {
oldInterface, newInterface := d.GetChange(hostsKey)

oldEntries, err := parseNetworkDNSHostsChange(oldInterface)
if err != nil {
return fmt.Errorf("parse old %s: %s", hostsKey, err)
}

newEntries, err := parseNetworkDNSHostsChange(newInterface)
if err != nil {
return fmt.Errorf("parse new %s: %s", hostsKey, err)
}

for _, oldEntry := range oldEntries {
found := false
for _, newEntry := range newEntries {
if reflect.DeepEqual(newEntry, oldEntry) {
found = true
break
}
}
if found {
continue
}

data, err := xmlMarshallIndented(libvirtxml.NetworkDNSHost{IP: oldEntry.IP})
if err != nil {
return fmt.Errorf("serialize update: %s", err)
}

err = network.Update(libvirt.NETWORK_UPDATE_COMMAND_DELETE, libvirt.NETWORK_SECTION_DNS_HOST, -1, data, libvirt.NETWORK_UPDATE_AFFECT_LIVE|libvirt.NETWORK_UPDATE_AFFECT_CONFIG)
if err != nil {
return fmt.Errorf("delete %s: %s", oldEntry.IP, err)
}
}

for _, newEntry := range newEntries {
found := false
for _, oldEntry := range oldEntries {
if reflect.DeepEqual(oldEntry, newEntry) {
found = true
break
}
}
if found {
continue
}

data, err := xmlMarshallIndented(newEntry)
if err != nil {
return fmt.Errorf("serialize update: %s", err)
}

err = network.Update(libvirt.NETWORK_UPDATE_COMMAND_ADD_LAST, libvirt.NETWORK_SECTION_DNS_HOST, -1, data, libvirt.NETWORK_UPDATE_AFFECT_LIVE|libvirt.NETWORK_UPDATE_AFFECT_CONFIG)
if err != nil {
return fmt.Errorf("add %v: %s", newEntry, err)
}
}

d.SetPartial(hostsKey)
}

return nil
}

func parseNetworkDNSHostsChange(change interface{}) (entries []libvirtxml.NetworkDNSHost, err error) {
slice, ok := change.([]interface{})
if !ok {
return entries, errors.New("not slice")
}

mapEntries := map[string][]string{}
for i, entryInterface := range slice {
entryMap, ok := entryInterface.(map[string]interface{})
if !ok {
return nil, fmt.Errorf("entry %d is not a map", i)
}

ipInterface, ok := entryMap["ip"]
if !ok {
return nil, fmt.Errorf("entry %d.ip is missing", i)
}

ip, ok := ipInterface.(string)
if !ok {
return nil, fmt.Errorf("entry %d.ip is not a string", i)
}

hostnameInterface, ok := entryMap["hostname"]
if !ok {
return nil, fmt.Errorf("entry %d.hostname is missing", i)
}

hostname, ok := hostnameInterface.(string)
if !ok {
return nil, fmt.Errorf("entry %d.hostname is not a string", i)
}

_, ok = mapEntries[ip]
if ok {
mapEntries[ip] = append(mapEntries[ip], hostname)
} else {
mapEntries[ip] = []string{hostname}
}
}

entries = make([]libvirtxml.NetworkDNSHost, 0, len(mapEntries))
for ip, hostnames := range mapEntries {
sort.Strings(hostnames)
xmlHostnames := make([]libvirtxml.NetworkDNSHostHostname, 0, len(hostnames))
for _, hostname := range hostnames {
xmlHostnames = append(xmlHostnames, libvirtxml.NetworkDNSHostHostname{
Hostname: hostname,
})
}
entries = append(entries, libvirtxml.NetworkDNSHost{
IP: ip,
Hostnames: xmlHostnames,
})
}

return entries, nil
}
9 changes: 6 additions & 3 deletions libvirt/resource_libvirt_network.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ func resourceLibvirtNetwork() *schema.Resource {
"hosts": {
Type: schema.TypeList,
Optional: true,
ForceNew: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"ip": {
Expand All @@ -179,15 +178,13 @@ func resourceLibvirtNetwork() *schema.Resource {
// and therefore doesn't recognize that this is set when assigning from
// a rendered dns_host template.
Optional: true,
ForceNew: true,
},
"hostname": {
Type: schema.TypeString,
// This should be required, but Terraform does validation too early
// and therefore doesn't recognize that this is set when assigning from
// a rendered dns_host template.
Optional: true,
ForceNew: true,
},
},
},
Expand Down Expand Up @@ -266,6 +263,12 @@ func resourceLibvirtNetworkUpdate(d *schema.ResourceData, meta interface{}) erro
}
d.SetPartial("autostart")
}

err = resourceLibvirtNetworkUpdateDNSHosts(d, network)
if err != nil {
return fmt.Errorf("update DNS hosts: %s", err)
}

d.Partial(false)
return nil
}
Expand Down
70 changes: 70 additions & 0 deletions libvirt/resource_libvirt_network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,76 @@ func TestAccLibvirtNetwork_DNSHosts(t *testing.T) {
}),
),
},
{
Config: fmt.Sprintf(`
resource "libvirt_network" "%s" {
name = "%s"
domain = "k8s.local"
addresses = ["10.17.3.0/24"]
dns {
hosts = [
{
hostname = "myhost1",
ip = "1.1.1.1",
},
]
}
}`, randomNetworkResource, randomNetworkName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("libvirt_network."+randomNetworkResource, "dns.0.hosts.0.hostname", "myhost1"),
resource.TestCheckResourceAttr("libvirt_network."+randomNetworkResource, "dns.0.hosts.0.ip", "1.1.1.1"),
checkDNSHosts("libvirt_network."+randomNetworkResource, []libvirtxml.NetworkDNSHost{
{
IP: "1.1.1.1",
Hostnames: []libvirtxml.NetworkDNSHostHostname{
{Hostname: "myhost1"},
},
},
}),
),
},
{
Config: fmt.Sprintf(`
resource "libvirt_network" "%s" {
name = "%s"
domain = "k8s.local"
addresses = ["10.17.3.0/24"]
dns {
hosts = [
{
hostname = "myhost1",
ip = "1.1.1.1",
},
# Without https:#www.redhat.com/archives/libvir-list/2018-November/msg00231.html, this raises:
#
# update DNS hosts: add {{ } 1.1.1.2 [{myhost1}]}: virError(Code=55, Domain=19, Message='Requested operation is not valid: there is already at least one DNS HOST record with a matching field in network fo64d9y6w9')
# {
# hostname = "myhost1",
# ip = "1.1.1.2",
# },
{
hostname = "myhost2",
ip = "1.1.1.1",
},
]
}
}`, randomNetworkResource, randomNetworkName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("libvirt_network."+randomNetworkResource, "dns.0.hosts.0.hostname", "myhost1"),
resource.TestCheckResourceAttr("libvirt_network."+randomNetworkResource, "dns.0.hosts.0.ip", "1.1.1.1"),
resource.TestCheckResourceAttr("libvirt_network."+randomNetworkResource, "dns.0.hosts.1.hostname", "myhost2"),
resource.TestCheckResourceAttr("libvirt_network."+randomNetworkResource, "dns.0.hosts.1.ip", "1.1.1.1"),
checkDNSHosts("libvirt_network."+randomNetworkResource, []libvirtxml.NetworkDNSHost{
{
IP: "1.1.1.1",
Hostnames: []libvirtxml.NetworkDNSHostHostname{
{Hostname: "myhost1"},
{Hostname: "myhost2"},
},
},
}),
),
},
},
})
}
Expand Down

0 comments on commit ef44231

Please sign in to comment.