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.

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

Add joinSubnets config for UDNs #4507

Merged
merged 12 commits into from
Jul 17, 2024

Conversation

tssurya
Copy link
Member

@tssurya tssurya commented Jul 8, 2024

JoinSubnets are allocated only for default networks as of today.
We never needed them on UDNs since we never supported
pod2Egress there and never had a join or GR in the topologies.

Given this is changing moving forward, this PR aims to ensure
joinSubnet is configurable via NAD configs. If not provided
assumes the default values. We also change code to ensure
join subnet is calculated also for UDNs (primary only) moving
forward.

NOTE: Validation of joinSubnet for UDN against default network's
joinSubnet will be done in another PR. I don't think there is any
validation for pod subnet cidr range as well, so might as well get
both done together in the future (Being tracked separately)

@tssurya tssurya added the feature/user-defined-network-segmentation All PRs related to User defined network segmentation label Jul 8, 2024
@github-actions github-actions bot added area/unit-testing Issues related to adding/updating unit tests feature/kubevirt-live-migration All issues related to kubevirt live migration feature/egress-firewall All issues related to egress firewall labels Jul 8, 2024
@tssurya tssurya changed the title Add joinSubnets for UDNs with role:Primary Add joinSubnets config for UDNs with role:Primary Jul 8, 2024
@tssurya tssurya mentioned this pull request Jul 8, 2024
9 tasks
@tssurya
Copy link
Member Author

tssurya commented Jul 8, 2024

@tssurya tssurya force-pushed the udn-join-subnet-allocator branch from 2571830 to 36f6b78 Compare July 8, 2024 14:43
@tssurya tssurya marked this pull request as ready for review July 8, 2024 17:00
@tssurya tssurya requested a review from a team as a code owner July 8, 2024 17:00
@tssurya tssurya requested a review from maiqueb July 8, 2024 17:00
@tssurya tssurya changed the title Add joinSubnets config for UDNs with role:Primary Add joinSubnets config for UDNs Jul 8, 2024
Copy link
Contributor

@qinqon qinqon left a comment

Choose a reason for hiding this comment

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

For layer2 we commented to use transit similar subnet rather than join subnet but I think that this is just coscmetics.

go-controller/pkg/cni/types/types.go Show resolved Hide resolved
go-controller/pkg/util/multi_network.go Show resolved Hide resolved
go-controller/pkg/util/multi_network.go Outdated Show resolved Hide resolved
go-controller/pkg/util/multi_network.go Outdated Show resolved Hide resolved
go-controller/pkg/util/multi_network.go Show resolved Hide resolved
go-controller/pkg/ovn/base_network_controller.go Outdated Show resolved Hide resolved
go-controller/pkg/util/pod_annotation.go Show resolved Hide resolved
go-controller/pkg/util/node_annotations.go Outdated Show resolved Hide resolved
This commit adds `joinSubnet` field
to the NAD's config spec so that users
can specify a custom v4/v6 join subnet
CIDR. It is expected that for a given
pod, the joinSubnet's across all the
networks the pod is attached to should
be unique. However given we don't support
services on secondary networks today,
this means the primary UDN's joinSubnet
should not overlap with default network's
joinSubnet.

NOTE: Validation is not in scope, this
will be a different PR.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
This commit adds the joinSubnet field
to the netInfo internal struct that will
store the custom value passed by the user
if any OR pick the default values:
"100.65.0.0/16" and "fd99::/64".

This field shouldn't be allowed for
localnet topology. This PR also adds
util functions to easily fetch the
subnets from other parts of the code.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
ovnClusterLRPToJoinIfAddrs field stores the GR's allocated
joinsubnet IPs for that network. This was only used from
DNC, but let's move this to BNC so that secondary networks
can leverage this.

Also make this use the new utils JoinSubnetV4() and JoinSubnetV6()
that we added in the previous commit.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
This commit adds routes towards UDN with role:primary
for each UDN pod.
This is required to steer service traffic replies
correctly.
NOTE: Given default routes are towards ovn-udn interface always
its ok for us to not have specific services/join routes; this is
being done for design completion.

Sample Output L3 Network:

surya@fedora:~$ oc rsh -n ns1 tinypod
/ # ip r
default via 10.128.1.1 dev ovn-udn1
10.96.0.0/16 via 10.128.1.1 dev ovn-udn1
10.128.0.0/16 via 10.128.1.1 dev ovn-udn1
10.128.1.0/24 dev ovn-udn1 proto kernel scope link src 10.128.1.4
10.244.0.0/16 via 10.244.1.1 dev eth0
10.244.1.0/24 dev eth0 proto kernel scope link src 10.244.1.3
100.64.0.0/16 via 10.244.1.1 dev eth0
100.65.0.0/16 via 10.128.1.1 dev ovn-udn1
/ # ip -6 r
2010:100:200:1::/64 dev ovn-udn1 proto kernel metric 256 pref medium
2010:100:200::/60 via 2010:100:200:1::1 dev ovn-udn1 metric 1024 pref medium
fd00:10:96::/112 via 2010:100:200:1::1 dev ovn-udn1 metric 1024 pref medium
fd00:10:244:2::/64 dev eth0 proto kernel metric 256 pref medium
fd00:10:244::/48 via fd00:10:244:2::1 dev eth0 metric 1024 pref medium
fd98::/64 via fd00:10:244:2::1 dev eth0 metric 1024 pref medium
fd99::/64 via 2010:100:200:1::1 dev ovn-udn1 metric 1024 pref medium
fe80::/64 dev eth0 proto kernel metric 256 pref medium
fe80::/64 dev ovn-udn1 proto kernel metric 256 pref medium
default via 2010:100:200:1::1 dev ovn-udn1 metric 1024 pref medium

Sample Output L2 Network:
surya@fedora:~$ oc rsh -n ns2 tinypod
/ # ip r
default via 10.100.200.1 dev ovn-udn1
10.96.0.0/16 via 10.100.200.1 dev ovn-udn1
10.100.200.0/24 dev ovn-udn1 proto kernel scope link src 10.100.200.7
10.244.0.0/16 via 10.244.1.1 dev eth0
10.244.1.0/24 dev eth0 proto kernel scope link src 10.244.1.4
100.64.0.0/16 via 10.244.1.1 dev eth0
100.65.0.0/16 via 10.100.200.1 dev ovn-udn1
/ # ip -6 r
2010:100:200::/60 dev ovn-udn1 proto kernel metric 256 pref medium
fd00:10:96::/112 via 2010:100:200::1 dev ovn-udn1 metric 1024 pref medium
fd00:10:244:2::/64 dev eth0 proto kernel metric 256 pref medium
fd00:10:244::/48 via fd00:10:244:2::1 dev eth0 metric 1024 pref medium
fd98::/64 via fd00:10:244:2::1 dev eth0 metric 1024 pref medium
fd99::/64 via 2010:100:200::1 dev ovn-udn1 metric 1024 pref medium
fe80::/64 dev eth0 proto kernel metric 256 pref medium
fe80::/64 dev ovn-udn1 proto kernel metric 256 pref medium
default via 2010:100:200::1 dev ovn-udn1 metric 1024 pref medium

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
This commit adds a new annotation for storing
GR joinIP information on a per network basis.

The older annotation "node-gateway-router-lrp-ifaddr"
was not saving this on a per network basis and only
doing it for default network. Future commits will
deprecate this annotation in favor of the new one.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Split out common functionality of converting PrimaryIfAddrAnnotation
to net.IPNet into its own util.
This will be useful in the next commit to avoid
code duplication.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
This commit adds util to parse the new
k8s.ovn.org/node-gateway-router-lrp-ifaddrs
annotation. Will be used in future commits.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
This commit adds UpdateNodeGatewayRouterLRPAddrsAnnotation
util function that will be used from cluster-manager
in future commits

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Deprecare the older annotation and its utils by starting
to use the newly added utils, however for backwards
compatibility during upgrades where controller upgrades
before cluster-manager, we still need to keep the older
annotation around for 1 more release.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Move the logic to generate join subnet IPs from
zone interconnect controllers that is specific
to default network over to node allocators so that
it happens for both default and primary networks
(as long as topology is not localnet).

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Copy link
Contributor

@qinqon qinqon left a comment

Choose a reason for hiding this comment

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

LGTM

@coveralls
Copy link

Coverage Status

coverage: 52.624% (+0.1%) from 52.523%
when pulling 9ebbc18 on tssurya:udn-join-subnet-allocator
into 64265c7 on ovn-org:master.

@tssurya
Copy link
Member Author

tssurya commented Jul 17, 2024

@tssurya
Copy link
Member Author

tssurya commented Jul 17, 2024

there is a slight dependency with changes in annotation names.. so have started a d/s merge; openshift/ovn-kubernetes#2228 once upgrades passes there I will merge this.

Meanwhile let me rebase this on master latest

@tssurya
Copy link
Member Author

tssurya commented Jul 17, 2024

So even if the upgrades failed: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_ovn-kubernetes/2228/pull-ci-openshift-ovn-kubernetes-master-4.17-upgrade-from-stable-4.16-e2e-aws-ovn-upgrade/1813615743032365056
it was for different reasons.
the PR here is doing things correctly: https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_ovn-kubernetes/2228/pull-ci-openshift-ovn-kubernetes-master-4.17-upgrade-from-stable-4.16-e2e-aws-ovn-upgrade/1813615743032365056/artifacts/e2e-aws-ovn-upgrade/gather-extra/artifacts/nodes.json

                    "k8s.ovn.org/network-ids": "{\"default\":\"0\"}",
                    "k8s.ovn.org/node-chassis-id": "bd3161f1-7ec4-494f-a985-1a1d7697ded6",
                    "k8s.ovn.org/node-gateway-router-lrp-ifaddr": "{\"ipv4\":\"100.64.0.6/16\"}",
                    "k8s.ovn.org/node-gateway-router-lrp-ifaddrs": "{\"default\":{\"ipv4\":\"100.64.0.6/16\"}}",
                    "k8s.ovn.org/node-id": "6",

@tssurya tssurya merged commit 3022f31 into ovn-org:master Jul 17, 2024
38 of 39 checks passed
@tssurya tssurya added this to the v1.1.0 milestone Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/unit-testing Issues related to adding/updating unit tests feature/egress-firewall All issues related to egress firewall feature/kubevirt-live-migration All issues related to kubevirt live migration feature/user-defined-network-segmentation All PRs related to User defined network segmentation
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants