diff --git a/go-controller/pkg/clustermanager/pod/allocator.go b/go-controller/pkg/clustermanager/pod/allocator.go index 4eae7ddedc7..a3096b72818 100644 --- a/go-controller/pkg/clustermanager/pod/allocator.go +++ b/go-controller/pkg/clustermanager/pod/allocator.go @@ -100,7 +100,7 @@ func (a *PodAllocator) Init() error { // getActiveNetworkForNamespace returns the active network for the given namespace // and is a wrapper around util.GetActiveNetworkForNamespace -func (a *PodAllocator) getActiveNetworkForNamespace(namespace string) (string, error) { +func (a *PodAllocator) getActiveNetworkForNamespace(namespace string) (util.NetInfo, error) { return util.GetActiveNetworkForNamespace(namespace, a.nadLister) } @@ -139,16 +139,11 @@ func (a *PodAllocator) GetNetworkRole(pod *corev1.Pod) (string, error) { } activeNetwork, err := a.getActiveNetworkForNamespace(pod.Namespace) if err != nil { - return "", err - } - if activeNetwork == types.UnknownNetworkName { - // FIXME(tssurya) emit event here; add support for + // FIXME(tssurya) emit event here if util.IsUnknownActiveNetworkError; add support for // recorder in the NCM controller - return "", fmt.Errorf("unable to determine what is the"+ - "primary network for this pod %s; please remove multiple primary network"+ - "NADs from namespace %s", pod.Name, pod.Namespace) + return "", err } - if activeNetwork == a.netInfo.GetNetworkName() { + if activeNetwork.GetNetworkName() == a.netInfo.GetNetworkName() { return types.NetworkRolePrimary, nil } if a.netInfo.IsDefault() { @@ -206,7 +201,12 @@ func (a *PodAllocator) reconcile(old, new *corev1.Pod, releaseFromAllocator bool return nil } - onNetwork, networkMap, err := util.GetPodNADToNetworkMapping(pod, a.netInfo) + activeNetwork, err := a.getActiveNetworkForNamespace(pod.Namespace) + if err != nil { + return fmt.Errorf("failed looking for an active network: %w", err) + } + + onNetwork, networkMap, err := util.GetPodNADToNetworkMappingWithActiveNetwork(pod, a.netInfo, activeNetwork) if err != nil { return fmt.Errorf("failed to get NAD to network mapping: %w", err) } diff --git a/go-controller/pkg/cni/cni.go b/go-controller/pkg/cni/cni.go index 5a9a1d6212e..74971852ea2 100644 --- a/go-controller/pkg/cni/cni.go +++ b/go-controller/pkg/cni/cni.go @@ -1,8 +1,12 @@ package cni import ( + "context" "fmt" "net" + "time" + + v1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" kapi "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -10,6 +14,8 @@ import ( utilnet "k8s.io/utils/net" current "github.com/containernetworking/cni/pkg/types/100" + ovncnitypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/cni/types" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/cni/udn" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kube" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kubevirt" @@ -131,8 +137,9 @@ func (pr *PodRequest) cmdAdd(kubeAuth *KubeAPIAuth, clientset *ClientSet) (*Resp } // Get the IP address and MAC address of the pod // for DPU, ensure connection-details is present - pod, annotations, podNADAnnotation, err := GetPodWithAnnotations(pr.ctx, clientset, namespace, podName, - pr.nadName, annotCondFn) + + primaryUDN := udn.NewPrimaryNetwork(clientset.nadLister) + pod, annotations, podNADAnnotation, err := GetPodWithAnnotations(pr.ctx, clientset, namespace, podName, pr.nadName, primaryUDN.WaitForPrimaryAnnotationFn(namespace, annotCondFn)) if err != nil { return nil, fmt.Errorf("failed to get pod annotation: %v", err) } @@ -140,8 +147,7 @@ func (pr *PodRequest) cmdAdd(kubeAuth *KubeAPIAuth, clientset *ClientSet) (*Resp return nil, err } - podInterfaceInfo, err := PodAnnotation2PodInfo(annotations, podNADAnnotation, pr.PodUID, netdevName, - pr.nadName, pr.netName, pr.CNIConf.MTU) + podInterfaceInfo, err := pr.buildPodInterfaceInfo(annotations, podNADAnnotation, netdevName) if err != nil { return nil, err } @@ -150,10 +156,22 @@ func (pr *PodRequest) cmdAdd(kubeAuth *KubeAPIAuth, clientset *ClientSet) (*Resp response := &Response{KubeAuth: kubeAuth} if !config.UnprivilegedMode { + //TODO: There is nothing technical to run this at unprivileged mode but + // we will tackle that later on. response.Result, err = pr.getCNIResult(clientset, podInterfaceInfo) if err != nil { return nil, err } + if primaryUDN.Found() { + primaryUDNPodRequest := pr.buildPrimaryUDNPodRequest(pod, primaryUDN) + primaryUDNPodInfo, err := primaryUDNPodRequest.buildPodInterfaceInfo(annotations, primaryUDN.Annotation(), primaryUDN.NetworkDevice()) + if err != nil { + return nil, err + } + if _, err := primaryUDNPodRequest.getCNIResult(clientset, primaryUDNPodInfo); err != nil { + return nil, err + } + } } else { response.PodIFInfo = podInterfaceInfo } @@ -329,3 +347,42 @@ func (pr *PodRequest) getCNIResult(getter PodInfoGetter, podInterfaceInfo *PodIn IPs: ips, }, nil } + +func (pr *PodRequest) buildPrimaryUDNPodRequest( + pod *kapi.Pod, + primaryUDN *udn.UserDefinedPrimaryNetwork, +) *PodRequest { + req := &PodRequest{ + Command: pr.Command, + PodNamespace: pod.Namespace, + PodName: pod.Name, + PodUID: string(pod.UID), + SandboxID: pr.SandboxID, + Netns: pr.Netns, + IfName: primaryUDN.InterfaceName(), + CNIConf: &ovncnitypes.NetConf{ + // primary UDN MTU will be taken from config.Default.MTU + // if not specified at the NAD + MTU: primaryUDN.MTU(), + }, + timestamp: time.Now(), + IsVFIO: pr.IsVFIO, + netName: primaryUDN.NetworkName(), + nadName: primaryUDN.NADName(), + deviceInfo: v1.DeviceInfo{}, + } + req.ctx, req.cancel = context.WithTimeout(context.Background(), 2*time.Minute) + return req +} + +func (pr *PodRequest) buildPodInterfaceInfo(annotations map[string]string, podAnnotation *util.PodAnnotation, netDevice string) (*PodInterfaceInfo, error) { + return PodAnnotation2PodInfo( + annotations, + podAnnotation, + pr.PodUID, + netDevice, + pr.nadName, + pr.netName, + pr.CNIConf.MTU, + ) +} diff --git a/go-controller/pkg/cni/cniserver.go b/go-controller/pkg/cni/cniserver.go index 6e5c39d62e3..2c05fa94b32 100644 --- a/go-controller/pkg/cni/cniserver.go +++ b/go-controller/pkg/cni/cniserver.go @@ -71,6 +71,10 @@ func NewCNIServer(factory factory.NodeWatchFactory, kclient kubernetes.Interface handlePodRequestFunc: HandlePodRequest, } + if util.IsNetworkSegmentationSupportEnabled() { + s.clientSet.nadLister = factory.NADInformer().Lister() + } + if len(config.Kubernetes.CAData) > 0 { s.kubeAuth.KubeCAData = base64.StdEncoding.EncodeToString(config.Kubernetes.CAData) } diff --git a/go-controller/pkg/cni/helper_linux.go b/go-controller/pkg/cni/helper_linux.go index 6e5badcbf9d..29d6ad0fdc8 100644 --- a/go-controller/pkg/cni/helper_linux.go +++ b/go-controller/pkg/cni/helper_linux.go @@ -192,7 +192,7 @@ func setupNetwork(link netlink.Link, ifInfo *PodInterfaceInfo) error { } for _, gw := range ifInfo.Gateways { if err := cniPluginLibOps.AddRoute(nil, gw, link, ifInfo.RoutableMTU); err != nil { - return fmt.Errorf("failed to add gateway route: %v", err) + return fmt.Errorf("failed to add gateway route to link '%s': %v", link.Attrs().Name, err) } } for _, route := range ifInfo.Routes { @@ -438,8 +438,8 @@ func ConfigureOVS(ctx context.Context, namespace, podName, hostIfaceName string, return fmt.Errorf("failed to get datapath type for bridge br-int : %v", err) } - klog.Infof("ConfigureOVS: namespace: %s, podName: %s, network: %s, NAD %s, SandboxID: %q, PCI device ID: %s, UID: %q, MAC: %s, IPs: %v", - namespace, podName, ifInfo.NetName, ifInfo.NADName, sandboxID, deviceID, initialPodUID, ifInfo.MAC, ipStrs) + klog.Infof("ConfigureOVS: namespace: %s, podName: %s, hostIfaceName: %s, network: %s, NAD %s, SandboxID: %q, PCI device ID: %s, UID: %q, MAC: %s, IPs: %v", + namespace, podName, hostIfaceName, ifInfo.NetName, ifInfo.NADName, sandboxID, deviceID, initialPodUID, ifInfo.MAC, ipStrs) // Find and remove any existing OVS port with this iface-id. Pods can // have multiple sandboxes if some are waiting for garbage collection, diff --git a/go-controller/pkg/cni/helper_linux_test.go b/go-controller/pkg/cni/helper_linux_test.go index 10b05263287..929ae4dd390 100644 --- a/go-controller/pkg/cni/helper_linux_test.go +++ b/go-controller/pkg/cni/helper_linux_test.go @@ -319,7 +319,7 @@ func TestSetupNetwork(t *testing.T) { {OnCallMethodName: "AddRoute", OnCallMethodArgType: []string{"*net.IPNet", "net.IP", "*mocks.Link", "int"}, RetArgList: []interface{}{fmt.Errorf("mock error")}}, }, linkMockHelper: []ovntest.TestifyMockHelper{ - {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: "testIfaceName"}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Name: "testIfaceName"}}, CallTimes: 2}, }, }, { @@ -1128,7 +1128,7 @@ func TestSetupSriovInterface(t *testing.T) { {OnCallMethodName: "AddRoute", OnCallMethodArgType: []string{"*net.IPNet", "net.IP", "*mocks.Link", "int"}, RetArgList: []interface{}{fmt.Errorf("mock error")}}, }, linkMockHelper: []ovntest.TestifyMockHelper{ - {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Flags: net.FlagUp}}}, + {OnCallMethodName: "Attrs", OnCallMethodArgType: []string{}, RetArgList: []interface{}{&netlink.LinkAttrs{Flags: net.FlagUp}}, CallTimes: 2}, }, nsMockHelper: []ovntest.TestifyMockHelper{}, }, diff --git a/go-controller/pkg/cni/types.go b/go-controller/pkg/cni/types.go index c714edb031b..57945ad9356 100644 --- a/go-controller/pkg/cni/types.go +++ b/go-controller/pkg/cni/types.go @@ -15,6 +15,8 @@ import ( "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" nadapi "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + nadlister "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/listers/k8s.cni.cncf.io/v1" + kapi "k8s.io/api/core/v1" ) @@ -177,6 +179,7 @@ type ClientSet struct { PodInfoGetter kclient kubernetes.Interface podLister corev1listers.PodLister + nadLister nadlister.NetworkAttachmentDefinitionLister } func NewClientSet(kclient kubernetes.Interface, podLister corev1listers.PodLister) *ClientSet { diff --git a/go-controller/pkg/cni/udn/primary_network.go b/go-controller/pkg/cni/udn/primary_network.go new file mode 100644 index 00000000000..fdd381124a3 --- /dev/null +++ b/go-controller/pkg/cni/udn/primary_network.go @@ -0,0 +1,132 @@ +package udn + +import ( + "fmt" + + "k8s.io/klog/v2" + + nadlister "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/listers/k8s.cni.cncf.io/v1" + + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" +) + +// wait on a certain pod annotation related condition +type podAnnotWaitCond = func(map[string]string, string) (*util.PodAnnotation, bool) + +type UserDefinedPrimaryNetwork struct { + nadLister nadlister.NetworkAttachmentDefinitionLister + annotation *util.PodAnnotation + activeNetwork util.NetInfo +} + +func NewPrimaryNetwork(nadLister nadlister.NetworkAttachmentDefinitionLister) *UserDefinedPrimaryNetwork { + return &UserDefinedPrimaryNetwork{ + nadLister: nadLister, + } +} + +func (p *UserDefinedPrimaryNetwork) InterfaceName() string { + return "ovn-udn1" +} + +func (p *UserDefinedPrimaryNetwork) NetworkDevice() string { + // TODO: Support for non VFIO devices like SRIOV have to be implemented + return "" +} + +func (p *UserDefinedPrimaryNetwork) Annotation() *util.PodAnnotation { + return p.annotation +} + +func (p *UserDefinedPrimaryNetwork) NetworkName() string { + if p.activeNetwork == nil { + return "" + } + return p.activeNetwork.GetNetworkName() +} + +func (p *UserDefinedPrimaryNetwork) NADName() string { + if p.activeNetwork == nil || p.activeNetwork.IsDefault() { + return "" + } + nads := p.activeNetwork.GetNADs() + if len(nads) < 1 { + return "" + } + return nads[0] +} + +func (p *UserDefinedPrimaryNetwork) MTU() int { + if p.activeNetwork == nil { + return 0 + } + return p.activeNetwork.MTU() +} + +func (p *UserDefinedPrimaryNetwork) Found() bool { + return p.annotation != nil && p.activeNetwork != nil +} + +func (p *UserDefinedPrimaryNetwork) WaitForPrimaryAnnotationFn(namespace string, annotCondFn podAnnotWaitCond) podAnnotWaitCond { + return func(annotations map[string]string, nadName string) (*util.PodAnnotation, bool) { + annotation, isReady := annotCondFn(annotations, nadName) + if annotation == nil { + return nil, false + } + if nadName != types.DefaultNetworkName || annotation.Role == types.NetworkRolePrimary { + return annotation, isReady + } + + if err := p.ensureAnnotation(annotations); err != nil { + //TODO: Event ? + klog.Warningf("Failed looking for primary network annotation: %v", err) + return nil, false + } + if err := p.ensureActiveNetwork(namespace); err != nil { + //TODO: Event ? + klog.Warningf("Failed looking for primary network name: %v", err) + return nil, false + } + return annotation, isReady + } +} + +func (p *UserDefinedPrimaryNetwork) ensureActiveNetwork(namespace string) error { + if p.activeNetwork != nil { + return nil + } + activeNetwork, err := util.GetActiveNetworkForNamespace(namespace, p.nadLister) + if err != nil { + return err + } + if activeNetwork.IsDefault() { + return fmt.Errorf("missing primary user defined network NAD") + } + p.activeNetwork = activeNetwork + return nil +} + +func (p *UserDefinedPrimaryNetwork) ensureAnnotation(annotations map[string]string) error { + if p.annotation != nil { + return nil + } + podNetworks, err := util.UnmarshalPodAnnotationAllNetworks(annotations) + if err != nil { + return err + } + for nadName, podNetwork := range podNetworks { + if podNetwork.Role != types.NetworkRolePrimary { + continue + } + p.annotation, err = util.UnmarshalPodAnnotation(annotations, nadName) + if err != nil { + return err + } + break + } + if p.annotation == nil { + return fmt.Errorf("missing network annotation with primary role '%+v'", annotations) + } + return nil +} diff --git a/go-controller/pkg/cni/udn/primary_network_test.go b/go-controller/pkg/cni/udn/primary_network_test.go new file mode 100644 index 00000000000..7d931061c37 --- /dev/null +++ b/go-controller/pkg/cni/udn/primary_network_test.go @@ -0,0 +1,188 @@ +package udn + +import ( + "testing" + + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/labels" + + nadapi "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" + ovntest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing" + v1nadmocks "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/mocks/github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/listers/k8s.cni.cncf.io/v1" + types "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" +) + +func TestWaitForPrimaryAnnotationFn(t *testing.T) { + + config.OVNKubernetesFeature.EnableMultiNetwork = true + config.OVNKubernetesFeature.EnableNetworkSegmentation = true + + tests := []struct { + description string + namespace string + nadName string + annotationFromFn *util.PodAnnotation + isReadyFromFn bool + annotations map[string]string + nads []*nadapi.NetworkAttachmentDefinition + getActiveNetworkError error + expectedIsReady bool + expectedFound bool + expectedAnnotation *util.PodAnnotation + expectedNADName string + expectedNetworkName string + expectedMTU int + }{ + { + description: "With non default nad should be ready", + nadName: "red", + annotationFromFn: &util.PodAnnotation{ + Role: types.NetworkRoleSecondary, + }, + isReadyFromFn: true, + expectedIsReady: true, + expectedAnnotation: &util.PodAnnotation{ + Role: types.NetworkRoleSecondary, + }, + }, + { + description: "With no ovn annotation should force return not ready", + nadName: types.DefaultNetworkName, + annotationFromFn: nil, + isReadyFromFn: true, + expectedAnnotation: nil, + expectedIsReady: false, + }, + { + description: "With primary default should be ready", + nadName: types.DefaultNetworkName, + annotationFromFn: &util.PodAnnotation{ + Role: types.NetworkRolePrimary, + }, + isReadyFromFn: true, + expectedAnnotation: &util.PodAnnotation{ + Role: types.NetworkRolePrimary, + }, + expectedIsReady: true, + }, + { + description: "With missing primary annotation and active network should return not ready", + nadName: types.DefaultNetworkName, + annotationFromFn: &util.PodAnnotation{ + Role: types.NetworkRoleInfrastructure, + }, + isReadyFromFn: true, + expectedIsReady: false, + }, + { + description: "With primary network annotation and missing active network should return not ready", + namespace: "ns1", + nadName: types.DefaultNetworkName, + annotationFromFn: &util.PodAnnotation{ + Role: types.NetworkRoleInfrastructure, + }, + isReadyFromFn: true, + annotations: map[string]string{ + "k8s.ovn.org/pod-networks": `{"ns1/nad1": { + "role": "primary", + "mac_address": "0a:58:fd:98:00:01" + }}`, + }, + expectedIsReady: false, + }, + { + description: "With missing primary network annotation and active network should return not ready", + nadName: types.DefaultNetworkName, + annotationFromFn: &util.PodAnnotation{ + Role: types.NetworkRoleInfrastructure, + }, + isReadyFromFn: true, + nads: []*nadapi.NetworkAttachmentDefinition{ + ovntest.GenerateNAD("blue", "nad1", "ns1", + types.Layer2Topology, "10.100.200.0/24", types.NetworkRolePrimary), + }, + expectedIsReady: false, + }, + { + description: "With primary network annotation and active network should return ready", + namespace: "ns1", + nadName: types.DefaultNetworkName, + annotationFromFn: &util.PodAnnotation{ + Role: types.NetworkRoleInfrastructure, + }, + isReadyFromFn: true, + annotations: map[string]string{ + "k8s.ovn.org/pod-networks": `{"ns1/nad1": { + "role": "primary", + "mac_address": "0a:58:fd:98:00:01" + }}`, + }, + nads: []*nadapi.NetworkAttachmentDefinition{ + ovntest.GenerateNAD("blue", "nad1", "ns1", + types.Layer2Topology, "10.100.200.0/24", types.NetworkRolePrimary), + }, + expectedIsReady: true, + expectedFound: true, + expectedNetworkName: "blue", + expectedNADName: "ns1/nad1", + expectedAnnotation: &util.PodAnnotation{ + Role: types.NetworkRoleInfrastructure, + }, + expectedMTU: 1300, + }, + { + description: "With primary network annotation and active network and no MTU should return ready with default MTU", + namespace: "ns1", + nadName: types.DefaultNetworkName, + annotationFromFn: &util.PodAnnotation{ + Role: types.NetworkRoleInfrastructure, + }, + isReadyFromFn: true, + annotations: map[string]string{ + "k8s.ovn.org/pod-networks": `{"ns1/nad1": { + "role": "primary", + "mac_address": "0a:58:fd:98:00:01" + }}`, + }, + nads: []*nadapi.NetworkAttachmentDefinition{ + ovntest.GenerateNADWithoutMTU("blue", "nad1", "ns1", + types.Layer2Topology, "10.100.200.0/24", types.NetworkRolePrimary), + }, + expectedIsReady: true, + expectedFound: true, + expectedNetworkName: "blue", + expectedNADName: "ns1/nad1", + expectedAnnotation: &util.PodAnnotation{ + Role: types.NetworkRoleInfrastructure, + }, + expectedMTU: 1400, + }, + } + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + g := NewWithT(t) + nadLister := v1nadmocks.NetworkAttachmentDefinitionLister{} + nadNamespaceLister := v1nadmocks.NetworkAttachmentDefinitionNamespaceLister{} + nadLister.On("NetworkAttachmentDefinitions", tt.namespace).Return(&nadNamespaceLister) + nadNamespaceLister.On("List", labels.Everything()).Return(tt.nads, nil) + waitCond := func(map[string]string, string) (*util.PodAnnotation, bool) { + return tt.annotationFromFn, tt.isReadyFromFn + } + userDefinedPrimaryNetwork := NewPrimaryNetwork(&nadLister) + obtainedAnnotation, obtainedIsReady := userDefinedPrimaryNetwork.WaitForPrimaryAnnotationFn(tt.namespace, waitCond)(tt.annotations, tt.nadName) + obtainedFound := userDefinedPrimaryNetwork.Found() + obtainedNetworkName := userDefinedPrimaryNetwork.NetworkName() + obtainedNADName := userDefinedPrimaryNetwork.NADName() + obtainedMTU := userDefinedPrimaryNetwork.MTU() + g.Expect(obtainedIsReady).To(Equal(tt.expectedIsReady), "should return expected readiness") + g.Expect(obtainedFound).To(Equal(tt.expectedFound), "should return expected found flag") + g.Expect(obtainedNetworkName).To(Equal(tt.expectedNetworkName), "should return expected network name") + g.Expect(obtainedNADName).To(Equal(tt.expectedNADName), "should return expected nad name") + g.Expect(obtainedAnnotation).To(Equal(tt.expectedAnnotation), "should return expected ovn pod annotation") + g.Expect(obtainedMTU).To(Equal(tt.expectedMTU), "should return expected MTU") + }) + } +} diff --git a/go-controller/pkg/cni/utils.go b/go-controller/pkg/cni/utils.go index d85cb54bead..2b379d26671 100644 --- a/go-controller/pkg/cni/utils.go +++ b/go-controller/pkg/cni/utils.go @@ -16,7 +16,7 @@ import ( ) // wait on a certain pod annotation related condition -type podAnnotWaitCond func(map[string]string, string) (*util.PodAnnotation, bool) +type podAnnotWaitCond = func(map[string]string, string) (*util.PodAnnotation, bool) // isOvnReady is a wait condition for OVN master to set pod-networks annotation func isOvnReady(podAnnotation map[string]string, nadName string) (*util.PodAnnotation, bool) { diff --git a/go-controller/pkg/ovn/base_network_controller.go b/go-controller/pkg/ovn/base_network_controller.go index d5ed445d6e7..f47addd3840 100644 --- a/go-controller/pkg/ovn/base_network_controller.go +++ b/go-controller/pkg/ovn/base_network_controller.go @@ -36,6 +36,8 @@ import ( ref "k8s.io/client-go/tools/reference" "k8s.io/klog/v2" utilnet "k8s.io/utils/net" + + nadlister "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/listers/k8s.cni.cncf.io/v1" ) // CommonNetworkControllerInfo structure is place holder for all fields shared among controllers. @@ -769,8 +771,12 @@ func (bnc *BaseNetworkController) isLocalZoneNode(node *kapi.Node) bool { // getActiveNetworkForNamespace returns the active network for the given namespace // and is a wrapper around util.GetActiveNetworkForNamespace -func (bnc *BaseNetworkController) getActiveNetworkForNamespace(namespace string) (string, error) { - return util.GetActiveNetworkForNamespace(namespace, bnc.watchFactory.NADInformer().Lister()) +func (bnc *BaseNetworkController) getActiveNetworkForNamespace(namespace string) (util.NetInfo, error) { + var nadLister nadlister.NetworkAttachmentDefinitionLister + if util.IsNetworkSegmentationSupportEnabled() { + nadLister = bnc.watchFactory.NADInformer().Lister() + } + return util.GetActiveNetworkForNamespace(namespace, nadLister) } // GetNetworkRole returns the role of this controller's @@ -808,16 +814,12 @@ func (bnc *BaseNetworkController) GetNetworkRole(pod *kapi.Pod) (string, error) } activeNetwork, err := bnc.getActiveNetworkForNamespace(pod.Namespace) if err != nil { + if util.IsUnknownActiveNetworkError(err) { + bnc.recordPodErrorEvent(pod, err) + } return "", err } - if activeNetwork == types.UnknownNetworkName { - err := fmt.Errorf("unable to determine what is the"+ - "primary network for this pod %s; please remove multiple primary network"+ - "NADs from namespace %s", pod.Name, pod.Namespace) - bnc.recordPodErrorEvent(pod, err) - return "", err - } - if activeNetwork == bnc.GetNetworkName() { + if activeNetwork.GetNetworkName() == bnc.GetNetworkName() { return types.NetworkRolePrimary, nil } if bnc.IsDefault() { diff --git a/go-controller/pkg/ovn/base_network_controller_secondary.go b/go-controller/pkg/ovn/base_network_controller_secondary.go index d34f388a28b..c6972a992f9 100644 --- a/go-controller/pkg/ovn/base_network_controller_secondary.go +++ b/go-controller/pkg/ovn/base_network_controller_secondary.go @@ -235,7 +235,12 @@ func (bsnc *BaseSecondaryNetworkController) ensurePodForSecondaryNetwork(pod *ka return err } - on, networkMap, err := util.GetPodNADToNetworkMapping(pod, bsnc.NetInfo) + activeNetwork, err := bsnc.getActiveNetworkForNamespace(pod.Namespace) + if err != nil { + return fmt.Errorf("failed looking for the active network at namespace '%s': %w", pod.Namespace, err) + } + + on, networkMap, err := util.GetPodNADToNetworkMappingWithActiveNetwork(pod, bsnc.NetInfo, activeNetwork) if err != nil { // configuration error, no need to retry, do not return error klog.Errorf("Error getting network-attachment for pod %s/%s network %s: %v", @@ -409,12 +414,18 @@ func (bsnc *BaseSecondaryNetworkController) removePodForSecondaryNetwork(pod *ka if portInfoMap == nil { portInfoMap = map[string]*lpInfo{} } + + activeNetwork, err := bsnc.getActiveNetworkForNamespace(pod.Namespace) + if err != nil { + return fmt.Errorf("failed looking for the active network at namespace '%s': %w", pod.Namespace, err) + } + for nadName := range podNetworks { if !bsnc.HasNAD(nadName) { continue } - _, networkMap, err := util.GetPodNADToNetworkMapping(pod, bsnc.NetInfo) + _, networkMap, err := util.GetPodNADToNetworkMappingWithActiveNetwork(pod, bsnc.NetInfo, activeNetwork) if err != nil { return err } @@ -487,7 +498,13 @@ func (bsnc *BaseSecondaryNetworkController) syncPodsForSecondaryNetwork(pods []i if !ok { return fmt.Errorf("spurious object in syncPods: %v", podInterface) } - on, networkMap, err := util.GetPodNADToNetworkMapping(pod, bsnc.NetInfo) + + activeNetwork, err := bsnc.getActiveNetworkForNamespace(pod.Namespace) + if err != nil { + return fmt.Errorf("failed looking for the active network at namespace '%s': %w", pod.Namespace, err) + } + + on, networkMap, err := util.GetPodNADToNetworkMappingWithActiveNetwork(pod, bsnc.NetInfo, activeNetwork) if err != nil || !on { if err != nil { klog.Warningf("Failed to determine if pod %s/%s needs to be plumb interface on network %s: %v", diff --git a/go-controller/pkg/testing/util.go b/go-controller/pkg/testing/util.go index 8eed33bf953..b0b22f4d4e8 100644 --- a/go-controller/pkg/testing/util.go +++ b/go-controller/pkg/testing/util.go @@ -8,7 +8,7 @@ import ( ) func GenerateNAD(networkName, name, namespace, topology, cidr, role string) *nadapi.NetworkAttachmentDefinition { - nadSpec := fmt.Sprintf( + return GenerateNADWithConfig(name, namespace, fmt.Sprintf( ` { "cniVersion": "0.4.0", @@ -26,8 +26,27 @@ func GenerateNAD(networkName, name, namespace, topology, cidr, role string) *nad cidr, fmt.Sprintf("%s/%s", namespace, name), role, - ) - return GenerateNADWithConfig(name, namespace, nadSpec) + )) +} +func GenerateNADWithoutMTU(networkName, name, namespace, topology, cidr, role string) *nadapi.NetworkAttachmentDefinition { + return GenerateNADWithConfig(name, namespace, fmt.Sprintf( + ` +{ + "cniVersion": "0.4.0", + "name": %q, + "type": "ovn-k8s-cni-overlay", + "topology":%q, + "subnets": %q, + "netAttachDefName": %q, + "role": %q +} +`, + networkName, + topology, + cidr, + fmt.Sprintf("%s/%s", namespace, name), + role, + )) } func GenerateNADWithConfig(name, namespace, config string) *nadapi.NetworkAttachmentDefinition { diff --git a/go-controller/pkg/types/const.go b/go-controller/pkg/types/const.go index 886ec7e5eb8..240cdf14936 100644 --- a/go-controller/pkg/types/const.go +++ b/go-controller/pkg/types/const.go @@ -5,7 +5,6 @@ import "time" const ( // Default network name DefaultNetworkName = "default" - UnknownNetworkName = "unknown" K8sPrefix = "k8s-" HybridOverlayPrefix = "int-" HybridOverlayGRSubfix = "-gr" diff --git a/go-controller/pkg/util/multi_network.go b/go-controller/pkg/util/multi_network.go index 579a79c9608..2d3d6646f31 100644 --- a/go-controller/pkg/util/multi_network.go +++ b/go-controller/pkg/util/multi_network.go @@ -662,6 +662,9 @@ func GetPodNADToNetworkMapping(pod *kapi.Pod, nInfo NetInfo) (bool, map[string]* for _, network := range allNetworks { nadName := GetNADName(network.Namespace, network.Name) if nInfo.HasNAD(nadName) { + if nInfo.IsPrimaryNetwork() { + return false, nil, fmt.Errorf("unexpected primary network %q specified with a NetworkSelectionElement %+v", nInfo.GetNetworkName(), network) + } if _, ok := networkSelections[nadName]; ok { return false, nil, fmt.Errorf("unexpected error: more than one of the same NAD %s specified for pod %s", nadName, podDesc) @@ -669,12 +672,49 @@ func GetPodNADToNetworkMapping(pod *kapi.Pod, nInfo NetInfo) (bool, map[string]* networkSelections[nadName] = network } } + if len(networkSelections) == 0 { return false, nil, nil } + return true, networkSelections, nil } +// GetPodNADToNetworkMappingWithActiveNetwork will call `GetPodNADToNetworkMapping` passing "nInfo" which correspond +// to the NetInfo representing the NAD, the resulting NetworkSelectingElements will be decorated with the ones +// from found active network +func GetPodNADToNetworkMappingWithActiveNetwork(pod *kapi.Pod, nInfo NetInfo, activeNetwork NetInfo) (bool, map[string]*nettypes.NetworkSelectionElement, error) { + on, networkSelections, err := GetPodNADToNetworkMapping(pod, nInfo) + if err != nil { + return false, nil, err + } + + if activeNetwork == nil { + return on, networkSelections, nil + } + + if activeNetwork.IsDefault() || + activeNetwork.GetNetworkName() != nInfo.GetNetworkName() || + nInfo.TopologyType() == types.LocalnetTopology { + return on, networkSelections, nil + } + + // Add the active network to the NSE map if it is configured + activeNetworkNADs := activeNetwork.GetNADs() + if len(activeNetworkNADs) < 1 { + return false, nil, fmt.Errorf("missing NADs at active network '%s' for namesapce '%s'", activeNetwork.GetNetworkName(), pod.Namespace) + } + activeNetworkNADKey := strings.Split(activeNetworkNADs[0], "/") + if len(networkSelections) == 0 { + networkSelections = map[string]*nettypes.NetworkSelectionElement{} + } + networkSelections[activeNetworkNADs[0]] = &nettypes.NetworkSelectionElement{ + Namespace: activeNetworkNADKey[0], + Name: activeNetworkNADKey[1], + } + + return true, networkSelections, nil +} func IsMultiNetworkPoliciesSupportEnabled() bool { return config.OVNKubernetesFeature.EnableMultiNetwork && config.OVNKubernetesFeature.EnableMultiNetworkPolicy } diff --git a/go-controller/pkg/util/util.go b/go-controller/pkg/util/util.go index 0f34360b5b1..4c2a9d7446b 100644 --- a/go-controller/pkg/util/util.go +++ b/go-controller/pkg/util/util.go @@ -341,41 +341,64 @@ func IsClusterIP(svcVIP string) bool { return false } -// GetActiveNetworkForNamespace returns the active network for the given namespace -// based on the NADs present in that namespace. +type UnknownActiveNetworkError struct { + namespace string +} + +func (m UnknownActiveNetworkError) Error() string { + return fmt.Sprintf("unable to determine what is the "+ + "primary role network for namespace '%s'; please remove multiple primary role network"+ + "NADs from it", m.namespace) +} + +func IsUnknownActiveNetworkError(err error) bool { + return errors.As(err, &UnknownActiveNetworkError{}) +} + +// GetActiveNetworkForNamespace returns the NetInfo struct of the active network +// for the given namespace based on the NADs present in that namespace. // active network here means the network managing this namespace and responsible for // plumbing all the entities for this namespace // this is: -// 1) "default" if there are no NADs in the namespace OR all NADs are primaryNetwork:false -// 2) "" if there is exactly ONE NAD with primaryNetwork:true -// 3) "unknown" under all other conditions -func GetActiveNetworkForNamespace(namespace string, nadLister nadlister.NetworkAttachmentDefinitionLister) (string, error) { - var activeNetwork string +// 1) &DefaultNetInfo if there are no NADs in the namespace OR all NADs are Role: "primary" +// 2) &NetConf{Name: ""} if there is exactly ONE NAD with Role: "primary" +// 3) Multiple primary network role NADs ActiveNetworkUnknown error +// 4) error under all other conditions +func GetActiveNetworkForNamespace(namespace string, nadLister nadlister.NetworkAttachmentDefinitionLister) (NetInfo, error) { + if nadLister == nil { + return &DefaultNetInfo{}, nil + } + if !IsNetworkSegmentationSupportEnabled() { + return &DefaultNetInfo{}, nil + } namespaceNADs, err := nadLister.NetworkAttachmentDefinitions(namespace).List(labels.Everything()) if err != nil { - return activeNetwork, err + return nil, err } if len(namespaceNADs) == 0 { - return types.DefaultNetworkName, nil + return &DefaultNetInfo{}, nil } numberOfPrimaryNetworks := 0 + var primaryNetwork NetInfo for _, nad := range namespaceNADs { - nadInfo, err := ParseNADInfo(nad) + netInfo, err := ParseNADInfo(nad) if err != nil { klog.Warningf("Skipping nad '%s/%s' as active network after failing parsing it with %v", nad.Namespace, nad.Name, err) continue } - if nadInfo.IsPrimaryNetwork() { - activeNetwork = nadInfo.GetNetworkName() + + if netInfo.IsPrimaryNetwork() { + primaryNetwork = netInfo numberOfPrimaryNetworks++ + primaryNetwork.AddNADs(GetNADName(nad.Namespace, nad.Name)) } } if numberOfPrimaryNetworks == 1 { - return activeNetwork, nil + return primaryNetwork, nil } else if numberOfPrimaryNetworks == 0 { - return types.DefaultNetworkName, nil + return &DefaultNetInfo{}, nil } - return types.UnknownNetworkName, nil + return nil, &UnknownActiveNetworkError{namespace: namespace} } func GetSecondaryNetworkLogicalPortName(podNamespace, podName, nadName string) string { diff --git a/go-controller/pkg/util/util_unit_test.go b/go-controller/pkg/util/util_unit_test.go index a9652229cf0..d0aab23711d 100644 --- a/go-controller/pkg/util/util_unit_test.go +++ b/go-controller/pkg/util/util_unit_test.go @@ -10,9 +10,11 @@ import ( "testing" nadapi "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" ovntest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/sets" v1nadmocks "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/mocks/github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/listers/k8s.cni.cncf.io/v1" mock_k8s_io_utils_exec "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/mocks/k8s.io/utils/exec" @@ -246,11 +248,13 @@ func TestFilterIPsSlice(t *testing.T) { func TestGetActiveNetworkForNamespace(t *testing.T) { + config.OVNKubernetesFeature.EnableMultiNetwork = true + config.OVNKubernetesFeature.EnableNetworkSegmentation = true var tests = []struct { name string nads []*nadapi.NetworkAttachmentDefinition namespace string - expectedActiveNetwork string + expectedActiveNetwork NetInfo expectedErr error }{ { @@ -261,9 +265,9 @@ func TestGetActiveNetworkForNamespace(t *testing.T) { ovntest.GenerateNAD("surya", "miguel", "default", types.Layer2Topology, "10.100.200.0/24", types.NetworkRolePrimary), }, - expectedErr: nil, + expectedErr: &UnknownActiveNetworkError{namespace: "default"}, namespace: "default", - expectedActiveNetwork: types.UnknownNetworkName, + expectedActiveNetwork: nil, }, { name: "0 NADs found in the provided namespace", @@ -275,7 +279,7 @@ func TestGetActiveNetworkForNamespace(t *testing.T) { }, expectedErr: nil, namespace: "default", - expectedActiveNetwork: types.DefaultNetworkName, + expectedActiveNetwork: &DefaultNetInfo{}, }, { name: "exactly 1 primary NAD found in the provided namespace", @@ -292,28 +296,39 @@ func TestGetActiveNetworkForNamespace(t *testing.T) { } `), }, - expectedErr: nil, - namespace: "ns1", - expectedActiveNetwork: "surya", + expectedErr: nil, + namespace: "ns1", + expectedActiveNetwork: &secondaryNetInfo{ + netName: "surya", + primaryNetwork: true, + topology: "layer3", + nadNames: sets.New("ns1/quique"), + mtu: 1300, + ipv4mode: true, + subnets: []config.CIDRNetworkEntry{{ + CIDR: ovntest.MustParseIPNet("100.128.0.0/16"), + HostSubnetLength: 24, + }}, + }, }, { name: "no NADs found in provided namespace", nads: []*nadapi.NetworkAttachmentDefinition{}, expectedErr: nil, namespace: "default", - expectedActiveNetwork: types.DefaultNetworkName, + expectedActiveNetwork: &DefaultNetInfo{}, }, { name: "no primary NADs found in the provided namespace", nads: []*nadapi.NetworkAttachmentDefinition{ ovntest.GenerateNAD("quique", "miguel", "default", - types.Layer3Topology, "100.128.0.0/16", types.NetworkRoleSecondary), + types.Layer3Topology, "100.128.0.0/16/24", types.NetworkRoleSecondary), ovntest.GenerateNAD("quique", "miguel", "default", types.Layer2Topology, "10.100.200.0/24", types.NetworkRoleSecondary), }, expectedErr: nil, namespace: "default", - expectedActiveNetwork: types.DefaultNetworkName, + expectedActiveNetwork: &DefaultNetInfo{}, }, } diff --git a/test/e2e/multihoming_utils.go b/test/e2e/multihoming_utils.go index 5fa7c08d95c..aff5c0eddbb 100644 --- a/test/e2e/multihoming_utils.go +++ b/test/e2e/multihoming_utils.go @@ -118,7 +118,9 @@ type podConfiguration struct { func generatePodSpec(config podConfiguration) *v1.Pod { podSpec := e2epod.NewAgnhostPod(config.namespace, config.name, nil, nil, nil, config.containerCmd...) - podSpec.Annotations = networkSelectionElements(config.attachments...) + if len(config.attachments) > 0 { + podSpec.Annotations = networkSelectionElements(config.attachments...) + } podSpec.Spec.NodeSelector = config.nodeSelector podSpec.Labels = config.labels if config.isPrivileged { diff --git a/test/e2e/network_segmentation.go b/test/e2e/network_segmentation.go index 995ac4e19bb..8d4a5d043af 100644 --- a/test/e2e/network_segmentation.go +++ b/test/e2e/network_segmentation.go @@ -7,8 +7,8 @@ import ( "time" iputils "github.com/containernetworking/plugins/pkg/ip" - nadapi "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" nadclient "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/clientset/versioned/typed/k8s.cni.cncf.io/v1" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" @@ -110,12 +110,10 @@ var _ = Describe("Network Segmentation", func() { }, *podConfig( "client-pod", - nadName, withNodeSelector(map[string]string{nodeHostnameKey: workerOneNodeName}), ), *podConfig( "server-pod", - nadName, withCommand(func() []string { return httpServerContainerCmd(port) }), @@ -132,12 +130,10 @@ var _ = Describe("Network Segmentation", func() { }, *podConfig( "client-pod", - nadName, withNodeSelector(map[string]string{nodeHostnameKey: workerOneNodeName}), ), *podConfig( "server-pod", - nadName, withCommand(func() []string { return httpServerContainerCmd(port) }), @@ -334,7 +330,6 @@ var _ = Describe("Network Segmentation", func() { }, *podConfig( "udn-pod", - nadName, withCommand(func() []string { return httpServerContainerCmd(port) }), @@ -351,7 +346,6 @@ var _ = Describe("Network Segmentation", func() { }, *podConfig( "udn-pod", - nadName, withCommand(func() []string { return httpServerContainerCmd(port) }), @@ -364,10 +358,9 @@ var _ = Describe("Network Segmentation", func() { type podOption func(*podConfiguration) -func podConfig(podName, nadName string, opts ...podOption) *podConfiguration { +func podConfig(podName string, opts ...podOption) *podConfiguration { pod := &podConfiguration{ - attachments: []nadapi.NetworkSelectionElement{{Name: nadName}}, - name: podName, + name: podName, } for _, opt := range opts { opt(pod)