diff --git a/lxd/network/openvswitch/ovn.go b/lxd/network/openvswitch/ovn.go index d15d50825311..eafcf22c9ff2 100644 --- a/lxd/network/openvswitch/ovn.go +++ b/lxd/network/openvswitch/ovn.go @@ -12,6 +12,7 @@ import ( "github.com/canonical/lxd/lxd/linux" "github.com/canonical/lxd/lxd/state" "github.com/canonical/lxd/shared" + "github.com/canonical/lxd/shared/dnsutil" ) // OVNRouter OVN router name. @@ -574,7 +575,7 @@ func (o *OVN) LogicalRouterPortSetIPv6Advertisements(portName OVNRouterPort, opt fmt.Sprintf("ipv6_ra_configs:send_periodic=%t", opts.SendPeriodic), } - var removeRAConfigKeys []string + var removeRAConfigKeys []string //nolint:prealloc if opts.AddressMode != "" { args = append(args, fmt.Sprintf("ipv6_ra_configs:address_mode=%s", string(opts.AddressMode))) @@ -765,7 +766,7 @@ func (o *OVN) logicalSwitchParseExcludeIPs(ips []shared.IPRange) ([]string, erro // LogicalSwitchSetIPAllocation sets the IP allocation config on the logical switch. func (o *OVN) LogicalSwitchSetIPAllocation(switchName OVNSwitch, opts *OVNIPAllocationOpts) error { - var removeOtherConfigKeys []string + var removeOtherConfigKeys []string //nolint:prealloc args := []string{"set", "logical_switch", string(switchName)} if opts.PrefixIPv4 != nil { @@ -813,7 +814,7 @@ func (o *OVN) LogicalSwitchSetIPAllocation(switchName OVNSwitch, opts *OVNIPAllo // LogicalSwitchDHCPv4RevervationsSet sets the DHCPv4 IP reservations. func (o *OVN) LogicalSwitchDHCPv4RevervationsSet(switchName OVNSwitch, reservedIPs []shared.IPRange) error { - var removeOtherConfigKeys []string + var removeOtherConfigKeys []string //nolint:prealloc args := []string{"set", "logical_switch", string(switchName)} if len(reservedIPs) > 0 { @@ -1186,7 +1187,7 @@ func (o *OVN) LogicalSwitchIPs(switchName OVNSwitch) (map[OVNSwitchPort][]net.IP for _, line := range lines { fields := shared.SplitNTrimSpace(line, ",", -1, true) portName := OVNSwitchPort(fields[0]) - var ips []net.IP + ips := make([]net.IP, 0, len(fields)) // Parse all IPs mentioned in addresses and dynamic_addresses fields. for i := 1; i < len(fields); i++ { @@ -1289,7 +1290,7 @@ func (o *OVN) LogicalSwitchPortIPs(portName OVNSwitchPort) ([]net.IP, error) { } addresses := strings.Split(strings.Replace(strings.TrimSpace(addressesRaw), ",", " ", 1), " ") - ips := make([]net.IP, 0) + ips := make([]net.IP, 0, len(addresses)) for _, address := range addresses { ip := net.ParseIP(address) @@ -1375,18 +1376,32 @@ func (o *OVN) LogicalSwitchPortSetDNS(switchName OVNSwitch, portName OVNSwitchPo fmt.Sprintf("external_ids:%s=%s", ovnExtIDLXDSwitchPort, portName), } - // Only include DNS name record if IPs supplied. + // Only generate DNS records if IPs are supplied. if len(dnsIPs) > 0 { - var dnsIPsStr strings.Builder + dnsNameLower := strings.ToLower(dnsName) + var dnsRecords strings.Builder + + // Generate A and AAAA records. + dnsRecords.WriteString(`records={"` + dnsNameLower + `"="`) for i, dnsIP := range dnsIPs { if i > 0 { - dnsIPsStr.WriteString(" ") + dnsRecords.WriteString(" ") } - dnsIPsStr.WriteString(dnsIP.String()) + dnsRecords.WriteString(dnsIP.String()) + } + + dnsRecords.WriteString(`"`) + + // Generate PTR records. + for _, dnsIP := range dnsIPs { + // Trim the "." from the end of the PTR record as OVN doesn't like it. + dnsRecords.WriteString(` "` + strings.TrimSuffix(dnsutil.Reverse(dnsIP), ".") + `"="` + dnsNameLower + `"`) } - cmdArgs = append(cmdArgs, fmt.Sprintf(`records={"%s"="%s"}`, strings.ToLower(dnsName), dnsIPsStr.String())) + dnsRecords.WriteString("}") + + cmdArgs = append(cmdArgs, dnsRecords.String()) } dnsUUID = strings.TrimSpace(dnsUUID) @@ -1434,7 +1449,7 @@ func (o *OVN) LogicalSwitchPortGetDNS(portName OVNSwitchPort) (OVNDNSUUID, strin dnsUUID := strings.TrimSpace(parts[0]) var dnsName string - var ips []net.IP + var ips []net.IP //nolint:prealloc // Try and parse the DNS name and IPs. if len(parts) > 1 { @@ -1613,22 +1628,47 @@ func (o *OVN) ChassisGroupChassisAdd(haChassisGroupName OVNChassisGroup, chassis // ChassisGroupChassisDelete deletes a chassis ID from an HA chassis group. func (o *OVN) ChassisGroupChassisDelete(haChassisGroupName OVNChassisGroup, chassisID string) error { - // Check if chassis group exists. ovn-nbctl doesn't provide an "--if-exists" option for this. - output, err := o.nbctl("--no-headings", "--data=bare", "--colum=name,ha_chassis", "find", "ha_chassis_group", fmt.Sprintf("name=%s", string(haChassisGroupName))) + // Map UUIDs with chassis_names. + output, err := o.nbctl("--format=csv", "--no-headings", "--data=bare", "--column=_uuid,chassis_name", "find", "ha_chassis") if err != nil { return err } lines := shared.SplitNTrimSpace(output, "\n", -1, true) + + uuidToChassis := make(map[string]string, len(lines)) + + for _, line := range lines { + // a74125a8-b580-4763-b389-11ce2c8c5509,node2 + key, value, match := strings.Cut(line, ",") + if match { + uuidToChassis[key] = value + } + } + + // Check if chassis group exists. ovn-nbctl doesn't provide an "--if-exists" option for this. + output, err = o.nbctl("--no-headings", "--data=bare", "--colum=name,ha_chassis", "find", "ha_chassis_group", "name="+string(haChassisGroupName)) + if err != nil { + return err + } + + lines = shared.SplitNTrimSpace(output, "\n", -1, true) if len(lines) > 1 { existingChassisGroup := lines[0] members := shared.SplitNTrimSpace(lines[1], " ", -1, true) // Remove chassis from group if exists. - if existingChassisGroup == string(haChassisGroupName) && shared.ValueInSlice(chassisID, members) { - _, err := o.nbctl("ha-chassis-group-remove-chassis", string(haChassisGroupName), chassisID) - if err != nil { - return err + if existingChassisGroup == string(haChassisGroupName) { + for _, member := range members { + name, found := uuidToChassis[member] + if found && name == chassisID { + _, err := o.nbctl("ha-chassis-group-remove-chassis", string(haChassisGroupName), chassisID) + if err != nil { + return err + } + + break + } } } } @@ -1690,7 +1730,7 @@ func (o *OVN) PortGroupAdd(projectID int64, portGroupName OVNPortGroup, associat // PortGroupDelete deletes port groups along with their ACL rules. func (o *OVN) PortGroupDelete(portGroupNames ...OVNPortGroup) error { - args := make([]string, 0) + args := make([]string, 0, 5*len(portGroupNames)) for _, portGroupName := range portGroupNames { if len(args) > 0 { @@ -1885,7 +1925,7 @@ func (o *OVN) loadBalancerUUIDs(loadBalancerName OVNLoadBalancer) ([]string, err lbTCPName := fmt.Sprintf("%s-tcp", loadBalancerName) lbUDPName := fmt.Sprintf("%s-udp", loadBalancerName) - var lbUUIDs []string + var lbUUIDs []string //nolint:prealloc // Use find command in order to workaround OVN bug where duplicate records of same name can exist. for _, lbName := range []string{lbTCPName, lbUDPName} { @@ -1914,7 +1954,7 @@ func (o *OVN) LoadBalancerApply(loadBalancerName OVNLoadBalancer, routers []OVNR return fmt.Errorf("Failed getting UUIDs: %w", err) } - var args []string + args := make([]string, 0, 5*len(lbUUIDs)) for _, lbUUID := range lbUUIDs { if len(args) > 0 { @@ -1991,7 +2031,7 @@ func (o *OVN) LoadBalancerApply(loadBalancerName OVNLoadBalancer, routers []OVNR // If there are some VIP rules then associate the load balancer to the requested routers. if len(vips) > 0 { - var args []string + args := make([]string, 0, 6*len(lbUUIDs)) // Get fresh list of load balancer UUIDs. lbUUIDs, err := o.loadBalancerUUIDs(loadBalancerName) @@ -2022,7 +2062,7 @@ func (o *OVN) LoadBalancerApply(loadBalancerName OVNLoadBalancer, routers []OVNR // LoadBalancerDelete deletes the specified load balancer(s). func (o *OVN) LoadBalancerDelete(loadBalancerNames ...OVNLoadBalancer) error { - var args []string + args := make([]string, 0, 5*len(loadBalancerNames)) for _, loadBalancerName := range loadBalancerNames { lbUUIDs, err := o.loadBalancerUUIDs(loadBalancerName) @@ -2084,7 +2124,8 @@ func (o *OVN) AddressSetCreate(addressSetPrefix OVNAddressSet, addresses ...net. // AddressSetAdd adds the supplied addresses to the address sets, or creates a new address sets if needed. // The address set name used is "_ip", e.g. "foo_ip4". func (o *OVN) AddressSetAdd(addressSetPrefix OVNAddressSet, addresses ...net.IPNet) error { - var args []string + args := make([]string, 0, 6*len(addresses)) + ipVersions := make(map[uint]struct{}) for _, address := range addresses { @@ -2128,7 +2169,7 @@ func (o *OVN) AddressSetAdd(addressSetPrefix OVNAddressSet, addresses ...net.IPN // AddressSetRemove removes the supplied addresses from the address set. // The address set name used is "_ip", e.g. "foo_ip4". func (o *OVN) AddressSetRemove(addressSetPrefix OVNAddressSet, addresses ...net.IPNet) error { - var args []string + args := make([]string, 0, 7*len(addresses)) for _, address := range addresses { if len(args) > 0 { @@ -2190,7 +2231,7 @@ func (o *OVN) LogicalRouterRoutes(routerName OVNRouter) ([]OVNRouterRoute, error } lines := shared.SplitNTrimSpace(strings.TrimSpace(output), "\n", -1, true) - routes := make([]OVNRouterRoute, 0) + routes := make([]OVNRouterRoute, 0, len(lines)) mainTable := true // Assume output starts with main table (supports ovn versions without multiple tables). for i, line := range lines { @@ -2268,7 +2309,7 @@ func (o *OVN) LogicalRouterPeeringApply(opts OVNRouterPeering) error { } // Start fresh command set. - var args []string + var args []string //nolint:prealloc // Will use the first IP from each family of the router port interfaces. localRouterGatewayIPs := make(map[uint]net.IP, 0) diff --git a/lxd/network/zone/zone.go b/lxd/network/zone/zone.go index 18d61f5e0ce2..8cf2bb377758 100644 --- a/lxd/network/zone/zone.go +++ b/lxd/network/zone/zone.go @@ -17,6 +17,7 @@ import ( "github.com/canonical/lxd/lxd/util" "github.com/canonical/lxd/shared" "github.com/canonical/lxd/shared/api" + "github.com/canonical/lxd/shared/dnsutil" "github.com/canonical/lxd/shared/logger" "github.com/canonical/lxd/shared/revert" "github.com/canonical/lxd/shared/validate" @@ -391,9 +392,9 @@ func (d *zone) Content() (*strings.Builder, error) { includeV6 := includeNAT || shared.IsFalseOrEmpty(netConfig["ipv6.nat"]) // Check if dealing with a reverse zone. - isReverse4 := strings.HasSuffix(d.info.Name, ip4Arpa) - isReverse6 := strings.HasSuffix(d.info.Name, ip6Arpa) - isReverse := isReverse4 || isReverse6 + isReverse := dnsutil.IsReverse(d.info.Name + ".") + isReverse4 := isReverse == 1 + isReverse6 := isReverse == 2 genRecord := func(name string, ip net.IP) map[string]string { isV4 := ip.To4() != nil @@ -409,7 +410,7 @@ func (d *zone) Content() (*strings.Builder, error) { record := map[string]string{} record["ttl"] = "300" - if !isReverse { + if isReverse == 0 { if isV4 { record["type"] = "A" } else { @@ -429,7 +430,7 @@ func (d *zone) Content() (*strings.Builder, error) { } // Get the ARPA record. - reverseAddr := reverse(ip) + reverseAddr := dnsutil.Reverse(ip) if reverseAddr == "" { return nil } @@ -442,7 +443,7 @@ func (d *zone) Content() (*strings.Builder, error) { return record } - if isReverse { + if isReverse > 0 { // Load network leases in correct project context for each forward zone referenced. for _, forwardZoneName := range shared.SplitNTrimSpace(n.Config()["dns.zone.forward"], ",", -1, true) { // Get forward zone's project. diff --git a/lxd/network/zone/reverse.go b/shared/dnsutil/reverse.go similarity index 70% rename from lxd/network/zone/reverse.go rename to shared/dnsutil/reverse.go index 3e3afa750049..0619ac1d5441 100644 --- a/lxd/network/zone/reverse.go +++ b/shared/dnsutil/reverse.go @@ -1,26 +1,22 @@ -package zone +package dnsutil import ( "net" ) -// Zone suffixes. -var ip4Arpa = ".in-addr.arpa" -var ip6Arpa = ".ip6.arpa" - -// reverse takes an IPv4 or IPv6 address and returns the matching ARPA record. -func reverse(ip net.IP) (arpa string) { +// Reverse takes an IPv4 or IPv6 address and returns the matching ARPA record. +func Reverse(ip net.IP) (arpa string) { if ip == nil { return "" } // Deal with IPv4. if ip.To4() != nil { - return uitoa(uint(ip[15])) + "." + uitoa(uint(ip[14])) + "." + uitoa(uint(ip[13])) + "." + uitoa(uint(ip[12])) + ip4Arpa + "." + return uitoa(uint(ip[15])) + "." + uitoa(uint(ip[14])) + "." + uitoa(uint(ip[13])) + "." + uitoa(uint(ip[12])) + IP4arpa } // Deal with IPv6. - buf := make([]byte, 0, len(ip)*4+len(ip6Arpa)) + buf := make([]byte, 0, len(ip)*4+len(IP6arpa)) // Add it, in reverse, to the buffer. for i := len(ip) - 1; i >= 0; i-- { @@ -32,7 +28,7 @@ func reverse(ip net.IP) (arpa string) { } // Add the suffix. - buf = append(buf, ip6Arpa[1:]+"."...) + buf = append(buf, IP6arpa[1:]...) return string(buf) } diff --git a/test/suites/network_ovn.sh b/test/suites/network_ovn.sh index 14b0c1f9fdba..d8fcf76d7990 100644 --- a/test/suites/network_ovn.sh +++ b/test/suites/network_ovn.sh @@ -100,8 +100,8 @@ test_network_ovn() { # Check expected chassis and chassis group are created. chassis_group_name="lxd-net${ovn_network_id}" - chassis_id="$(sudo ovn-nbctl --format json get ha_chassis_group "${chassis_group_name}" ha_chassis | tr -d '[]')" - sudo ovn-nbctl get ha_chassis "${chassis_id}" priority + chassis_id="$(ovn-nbctl --format json get ha_chassis_group "${chassis_group_name}" ha_chassis | tr -d '[]')" + ovn-nbctl get ha_chassis "${chassis_id}" priority # Check expected logical router has the correct name. logical_router_name="${chassis_group_name}-lr" @@ -209,8 +209,8 @@ test_network_ovn() { # Busybox test image won't bring up the IPv4 interface by itself. Get the address and bring it up. c1_ipv4_address="$(ovn-nbctl get logical_switch_port "${c1_internal_switch_port_name}" dynamic_addresses | tr -d '"' | cut -d' ' -f 2)" c1_ipv6_address="$(ovn-nbctl get logical_switch_port "${c1_internal_switch_port_name}" dynamic_addresses | tr -d '"' | cut -d' ' -f 3)" - lxc exec c1 -- ip -4 addr add "${c1_ipv4_address}/32" dev eth0 - lxc exec c1 -- ip -4 route add default dev eth0 + lxc exec c1 -- ip -4 addr add "${c1_ipv4_address}/24" dev eth0 + lxc exec c1 -- ip -4 route add default via 10.24.140.1 dev eth0 # Should now be able to get the same IPv4 address from the instance state. [ "$(lxc query /1.0/instances/c1?recursion=1 | jq -er '.state.network.eth0.addresses | .[] | select(.family == "inet").address')" = "${c1_ipv4_address}" ] @@ -228,12 +228,28 @@ test_network_ovn() { [ "$(ovn-nbctl get logical_switch_port "${c1_internal_switch_port_name}" external_ids:lxd_switch)" = "${internal_switch_name}" ] # Assert DNS configuration. - dns_entry_uuid="$(sudo ovn-nbctl --format csv --no-headings find dns "external_ids:lxd_switch_port=${c1_internal_switch_port_name}" | cut -d, -f1)" + dns_entry_uuid="$(ovn-nbctl --format csv --no-headings find dns "external_ids:lxd_switch_port=${c1_internal_switch_port_name}" | cut -d, -f1)" [ "$(ovn-nbctl get dns "${dns_entry_uuid}" external_ids:lxd_switch)" = "${internal_switch_name}" ] [ "$(ovn-nbctl get dns "${dns_entry_uuid}" records:c1.lxd)" = '"'"${c1_ipv4_address} ${c1_ipv6_address}"'"' ] + # Test DNS resolution. + [ "$(lxc exec c1 -- nslookup c1.lxd 10.10.10.1 | grep -cF "${c1_ipv6_address}")" = 1 ] + [ "$(lxc exec c1 -- nslookup c1.lxd fd42:4242:4242:1010::1 | grep -cF "${c1_ipv6_address}")" = 1 ] + + [ "$(lxc exec c1 -- nslookup "${c1_ipv4_address}" 10.10.10.1 | grep -cF c1.lxd)" = 1 ] + [ "$(lxc exec c1 -- nslookup "${c1_ipv4_address}" fd42:4242:4242:1010::1 | grep -cF c1.lxd)" = 1 ] + + [ "$(lxc exec c1 -- nslookup "${c1_ipv6_address}" 10.10.10.1 | grep -cF c1.lxd)" = 1 ] + [ "$(lxc exec c1 -- nslookup "${c1_ipv6_address}" fd42:4242:4242:1010::1 | grep -cF c1.lxd)" = 1 ] + # Clean up. lxc delete c1 --force + + # Test ha_chassis removal on shutdown + shutdown_lxd "${LXD_DIR}" + ! ovn-nbctl get ha_chassis "${chassis_id}" priority || false + respawn_lxd "${LXD_DIR}" true + lxc network delete "${ovn_network}" lxc network delete "${uplink_network}"