incusd/network/ovn/nb: Port DeleteLogicalRouter and LogicalRouterSNAT (WIP)

[Abhiram824]

No description provided.

[stgraber]

Did a few changes:

• Merged both commits together
• Split the two functions into separate commits
• Added check for logical router being already removed
• Removed the old LogicalRouterDelete and CreateLogicalRouterSNAT functions
• Added a commit to update usage of the old functions

[stgraber]

The NAT logic looks wrong.

The way things are stored in OVN is that each NAT rule belongs in the NAT table (ovnNB.NAT) and is referenced by its UUID from the LogicalRouter table. The logic you've added seems to assume that the external IP is directly part of the NAT field.

I'll take a quick stab at rewriting the logic. Basically I expect we need to:

• Get the LogicalRouter (that part is already there)
• Iterate over logicalRouter.NAT and fetch those entries from the DB
• Check if a matching entry already exists, if it does and mayExist is set, return, if it does and mayExist isn't set, return an error
• Create a new NAT table entry, add it to the DB and add a reference to it to the LogicalRouter

[stgraber]

Confirmed our test suite caught the bad logic, so at least I can use that to confirm my implementation works :)

Error: Failed adding router IPv4 SNAT rule: Wrong parameter type (*ovsmodel.LogicalRouter): Expected pointer to slice of valid Models

[stgraber]

Updated the SNAT function to do what I described above. I've got the testsuite running now to make sure it didn't regress anything.

[Abhiram824]

The NAT logic looks wrong.

The way things are stored in OVN is that each NAT rule belongs in the NAT table (ovnNB.NAT) and is referenced by its UUID from the LogicalRouter table. The logic you've added seems to assume that the external IP is directly part of the NAT field.

I'll take a quick stab at rewriting the logic. Basically I expect we need to:

• Get the LogicalRouter (that part is already there)
• Iterate over logicalRouter.NAT and fetch those entries from the DB
• Check if a matching entry already exists, if it does and mayExist is set, return, if it does and mayExist isn't set, return an error
• Create a new NAT table entry, add it to the DB and add a reference to it to the LogicalRouter

I see this makes sense, apologies for the confusion. Assuming this works I will go ahead and update the NAT related functions using a similar logic

[DhruvNistala]

Hi, we made a few more changes to the CreateLogicalRouterNAT with additional arguments to determine whether it is SNAT or DNAT, and then create the record accordingly. I also deleted the LogicalRouterDNATAdd function.