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.

Sign up for GitHub

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

OVN bug fixes backports (stable-5.21) #14966

Merged
merged 11 commits into from
Feb 12, 2025
Merged

OVN bug fixes backports (stable-5.21) #14966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
64fc2c1
Modified ChassisGroupChassisDelete to convert chassis_name to _uuid t…
escabo Feb 11, 2025
3af3719
lxd/network/openvswitch/ovn: Fix escaping of values in ChassisGroupCh…
tomponline Feb 12, 2025
5ba8b1c
test: Remove unnecessary sudo from ovn tests
tomponline Feb 12, 2025
293728b
test: Check for ha_chassis removal on lxd shutdown
tomponline Feb 12, 2025
0c8de15
lxd/network/zone/reverse: move to shared/dnsutil/reverse
tomponline Feb 10, 2025
9c2d62f
lxd/network/zone/zone: Switch to dnsutil package's Reverse function
tomponline Feb 10, 2025
7f3da9f
lxd/network/openvswitch/ovn: Add PTR record generation for instance N…
tomponline Feb 10, 2025
0a7a5be
lxd/network/openvswitch/ovn: Ignore prealloc warnings for complex loop
tomponline Feb 10, 2025
c36f338
test: Add tests for ovn DNS
tomponline Feb 10, 2025
7ab73df
Linter fixes
escabo Feb 11, 2025
b18f1c3
lxd/network/openvswitch/ovn: Don't initialise slices to empty capacity
tomponline Feb 12, 2025
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
93 changes: 67 additions & 26 deletions lxd/network/openvswitch/ovn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)))
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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++ {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
}
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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} {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 "<addressSetPrefix>_ip<IP version>", 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 {
Expand Down Expand Up @@ -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 "<addressSetPrefix>_ip<IP version>", 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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 7 additions & 6 deletions lxd/network/zone/zone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
}
Expand All @@ -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.
Expand Down
16 changes: 6 additions & 10 deletions lxd/network/zone/reverse.go → shared/dnsutil/reverse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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-- {
Expand All @@ -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)
}

Expand Down
26 changes: 21 additions & 5 deletions test/suites/network_ovn.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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}" ]
Expand All @@ -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}"

Expand Down
Loading