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.

Sign up for GitHub

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

Jump to bottom

User defined primary network canary impl #4459

Closed
wants to merge 16 commits into from
Closed

User defined primary network canary impl #4459

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2b934c0
factory,control-manager: Add namespaces informer
qinqon Jun 13, 2024
56aaf25
rbac: Allow controller manager to patch ns
qinqon Jun 13, 2024
8ccc653
util: Add namespace active-network updater
qinqon Jun 12, 2024
11f542e
nad, controller: Add manager system
qinqon Jun 12, 2024
cb57eb8
gh: Add network-segmentation lane
qinqon Jun 13, 2024
ffdd7a6
nad: Ensure active-network annotation at namespace
qinqon Jun 13, 2024
81fc424
nad: Handler primary network deletion
qinqon Jun 17, 2024
615bffc
Add primary field to pod annotation
tssurya Jun 15, 2024
8d19a52
Steer default and service traffic via primary secondary network
tssurya Jun 17, 2024
a6d6703
user-def-nets: patch OVN annotations
maiqueb Jun 20, 2024
9d40816
e2e: remove unused function
maiqueb Jun 20, 2024
21c6486
e2e, user-defined-nets: add east/west test
maiqueb Jun 20, 2024
c0d1609
user-def-nets, node: create LSP for primary net
maiqueb Jun 20, 2024
b2ad906
user-def-nets, cni: configure the additional net iface
maiqueb Jun 20, 2024
707485d
user-def-nets, control-plane: set default GW on the user defined net
maiqueb Jun 20, 2024
cb92150
e2e, user-defined-nets: add egress test
maiqueb Jun 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ jobs:
- {"target": "kv-live-migration", "ha": "noHA", "gateway-mode": "local", "ipfamily": "ipv4", "disable-snat-multiple-gws": "SnatGW", "second-bridge": "1br", "ic": "ic-disabled", "num-workers": "3"}
- {"target": "kv-live-migration", "ha": "noHA", "gateway-mode": "shared", "ipfamily": "dualstack", "disable-snat-multiple-gws": "SnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "num-workers": "3"}
- {"target": "control-plane", "ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv4", "disable-snat-multiple-gws": "SnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "forwarding": "disable-forwarding"}
- {"target": "network-segmentation", "ha": "noHA", "gateway-mode": "shared", "ipfamily": "dualstack", "disable-snat-multiple-gws": "SnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones"}
needs: [ build-pr ]
env:
JOB_NAME: "${{ matrix.target }}-${{ matrix.ha }}-${{ matrix.gateway-mode }}-${{ matrix.ipfamily }}-${{ matrix.disable-snat-multiple-gws }}-${{ matrix.second-bridge }}"
Expand All @@ -437,7 +438,8 @@ jobs:
OVN_SECOND_BRIDGE: "${{ matrix.second-bridge == '2br' }}"
KIND_IPV4_SUPPORT: "${{ matrix.ipfamily == 'IPv4' || matrix.ipfamily == 'dualstack' }}"
KIND_IPV6_SUPPORT: "${{ matrix.ipfamily == 'IPv6' || matrix.ipfamily == 'dualstack' }}"
ENABLE_MULTI_NET: "${{ matrix.target == 'multi-homing' || matrix.target == 'kv-live-migration' }}"
ENABLE_MULTI_NET: "${{ matrix.target == 'multi-homing' || matrix.target == 'kv-live-migration' || matrix.target == 'network-segmentation' }}"
ENABLE_NETWORK_SEGMENTATION: "${{ matrix.target == 'network-segmentation' }}"
KIND_INSTALL_KUBEVIRT: "${{ matrix.target == 'kv-live-migration' }}"
OVN_COMPACT_MODE: "${{ matrix.target == 'compact-mode' }}"
OVN_DUMMY_GATEWAY_BRIDGE: "${{ matrix.target == 'compact-mode' }}"
Expand Down Expand Up @@ -525,6 +527,8 @@ jobs:
make -C test control-plane WHAT="Kubevirt Virtual Machines"
elif [ "${{ matrix.target }}" == "control-plane-helm" ]; then
make -C test control-plane
elif [ "${{ matrix.target }}" == "network-segmentation" ]; then
make -C test control-plane WHAT="Network Segmentation"
else
make -C test ${{ matrix.target }}
if [ "${{ matrix.ipfamily }}" != "ipv6" ]; then
Expand Down
4 changes: 4 additions & 0 deletions dist/templates/rbac-ovnkube-cluster-manager.yaml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,7 @@ rules:
- adminnetworkpolicies/status
- baselineadminnetworkpolicies/status
verbs: [ "patch" ]
- apiGroups: [""]
resources:
- namespaces/status
verbs: [ "patch" ]
33 changes: 31 additions & 2 deletions go-controller/pkg/allocator/pod/pod_annotation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ import (

// PodAnnotationAllocator is a utility to handle allocation of the PodAnnotation to Pods.
type PodAnnotationAllocator struct {
podLister listers.PodLister
kube kube.InterfaceOVN
podLister listers.PodLister
namespaceLister listers.NamespaceLister
kube kube.InterfaceOVN

netInfo util.NetInfo
ipamClaimsReconciler persistentips.PersistentAllocations
Expand All @@ -32,11 +33,13 @@ type PodAnnotationAllocator struct {
func NewPodAnnotationAllocator(
netInfo util.NetInfo,
podLister listers.PodLister,
nsLister listers.NamespaceLister,
kube kube.InterfaceOVN,
claimsReconciler persistentips.PersistentAllocations,
) *PodAnnotationAllocator {
return &PodAnnotationAllocator{
podLister: podLister,
namespaceLister: nsLister,
kube: kube,
netInfo: netInfo,
ipamClaimsReconciler: claimsReconciler,
Expand All @@ -63,6 +66,7 @@ func (allocator *PodAnnotationAllocator) AllocatePodAnnotation(

return allocatePodAnnotation(
allocator.podLister,
allocator.namespaceLister,
allocator.kube,
ipAllocator,
allocator.netInfo,
Expand All @@ -75,6 +79,7 @@ func (allocator *PodAnnotationAllocator) AllocatePodAnnotation(

func allocatePodAnnotation(
podLister listers.PodLister,
namespaceLister listers.NamespaceLister,
kube kube.Interface,
ipAllocator subnet.NamedAllocator,
netInfo util.NetInfo,
Expand All @@ -92,6 +97,7 @@ func allocatePodAnnotation(
allocateToPodWithRollback := func(pod *v1.Pod) (*v1.Pod, func(), error) {
var rollback func()
pod, podAnnotation, rollback, err = allocatePodAnnotationWithRollback(
namespaceLister,
ipAllocator,
idAllocator,
netInfo,
Expand Down Expand Up @@ -137,6 +143,7 @@ func (allocator *PodAnnotationAllocator) AllocatePodAnnotationWithTunnelID(

return allocatePodAnnotationWithTunnelID(
allocator.podLister,
allocator.namespaceLister,
allocator.kube,
ipAllocator,
idAllocator,
Expand All @@ -150,6 +157,7 @@ func (allocator *PodAnnotationAllocator) AllocatePodAnnotationWithTunnelID(

func allocatePodAnnotationWithTunnelID(
podLister listers.PodLister,
namespaceLister listers.NamespaceLister,
kube kube.Interface,
ipAllocator subnet.NamedAllocator,
idAllocator id.NamedAllocator,
Expand All @@ -165,6 +173,7 @@ func allocatePodAnnotationWithTunnelID(
allocateToPodWithRollback := func(pod *v1.Pod) (*v1.Pod, func(), error) {
var rollback func()
pod, podAnnotation, rollback, err = allocatePodAnnotationWithRollback(
namespaceLister,
ipAllocator,
idAllocator,
netInfo,
Expand Down Expand Up @@ -207,6 +216,7 @@ func allocatePodAnnotationWithTunnelID(
// implementations. Use an inlined implementation if you want to extract
// information from it as a side-effect.
func allocatePodAnnotationWithRollback(
namespaceLister listers.NamespaceLister,
ipAllocator subnet.NamedAllocator,
idAllocator id.NamedAllocator,
netInfo util.NetInfo,
Expand Down Expand Up @@ -265,6 +275,25 @@ func allocatePodAnnotationWithRollback(
IPs: podAnnotation.IPs,
MAC: podAnnotation.MAC,
TunnelID: podAnnotation.TunnelID,
Primary: false,
}
if util.IsNetworkSegmentationSupportEnabled() {
tentative.Primary = netInfo.IsSecondary() && netInfo.IsPrimaryNetwork()
if !netInfo.IsSecondary() { // only do this extra bit if needed
var podNamespace *v1.Namespace
podNamespace, err = namespaceLister.Get(pod.Namespace)
if err != nil {
return
}
activeNetworkForPod := util.GetNamespaceActiveNetwork(podNamespace)
if activeNetworkForPod == nadName {
tentative.Primary = true
}
}
} else if !netInfo.IsSecondary() {
// if user defined network segmentation is not enabled
// then we know pod's primary network is "default"
tentative.Primary = true
}

hasIDAllocation := util.DoesNetworkRequireTunnelIDs(netInfo)
Expand Down
49 changes: 37 additions & 12 deletions go-controller/pkg/allocator/pod/pod_annotation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

cnitypes "github.com/containernetworking/cni/pkg/types"
mocks "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/mocks/k8s.io/client-go/listers/core/v1"
"github.com/stretchr/testify/mock"

ipamclaimsapi "github.com/k8snetworkplumbingwg/ipamclaims/pkg/crd/ipamclaims/v1alpha1"
nadapi "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
Expand Down Expand Up @@ -115,6 +117,7 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
ipam bool
idAllocation bool
persistentIPAllocation bool
primaryNetwork bool
podAnnotation *util.PodAnnotation
invalidNetworkAnnotation bool
wantUpdatedPod bool
Expand Down Expand Up @@ -157,9 +160,11 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
},
},
wantUpdatedPod: true,
primaryNetwork: true,
wantPodAnnotation: &util.PodAnnotation{
IPs: ovntest.MustParseIPNets("192.168.0.4/24"),
MAC: util.IPAddrToHWAddr(ovntest.MustParseIPNets("192.168.0.4/24")[0].IP),
IPs: ovntest.MustParseIPNets("192.168.0.4/24"),
MAC: util.IPAddrToHWAddr(ovntest.MustParseIPNets("192.168.0.4/24")[0].IP),
Primary: true,
},
},
{
Expand All @@ -175,11 +180,13 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
netxtIPs: ovntest.MustParseIPNets("192.168.0.3/24"),
},
},
primaryNetwork: true,
wantUpdatedPod: true,
wantPodAnnotation: &util.PodAnnotation{
IPs: ovntest.MustParseIPNets("192.168.0.4/24"),
MAC: util.IPAddrToHWAddr(ovntest.MustParseIPNets("192.168.0.4/24")[0].IP),
Gateways: ovntest.MustParseIPs("192.168.0.1"),
Primary: true,
},
},
{
Expand Down Expand Up @@ -216,6 +223,7 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
NextHop: ovntest.MustParseIP("192.168.0.1").To4(),
},
},
Primary: true, // default network's netInfo
},
wantReleasedIPsOnRollback: ovntest.MustParseIPNets("192.168.0.3/24"),
},
Expand All @@ -225,15 +233,17 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
name: "expect no updates, annotated, IPAM",
ipam: true,
podAnnotation: &util.PodAnnotation{
IPs: ovntest.MustParseIPNets("192.168.0.3/24"),
MAC: util.IPAddrToHWAddr(ovntest.MustParseIPNets("192.168.0.3/24")[0].IP),
IPs: ovntest.MustParseIPNets("192.168.0.3/24"),
MAC: util.IPAddrToHWAddr(ovntest.MustParseIPNets("192.168.0.3/24")[0].IP),
Primary: true, // default network netInfo,
},
args: args{
ipAllocator: &ipAllocatorStub{},
},
wantPodAnnotation: &util.PodAnnotation{
IPs: ovntest.MustParseIPNets("192.168.0.3/24"),
MAC: util.IPAddrToHWAddr(ovntest.MustParseIPNets("192.168.0.3/24")[0].IP),
IPs: ovntest.MustParseIPNets("192.168.0.3/24"),
MAC: util.IPAddrToHWAddr(ovntest.MustParseIPNets("192.168.0.3/24")[0].IP),
Primary: true, // default network netInfo,
},
wantReleasedIPsOnRollback: ovntest.MustParseIPNets("192.168.0.3/24"),
},
Expand All @@ -243,17 +253,19 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
name: "expect no updates, annotated, already allocated, IPAM",
ipam: true,
podAnnotation: &util.PodAnnotation{
IPs: ovntest.MustParseIPNets("192.168.0.3/24"),
MAC: util.IPAddrToHWAddr(ovntest.MustParseIPNets("192.168.0.3/24")[0].IP),
IPs: ovntest.MustParseIPNets("192.168.0.3/24"),
MAC: util.IPAddrToHWAddr(ovntest.MustParseIPNets("192.168.0.3/24")[0].IP),
Primary: true, // default network netInfo
},
args: args{
ipAllocator: &ipAllocatorStub{
allocateIPsError: ipam.ErrAllocated,
},
},
wantPodAnnotation: &util.PodAnnotation{
IPs: ovntest.MustParseIPNets("192.168.0.3/24"),
MAC: util.IPAddrToHWAddr(ovntest.MustParseIPNets("192.168.0.3/24")[0].IP),
IPs: ovntest.MustParseIPNets("192.168.0.3/24"),
MAC: util.IPAddrToHWAddr(ovntest.MustParseIPNets("192.168.0.3/24")[0].IP),
Primary: true, // default network netInfo
},
},
{
Expand Down Expand Up @@ -297,6 +309,7 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
NextHop: ovntest.MustParseIP("192.168.0.1").To4(),
},
},
Primary: true, // default network's netInfo
},
wantReleasedIPsOnRollback: ovntest.MustParseIPNets("192.168.0.4/24"),
},
Expand Down Expand Up @@ -326,6 +339,7 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
NextHop: ovntest.MustParseIP("192.168.0.1").To4(),
},
},
Primary: true, // default network netInfo
},
},
{
Expand Down Expand Up @@ -354,6 +368,7 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
NextHop: ovntest.MustParseIP("192.168.0.1").To4(),
},
},
Primary: true, // default network netInfo
},
wantReleasedIPsOnRollback: ovntest.MustParseIPNets("192.168.0.3/24"),
},
Expand Down Expand Up @@ -410,6 +425,7 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
NextHop: ovntest.MustParseIP("192.168.0.1").To4(),
},
},
Primary: true, // default network netInfo
},
wantReleasedIPsOnRollback: ovntest.MustParseIPNets("192.168.0.3/24"),
},
Expand Down Expand Up @@ -600,7 +616,9 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var err error

config.OVNKubernetesFeature.EnableMultiNetwork = true
config.OVNKubernetesFeature.EnableNetworkSegmentation = true
namespaceListerMock := &mocks.NamespaceLister{}
g := gomega.NewWithT(t)

network := tt.args.network
Expand All @@ -627,14 +645,20 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
NADName: nadName,
Subnets: subnets,
AllowPersistentIPs: tt.persistentIPAllocation,
PrimaryNetwork: tt.primaryNetwork,
})
if err != nil {
t.Fatalf("failed to create NetInfo: %v", err)
}
}

config.OVNKubernetesFeature.EnableInterconnect = tt.idAllocation

namespaceListerMock.On("Get", mock.AnythingOfType("string")).Return(&v1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "namespace",
Annotations: map[string]string{util.ActiveNetworkAnnotation: types.DefaultNetworkName},
},
}, nil)
pod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod",
Expand Down Expand Up @@ -666,6 +690,7 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) {
}

pod, podAnnotation, rollback, err := allocatePodAnnotationWithRollback(
namespaceListerMock,
tt.args.ipAllocator,
tt.args.idAllocator,
netInfo,
Expand Down
27 changes: 26 additions & 1 deletion go-controller/pkg/clustermanager/network_cluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"k8s.io/klog/v2"

ipamclaimsapi "github.com/k8snetworkplumbingwg/ipamclaims/pkg/crd/ipamclaims/v1alpha1"
networkattchmentdefclientset "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/clientset/versioned"

idallocator "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/id"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip/subnet"
Expand Down Expand Up @@ -62,6 +63,9 @@ type networkClusterController struct {
subnetAllocator subnet.Allocator

util.NetInfo

// TODO: this should be a watch grabbed from the watch factory
nadClient networkattchmentdefclientset.Interface
Comment on lines +66 to +68
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a leftover; but we'll need the NAD watcher to plumb into the allocator, so we can figure out what's the namespace's primary network

Copy link
Member

Choose a reason for hiding this comment

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

i did this already in my PR there is NAD watcher

}

func newNetworkClusterController(networkIDAllocator idallocator.NamedAllocator, netInfo util.NetInfo, ovnClient *util.OVNClusterManagerClientset, wf *factory.WatchFactory) *networkClusterController {
Expand All @@ -81,6 +85,7 @@ func newNetworkClusterController(networkIDAllocator idallocator.NamedAllocator,
stopChan: make(chan struct{}),
wg: wg,
networkIDAllocator: networkIDAllocator,
nadClient: ovnClient.NetworkAttchDefClient,
}

return ncc
Expand Down Expand Up @@ -181,11 +186,31 @@ func (ncc *networkClusterController) init() error {
podAllocationAnnotator = annotationalloc.NewPodAnnotationAllocator(
ncc.NetInfo,
ncc.watchFactory.PodCoreInformer().Lister(),
ncc.watchFactory.NamespaceCoreInformer().Lister(),
ncc.kube,
ipamClaimsReconciler,
)

ncc.podAllocator = pod.NewPodAllocator(ncc.NetInfo, podAllocationAnnotator, ipAllocator, ipamClaimsReconciler)
// TODO: get rid of these logs below
klog.Infof(
"DEBUG| creating pod allocator for netInfo: %q. Is it primary ? %t",
ncc.NetInfo.GetNetworkName(),
ncc.NetInfo.IsPrimaryNetwork(),
)

klog.Infof(
"DEBUG| creating pod allocator for netInfo: %q. Is it secondary ? %t",
ncc.NetInfo.GetNetworkName(),
ncc.NetInfo.IsSecondary(),
)

ncc.podAllocator = pod.NewPodAllocator(
ncc.NetInfo,
podAllocationAnnotator,
ipAllocator,
ipamClaimsReconciler,
ncc.watchFactory.NamespaceCoreInformer().Lister(),
)
if err := ncc.podAllocator.Init(); err != nil {
return fmt.Errorf("failed to initialize pod ip allocator: %w", err)
}
Expand Down
Loading