From 018c3f487ea4addf2925e1a3263dc2cf5527bf6c Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 5 Nov 2018 16:23:27 -0800 Subject: [PATCH] libvirt/resource_libvirt_network: Support updates for dns.hosts 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]. [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]: https://github.com/dmacvicar/terraform-provider-libvirt/pull/469#discussion_r231166011 --- libvirt/network.go | 138 +++++++++++++++++++++++ libvirt/resource_libvirt_network.go | 9 +- libvirt/resource_libvirt_network_test.go | 70 ++++++++++++ 3 files changed, 214 insertions(+), 3 deletions(-) create mode 100644 libvirt/network.go diff --git a/libvirt/network.go b/libvirt/network.go new file mode 100644 index 000000000..7f5952a5e --- /dev/null +++ b/libvirt/network.go @@ -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 +} diff --git a/libvirt/resource_libvirt_network.go b/libvirt/resource_libvirt_network.go index 5c55975c7..e88ec8a3f 100644 --- a/libvirt/resource_libvirt_network.go +++ b/libvirt/resource_libvirt_network.go @@ -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": { @@ -179,7 +178,6 @@ 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, @@ -187,7 +185,6 @@ 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, }, }, }, @@ -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 } diff --git a/libvirt/resource_libvirt_network_test.go b/libvirt/resource_libvirt_network_test.go index b8d517b02..616b30307 100644 --- a/libvirt/resource_libvirt_network_test.go +++ b/libvirt/resource_libvirt_network_test.go @@ -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"}, + }, + }, + }), + ), + }, }, }) }