Skip to content

Commit

Permalink
Network: Fix OVN ChassisGroupChassisDelete quoting values (#14964)
Browse files Browse the repository at this point in the history
Follow on from #14904

I suspected here
#14904 (comment) the
call to get `ha_chassis` list requires both `--format=csv` to get them
onto one line per record, *and* `--data=bare` to avoid the output being
(double in this case) quoted.

This was preventing the previous fix from actually working. 

Also moved the zero-length initialised slices back to uninitialised
ones, as nil slices can be appended to, and is more in keeping with how
we use slices elsewhere.

Also added a test for chassis removal on LXD shutdown.
  • Loading branch information
tomponline authored Feb 12, 2025
2 parents 898dba7 + 5dbecf2 commit 4ae84e8
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 11 deletions.
15 changes: 7 additions & 8 deletions lxd/network/openvswitch/ovn.go
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ func (o *OVN) LogicalRouterPortSetIPv6Advertisements(portName OVNRouterPort, opt
fmt.Sprintf("ipv6_ra_configs:send_periodic=%t", opts.SendPeriodic),
}

removeRAConfigKeys := make([]string, 0) //nolint:prealloc
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 @@ -766,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 {
removeOtherConfigKeys := make([]string, 0) //nolint:prealloc
var removeOtherConfigKeys []string //nolint:prealloc
args := []string{"set", "logical_switch", string(switchName)}

if opts.PrefixIPv4 != nil {
Expand Down Expand Up @@ -814,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 {
removeOtherConfigKeys := make([]string, 0) //nolint:prealloc
var removeOtherConfigKeys []string //nolint:prealloc
args := []string{"set", "logical_switch", string(switchName)}

if len(reservedIPs) > 0 {
Expand Down Expand Up @@ -1449,7 +1449,7 @@ func (o *OVN) LogicalSwitchPortGetDNS(portName OVNSwitchPort) (OVNDNSUUID, strin
dnsUUID := strings.TrimSpace(parts[0])

var dnsName string
ips := make([]net.IP, 0) //nolint:prealloc
var ips []net.IP //nolint:prealloc

// Try and parse the DNS name and IPs.
if len(parts) > 1 {
Expand Down Expand Up @@ -1629,7 +1629,7 @@ 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 {
// Map UUIDs with chassis_names.
output, err := o.nbctl("--format=csv", "--no-headings", "--column=_uuid,chassis_name", "find", "ha_chassis")
output, err := o.nbctl("--format=csv", "--no-headings", "--data=bare", "--column=_uuid,chassis_name", "find", "ha_chassis")
if err != nil {
return err
}
Expand Down Expand Up @@ -1925,7 +1925,7 @@ func (o *OVN) loadBalancerUUIDs(loadBalancerName OVNLoadBalancer) ([]string, err
lbTCPName := fmt.Sprintf("%s-tcp", loadBalancerName)
lbUDPName := fmt.Sprintf("%s-udp", loadBalancerName)

lbUUIDs := make([]string, 0) //nolint:prealloc
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 @@ -2309,8 +2309,7 @@ func (o *OVN) LogicalRouterPeeringApply(opts OVNRouterPeering) error {
}

// Start fresh command set.

args := make([]string, 0) //nolint:prealloc
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
12 changes: 9 additions & 3 deletions test/suites/network_ovn.sh
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 @@ -232,7 +232,7 @@ 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}"'"' ]

Expand All @@ -248,6 +248,12 @@ test_network_ovn() {

# 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}"

# Create project for following tests.
Expand Down

0 comments on commit 4ae84e8

Please sign in to comment.