Skip to content

Port network_load_balancers to database generator #2068

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.

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

Merged
merged 3 commits into from
May 29, 2025

Conversation

davidbockelman
Copy link
Contributor

Created internal/server/db/cluster/networks_load_balancers.go which allows for the network_load_balancers SQL code to be auto-generated with make update-schema. Changed the call sites to functions in internal/server/db/network_load_balancers.go to use the new auto-generated functions. Functions in internal/server/db/network_network_load_balancers.go are no longer used.
Fixes #1800

@davidbockelman davidbockelman requested a review from stgraber as a code owner May 2, 2025 23:45
@davidbockelman davidbockelman force-pushed the db-codegen-network-load-balancer branch from d46395d to 49eaa2e Compare May 3, 2025 03:48
@tonyn10
Copy link

tonyn10 commented May 5, 2025

Hi @davidbockelman .

Your pull request has been super helpful to me, guiding me on how to tackle the database generator issue I was assigned. I'm working on network_forwards and it seems pretty analogous to yours.

One thing I think you missed is calling CreateNetworkLoadBalancerConfig after each time you call CreateNetworkLoadBalancer. With the existing framework, I don't think creating a network load balancer automatically creates a config. You would need to do something like how network_integrations handles it.

As I'm working through mine and comparing against yours, I'll let you know if I find anything else.

for _, lb := range loadBalancers {
// memberSpecific requirements
if !memberSpecific || !lb.NodeID.Valid || (lb.NodeID.Valid && lb.NodeID.Int64 == tx.GetNodeID()) {
if projectNetworksLoadBalancersOnUplink[projectName] == nil {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initialize projectNetworksLoadBalancersOnUplink = make(map[string]map[string][]string)

if err != nil {
return nil, fmt.Errorf("Failed loading network forward listen addresses: %w", err)
return nil, fmt.Errorf("Failed loading network load balancer listen addresses: %w", err)
Copy link

@tonyn10 tonyn10 May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be using this logic for retrieving the network forward listen addresses too, so maybe we can have it more generic like return nil, err?

ListenAddress: loadBalancer.ListenAddress,
Description: loadBalancer.Description,
Backends: loadBalancer.Backends,
Ports: loadBalancer.Ports,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should check these two fields if they're null or not, and assign Backends/Ports to []. Otherwise, I believe this messes up the JSON marshalling and unmarshalling.

@stgraber stgraber force-pushed the db-codegen-network-load-balancer branch from 9a10703 to 2bb65e9 Compare May 28, 2025 05:34
@stgraber
Copy link
Member

Did a first pass of cleanup on this one.

Other than some formatting issues and other static analysis stuff, this is looking pretty good.
I need to do another pass on the actual logic and tweak some more minor things (comments and the like), but I should have this one merged tomorrow.

@stgraber stgraber force-pushed the db-codegen-network-load-balancer branch from 2bb65e9 to 3a811e0 Compare May 29, 2025 05:44
@stgraber stgraber enabled auto-merge May 29, 2025 06:31
@stgraber stgraber merged commit 45d40c3 into lxc:main May 29, 2025
71 of 72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Port network_load_balancers to the database generator
3 participants