-
Notifications
You must be signed in to change notification settings - Fork 932
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
Show gateway leases when features.networks
is disabled (for default project networks)
#14305
base: main
Are you sure you want to change the base?
Conversation
a2f8bac
to
6db63a7
Compare
lxd/network/driver_ovn.go
Outdated
|
||
// Only query for project when we need to check for "features.networks". That is, when the network project is | ||
// the default project and projectName refers to any one other project. | ||
if projectName != api.ProjectDefaultName && projectName != "" && n.project == api.ProjectDefaultName { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this logic into NetworkProject()
to avoid the duplication? Would that be correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this would be correct since we have to consider the networks from project default
and the instances from the requested project (e.g. projectName
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets move this to an exported function in network.Common
lxd/network/driver_ovn.go
Outdated
|
||
// Include gateway IPs when the network belongs to requested project, getting leases for all projects or | ||
// requested project doesn't have features.networks and network is in default project. | ||
if projectName == n.project || projectName == "" || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please dont put if statements on multiple lines.
If its too long please break it up into multiple variables to make the if statement clearer.
12a306e
to
9efb65f
Compare
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
Signed-off-by: hamistao <[email protected]>
We do want gateway Leases to show when listing them for a project if the network is visible from that project. However, when dealing with DNS zone configuration, we only want to consider these leases when dealing with the network's original project. Signed-off-by: hamistao <[email protected]>
521b255
to
d46f1fc
Compare
If project
foo
hasfeatures.networks
disabled on a project, it does not have networks of its own and the networks from the default project are visible from that project. However, the gateway leases for networks on the default project don't show onlxc network list-leases lxdbr0 --project foo
.This PR proposes changing this behavior and showing those leases, being more consistent with how we handle
features.networks
.