Merge pull request #5507 from trozet/udn_update_annos_labels

Ensure that UDN update events trigger updates for NAD annotations correctly

Author: trozet

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"