Merge remote-tracking branch 'upstream/master' into d/s-merge-08-25-2025

Author: origin-release-container

go-controller/pkg/clustermanager/userdefinednetwork/controller_helper.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"strings"
 
 	netv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 
@@ -76,12 +77,24 @@ func (c *Controller) updateNAD(obj client.Object, namespace string) (*netv1.Netw
 		return nil, fmt.Errorf("foreign NetworkAttachmentDefinition with the desired name already exist [%s/%s]", nadCopy.Namespace, nadCopy.Name)
 	}
 
-	if reflect.DeepEqual(nadCopy.Spec.Config, desiredNAD.Spec.Config) && reflect.DeepEqual(nadCopy.ObjectMeta.Labels, desiredNAD.ObjectMeta.Labels) {
+	// NAD update path, need to merge internal (k8s.ovn.org) current annotations with desired
+	for k, v := range nadCopy.Annotations {
+		if strings.HasPrefix(k, types.OvnK8sPrefix) {
+			if desiredNAD.Annotations == nil {
+				desiredNAD.Annotations = make(map[string]string)
+			}
+			desiredNAD.Annotations[k] = v
+		}
+	}
+
+	if reflect.DeepEqual(nadCopy.Spec.Config, desiredNAD.Spec.Config) && reflect.DeepEqual(nadCopy.ObjectMeta.Labels, desiredNAD.ObjectMeta.Labels) &&
+		reflect.DeepEqual(desiredNAD.Annotations, nadCopy.Annotations) {
 		return nadCopy, nil
 	}
 
 	nadCopy.Spec.Config = desiredNAD.Spec.Config
 	nadCopy.ObjectMeta.Labels = desiredNAD.ObjectMeta.Labels
+	nadCopy.Annotations = desiredNAD.Annotations
 	updatedNAD, err := c.nadClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(nadCopy.Namespace).Update(context.Background(), nadCopy, metav1.UpdateOptions{})
 	if err != nil {
 		return nil, fmt.Errorf("failed to update NetworkAttachmentDefinition: %w", err)

go-controller/pkg/clustermanager/userdefinednetwork/controller_test.go

@@ -445,6 +445,63 @@ var _ = Describe("User Defined Network Controller", func() {
 				}
 			})
 
+			It("should update NAD annotations and preserve internal OVNK annotations on UDN update", func() {
+				testNamespaces := []string{"red", "blue"}
+				var objs []runtime.Object
+				for _, nsName := range testNamespaces {
+					objs = append(objs, testNamespace(nsName))
+				}
+				cudn := testClusterUDN("test", testNamespaces...)
+				cudn.Spec.Network = udnv1.NetworkSpec{Topology: udnv1.NetworkTopologyLayer2, Layer2: &udnv1.Layer2Config{
+					Subnets: udnv1.DualStackCIDRs{"10.10.10.0/24"},
+				}}
+				cudn.Annotations = map[string]string{"foo": "bar"}
+
+				objs = append(objs, cudn)
+				networkName := ovntypes.CUDNPrefix + cudn.Name
+				expectedNsNADs := map[string]*netv1.NetworkAttachmentDefinition{}
+				for _, nsName := range testNamespaces {
+					nad := testClusterUdnNAD(cudn.Name, nsName)
+					nadName := nsName + "/" + cudn.Name
+					nad.Spec.Config = `{"cniVersion":"1.0.0","name":"` + networkName + `","netAttachDefName":"` + nadName + `","role":"","subnets":"10.10.10.0/24","topology":"layer2","type":"ovn-k8s-cni-overlay"}`
+					nad.Annotations = map[string]string{
+						"foo":                             "bar",
+						ovntypes.OvnNetworkNameAnnotation: networkName,
+						ovntypes.OvnNetworkIDAnnotation:   "6",
+					}
+					expectedNsNADs[nsName] = nad.DeepCopy()
+					objs = append(objs, nad)
+				}
+
+				c = newTestController(template.RenderNetAttachDefManifest, objs...)
+				Expect(c.Run()).To(Succeed())
+
+				By("updating CUDN with a new annotation")
+				cudn, err := cs.UserDefinedNetworkClient.K8sV1().ClusterUserDefinedNetworks().Get(context.Background(), cudn.Name, metav1.GetOptions{})
+				Expect(err).NotTo(HaveOccurred())
+				updatedCUDN := cudn.DeepCopy()
+				updatedCUDN.Annotations = map[string]string{"foo2": "bar2"}
+				_, err = cs.UserDefinedNetworkClient.K8sV1().ClusterUserDefinedNetworks().Update(context.Background(), updatedCUDN, metav1.UpdateOptions{})
+				Expect(err).NotTo(HaveOccurred())
+
+				for testNamespace, expectedNAD := range expectedNsNADs {
+					expectedNAD.Annotations = map[string]string{
+						"foo2":                            "bar2",
+						ovntypes.OvnNetworkNameAnnotation: networkName,
+						ovntypes.OvnNetworkIDAnnotation:   "6",
+					}
+
+					Eventually(func(g Gomega) {
+						actualNAD, err := cs.NetworkAttchDefClient.K8sCniCncfIoV1().
+							NetworkAttachmentDefinitions(testNamespace).
+							Get(context.Background(), cudn.Name, metav1.GetOptions{})
+						g.Expect(err).NotTo(HaveOccurred())
+						g.Expect(actualNAD).To(Equal(expectedNAD), "NAD should exist, have updated "+
+							"annotations, and preserve internal annotations")
+					}).Should(Succeed())
+				}
+			})
+
 			When("CR exist, and few connected & disconnected namespaces", func() {
 				const (
 					cudnName       = "global-network"

go-controller/pkg/cni/cni.go

@@ -238,7 +238,7 @@ func (pr *PodRequest) cmdAddWithGetCNIResultFunc(
 			return nil, err
 		}
 		if primaryUDNPodRequest != nil {
-			err = primaryUDNCmdAddGetCNIResultFunc(response, getCNIResultFn, primaryUDNPodRequest, clientset, primaryUDNPodInfo)
+			err = primaryUDNCmdAddGetCNIResultFunc(response.Result, getCNIResultFn, primaryUDNPodRequest, clientset, primaryUDNPodInfo)
 			if err != nil {
 				return nil, err
 			}
@@ -254,24 +254,24 @@ func (pr *PodRequest) cmdAddWithGetCNIResultFunc(
 	return response, nil
 }
 
-func primaryUDNCmdAddGetCNIResultFunc(response *Response, getCNIResultFn getCNIResultFunc, primaryUDNPodRequest *PodRequest,
+func primaryUDNCmdAddGetCNIResultFunc(result *current.Result, getCNIResultFn getCNIResultFunc, primaryUDNPodRequest *PodRequest,
 	clientset PodInfoGetter, primaryUDNPodInfo *PodInterfaceInfo) error {
 	primaryUDNResult, err := getCNIResultFn(primaryUDNPodRequest, clientset, primaryUDNPodInfo)
 	if err != nil {
 		return err
 	}
 
-	response.Result.Routes = append(response.Result.Routes, primaryUDNResult.Routes...)
-	numOfInitialIPs := len(response.Result.IPs)
-	numOfInitialIfaces := len(response.Result.Interfaces)
-	response.Result.Interfaces = append(response.Result.Interfaces, primaryUDNResult.Interfaces...)
-	response.Result.IPs = append(response.Result.IPs, primaryUDNResult.IPs...)
+	result.Routes = append(result.Routes, primaryUDNResult.Routes...)
+	numOfInitialIPs := len(result.IPs)
+	numOfInitialIfaces := len(result.Interfaces)
+	result.Interfaces = append(result.Interfaces, primaryUDNResult.Interfaces...)
+	result.IPs = append(result.IPs, primaryUDNResult.IPs...)
 
 	// Offset the index of the default network IPs to correctly point to the default network interfaces
-	for i := numOfInitialIPs; i < len(response.Result.IPs); i++ {
-		ifaceIPConfig := response.Result.IPs[i].Copy()
-		if response.Result.IPs[i].Interface != nil {
-			response.Result.IPs[i].Interface = current.Int(*ifaceIPConfig.Interface + numOfInitialIfaces)
+	for i := numOfInitialIPs; i < len(result.IPs); i++ {
+		ifaceIPConfig := result.IPs[i].Copy()
+		if result.IPs[i].Interface != nil {
+			result.IPs[i].Interface = current.Int(*ifaceIPConfig.Interface + numOfInitialIfaces)
 		}
 	}
 	return nil

go-controller/pkg/cni/cnishim.go

@@ -266,7 +266,7 @@ func (p *Plugin) CmdAdd(args *skel.CmdArgs) error {
 			primaryUDNPodRequest := response.PrimaryUDNPodReq
 			primaryUDNPodRequest.ctx, primaryUDNPodRequest.cancel = context.WithCancel(pr.ctx)
 			defer primaryUDNPodRequest.cancel()
-			err = primaryUDNCmdAddGetCNIResultFunc(response, getCNIResult, primaryUDNPodRequest, clientset, response.PrimaryUDNPodInfo)
+			err = primaryUDNCmdAddGetCNIResultFunc(result, getCNIResult, primaryUDNPodRequest, clientset, response.PrimaryUDNPodInfo)
 			if err != nil {
 				klog.Error(err.Error())
 				return err

go-controller/pkg/cni/udn/resource.go

@@ -20,12 +20,11 @@ func getPodResourceInfo(pod *corev1.Pod, resourceName string) (*types.ResourceIn
 	if err != nil {
 		return nil, fmt.Errorf("failed to get resources allocated for pod %s from ResourceClient: %v", podDesc, err)
 	}
-	klog.V(5).Infof("ResourceMap for pod %s: %+v", pod, resourceMap)
-
 	entry, ok := resourceMap[resourceName]
 	if !ok {
 		return nil, fmt.Errorf("failed to get resources allocated for pod %s: no resources for resource %s", podDesc, resourceName)
 	}
+	klog.V(5).Infof("ResourceMap for pod %s resource %s: %+v", podDesc, resourceName, entry)
 	return entry, nil
 }
 

go-controller/pkg/ovn/master_test.go

@@ -1566,45 +1566,62 @@ var _ = ginkgo.Describe("Default network controller operations", func() {
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
 	})
 
-	ginkgo.It("doesn't retry deleting a node that doesn't have a node-subnet annotation", func() {
-		app.Action = func(ctx *cli.Context) error {
-			_, err := config.InitConfig(ctx, nil, nil)
+	ginkgo.DescribeTable("doesn't retry deleting a node that is missing annotation",
+		func(node *corev1.Node) {
+			app.Action = func(ctx *cli.Context) error {
+				_, err := config.InitConfig(ctx, nil, nil)
+				gomega.Expect(err).NotTo(gomega.HaveOccurred())
+				startFakeController(oc, wg)
+				ginkgo.By("create new node with no annotation defined and ensure there's a retry entry")
+				_, err = kubeFakeClient.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{})
+				gomega.Expect(err).NotTo(gomega.HaveOccurred())
+				retry.CheckRetryObjectMultipleFieldsEventually(
+					node.Name,
+					oc.retryNodes,
+					gomega.BeNil(),             // oldObj should be nil
+					gomega.Not(gomega.BeNil()), // newObj should not be nil
+				)
+				keyExists := true
+				retry.CheckRetryObjectEventually(node.Name, keyExists, oc.retryNodes)
+				ginkgo.By("delete node and check that there are no retries for the deleted node")
+				err = kubeFakeClient.CoreV1().Nodes().Delete(context.TODO(), node.Name, metav1.DeleteOptions{})
+				gomega.Expect(err).NotTo(gomega.HaveOccurred())
+				retry.CheckRetryObjectEventually(node.Name, !keyExists, oc.retryNodes)
+				ginkgo.By("ensure failures for sync host network address or adding nodes are cleared")
+				gomega.Eventually(func() bool {
+					_, foundSyncHostNetAddrSetFailed := oc.syncHostNetAddrSetFailed.Load(node.Name)
+					_, foundAddNodeFailed := oc.addNodeFailed.Load(node.Name)
+					return foundSyncHostNetAddrSetFailed || foundAddNodeFailed
+				}).Should(gomega.BeFalse())
+				return nil
+			}
+			err := app.Run([]string{
+				app.Name,
+			})
 			gomega.Expect(err).NotTo(gomega.HaveOccurred())
-			startFakeController(oc, wg)
-			newNode := &corev1.Node{
+
+		},
+		ginkgo.Entry("k8s.ovn.org/node-subnets",
+			&corev1.Node{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:        "newNode",
-					Annotations: map[string]string{},
+					Name: "newNode",
+					Annotations: map[string]string{
+						"k8s.ovn.org/node-id": "2",
+					},
 				},
-			}
-			ginkgo.By("create new node with no host-subnet annotation defined and ensure theres a retry entry")
-			_, err = kubeFakeClient.CoreV1().Nodes().Create(context.TODO(), newNode, metav1.CreateOptions{})
-			gomega.Expect(err).NotTo(gomega.HaveOccurred())
-			retry.CheckRetryObjectMultipleFieldsEventually(
-				newNode.Name,
-				oc.retryNodes,
-				gomega.BeNil(),             // oldObj should be nil
-				gomega.Not(gomega.BeNil()), // newObj should not be nil
-			)
-			keyExists := true
-			retry.CheckRetryObjectEventually(newNode.Name, keyExists, oc.retryNodes)
-			ginkgo.By("delete node and check that there are no retries for the deleted node")
-			err = kubeFakeClient.CoreV1().Nodes().Delete(context.TODO(), newNode.Name, metav1.DeleteOptions{})
-			gomega.Expect(err).NotTo(gomega.HaveOccurred())
-			retry.CheckRetryObjectEventually(newNode.Name, !keyExists, oc.retryNodes)
-			ginkgo.By("ensure failures for sync host network address or adding nodes are cleared")
-			gomega.Eventually(func() bool {
-				_, foundSyncHostNetAddrSetFailed := oc.syncHostNetAddrSetFailed.Load(newNode.Name)
-				_, foundAddNodeFailed := oc.addNodeFailed.Load(newNode.Name)
-				return foundSyncHostNetAddrSetFailed || foundAddNodeFailed
-			}).Should(gomega.BeFalse())
-			return nil
-		}
-		err := app.Run([]string{
-			app.Name,
-		})
-		gomega.Expect(err).NotTo(gomega.HaveOccurred())
-	})
+			},
+		),
+		ginkgo.Entry("k8s.ovn.org/node-id",
+			&corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "newNode",
+					Annotations: map[string]string{
+						"k8s.ovn.org/node-subnets": "{\"default\": [\"10.130.0.0/23\", \"fd01:0:0:2::/64\"]}",
+					},
+				},
+			},
+		),
+	)
 
 	ginkgo.It("delete a partially constructed node", func() {
 		app.Action = func(ctx *cli.Context) error {

go-controller/pkg/ovn/namespace.go

@@ -376,16 +376,17 @@ func (oc *DefaultNetworkController) getHostNamespaceAddressesForNode(node *corev
 	}
 	// for shared gateway mode we will use LRP IPs to SNAT host network traffic
 	// so add these to the address set.
-	lrpIPs, err := udn.GetGWRouterIPs(node, oc.GetNetInfo())
-	if err != nil {
-		if util.IsAnnotationNotSetError(err) {
-			// FIXME(tssurya): This is present for backwards compatibility
-			// Remove me a few months from now
-			var err1 error
-			lrpIPs, err1 = util.ParseNodeGatewayRouterLRPAddrs(node)
-			if err1 != nil {
-				return nil, fmt.Errorf("failed to get join switch port IP address for node %s: %v/%v", node.Name, err, err1)
-			}
+	lrpIPs, gwIPsErr := udn.GetGWRouterIPs(node, oc.GetNetInfo())
+	if gwIPsErr != nil {
+		if !util.IsAnnotationNotSetError(gwIPsErr) {
+			return nil, gwIPsErr
+		}
+		// FIXME(tssurya): This is present for backwards compatibility
+		// Remove me a few months from now
+		var lrpAddrsErr error
+		lrpIPs, lrpAddrsErr = util.ParseNodeGatewayRouterLRPAddrs(node)
+		if lrpAddrsErr != nil {
+			return nil, fmt.Errorf("failed to fallback to annotations after error %q: %w", gwIPsErr, lrpAddrsErr)
 		}
 	}