Skip to content

Commit 9741174

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #2734 from openshift/d/s-merge-08-25-2025
OCPBUGS-60844: DownStream Merge [08-25-2025]
2 parents cbb5de7 + 4969fd3 commit 9741174

File tree

7 files changed

+148
-61
lines changed

7 files changed

+148
-61
lines changed

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"reflect"
7+
"strings"
78

89
netv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
910

@@ -76,12 +77,24 @@ func (c *Controller) updateNAD(obj client.Object, namespace string) (*netv1.Netw
7677
return nil, fmt.Errorf("foreign NetworkAttachmentDefinition with the desired name already exist [%s/%s]", nadCopy.Namespace, nadCopy.Name)
7778
}
7879

79-
if reflect.DeepEqual(nadCopy.Spec.Config, desiredNAD.Spec.Config) && reflect.DeepEqual(nadCopy.ObjectMeta.Labels, desiredNAD.ObjectMeta.Labels) {
80+
// NAD update path, need to merge internal (k8s.ovn.org) current annotations with desired
81+
for k, v := range nadCopy.Annotations {
82+
if strings.HasPrefix(k, types.OvnK8sPrefix) {
83+
if desiredNAD.Annotations == nil {
84+
desiredNAD.Annotations = make(map[string]string)
85+
}
86+
desiredNAD.Annotations[k] = v
87+
}
88+
}
89+
90+
if reflect.DeepEqual(nadCopy.Spec.Config, desiredNAD.Spec.Config) && reflect.DeepEqual(nadCopy.ObjectMeta.Labels, desiredNAD.ObjectMeta.Labels) &&
91+
reflect.DeepEqual(desiredNAD.Annotations, nadCopy.Annotations) {
8092
return nadCopy, nil
8193
}
8294

8395
nadCopy.Spec.Config = desiredNAD.Spec.Config
8496
nadCopy.ObjectMeta.Labels = desiredNAD.ObjectMeta.Labels
97+
nadCopy.Annotations = desiredNAD.Annotations
8598
updatedNAD, err := c.nadClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(nadCopy.Namespace).Update(context.Background(), nadCopy, metav1.UpdateOptions{})
8699
if err != nil {
87100
return nil, fmt.Errorf("failed to update NetworkAttachmentDefinition: %w", err)

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,63 @@ var _ = Describe("User Defined Network Controller", func() {
445445
}
446446
})
447447

448+
It("should update NAD annotations and preserve internal OVNK annotations on UDN update", func() {
449+
testNamespaces := []string{"red", "blue"}
450+
var objs []runtime.Object
451+
for _, nsName := range testNamespaces {
452+
objs = append(objs, testNamespace(nsName))
453+
}
454+
cudn := testClusterUDN("test", testNamespaces...)
455+
cudn.Spec.Network = udnv1.NetworkSpec{Topology: udnv1.NetworkTopologyLayer2, Layer2: &udnv1.Layer2Config{
456+
Subnets: udnv1.DualStackCIDRs{"10.10.10.0/24"},
457+
}}
458+
cudn.Annotations = map[string]string{"foo": "bar"}
459+
460+
objs = append(objs, cudn)
461+
networkName := ovntypes.CUDNPrefix + cudn.Name
462+
expectedNsNADs := map[string]*netv1.NetworkAttachmentDefinition{}
463+
for _, nsName := range testNamespaces {
464+
nad := testClusterUdnNAD(cudn.Name, nsName)
465+
nadName := nsName + "/" + cudn.Name
466+
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"}`
467+
nad.Annotations = map[string]string{
468+
"foo": "bar",
469+
ovntypes.OvnNetworkNameAnnotation: networkName,
470+
ovntypes.OvnNetworkIDAnnotation: "6",
471+
}
472+
expectedNsNADs[nsName] = nad.DeepCopy()
473+
objs = append(objs, nad)
474+
}
475+
476+
c = newTestController(template.RenderNetAttachDefManifest, objs...)
477+
Expect(c.Run()).To(Succeed())
478+
479+
By("updating CUDN with a new annotation")
480+
cudn, err := cs.UserDefinedNetworkClient.K8sV1().ClusterUserDefinedNetworks().Get(context.Background(), cudn.Name, metav1.GetOptions{})
481+
Expect(err).NotTo(HaveOccurred())
482+
updatedCUDN := cudn.DeepCopy()
483+
updatedCUDN.Annotations = map[string]string{"foo2": "bar2"}
484+
_, err = cs.UserDefinedNetworkClient.K8sV1().ClusterUserDefinedNetworks().Update(context.Background(), updatedCUDN, metav1.UpdateOptions{})
485+
Expect(err).NotTo(HaveOccurred())
486+
487+
for testNamespace, expectedNAD := range expectedNsNADs {
488+
expectedNAD.Annotations = map[string]string{
489+
"foo2": "bar2",
490+
ovntypes.OvnNetworkNameAnnotation: networkName,
491+
ovntypes.OvnNetworkIDAnnotation: "6",
492+
}
493+
494+
Eventually(func(g Gomega) {
495+
actualNAD, err := cs.NetworkAttchDefClient.K8sCniCncfIoV1().
496+
NetworkAttachmentDefinitions(testNamespace).
497+
Get(context.Background(), cudn.Name, metav1.GetOptions{})
498+
g.Expect(err).NotTo(HaveOccurred())
499+
g.Expect(actualNAD).To(Equal(expectedNAD), "NAD should exist, have updated "+
500+
"annotations, and preserve internal annotations")
501+
}).Should(Succeed())
502+
}
503+
})
504+
448505
When("CR exist, and few connected & disconnected namespaces", func() {
449506
const (
450507
cudnName = "global-network"

‎go-controller/pkg/cni/cni.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ func (pr *PodRequest) cmdAddWithGetCNIResultFunc(
238238
return nil, err
239239
}
240240
if primaryUDNPodRequest != nil {
241-
err = primaryUDNCmdAddGetCNIResultFunc(response, getCNIResultFn, primaryUDNPodRequest, clientset, primaryUDNPodInfo)
241+
err = primaryUDNCmdAddGetCNIResultFunc(response.Result, getCNIResultFn, primaryUDNPodRequest, clientset, primaryUDNPodInfo)
242242
if err != nil {
243243
return nil, err
244244
}
@@ -254,24 +254,24 @@ func (pr *PodRequest) cmdAddWithGetCNIResultFunc(
254254
return response, nil
255255
}
256256

257-
func primaryUDNCmdAddGetCNIResultFunc(response *Response, getCNIResultFn getCNIResultFunc, primaryUDNPodRequest *PodRequest,
257+
func primaryUDNCmdAddGetCNIResultFunc(result *current.Result, getCNIResultFn getCNIResultFunc, primaryUDNPodRequest *PodRequest,
258258
clientset PodInfoGetter, primaryUDNPodInfo *PodInterfaceInfo) error {
259259
primaryUDNResult, err := getCNIResultFn(primaryUDNPodRequest, clientset, primaryUDNPodInfo)
260260
if err != nil {
261261
return err
262262
}
263263

264-
response.Result.Routes = append(response.Result.Routes, primaryUDNResult.Routes...)
265-
numOfInitialIPs := len(response.Result.IPs)
266-
numOfInitialIfaces := len(response.Result.Interfaces)
267-
response.Result.Interfaces = append(response.Result.Interfaces, primaryUDNResult.Interfaces...)
268-
response.Result.IPs = append(response.Result.IPs, primaryUDNResult.IPs...)
264+
result.Routes = append(result.Routes, primaryUDNResult.Routes...)
265+
numOfInitialIPs := len(result.IPs)
266+
numOfInitialIfaces := len(result.Interfaces)
267+
result.Interfaces = append(result.Interfaces, primaryUDNResult.Interfaces...)
268+
result.IPs = append(result.IPs, primaryUDNResult.IPs...)
269269

270270
// Offset the index of the default network IPs to correctly point to the default network interfaces
271-
for i := numOfInitialIPs; i < len(response.Result.IPs); i++ {
272-
ifaceIPConfig := response.Result.IPs[i].Copy()
273-
if response.Result.IPs[i].Interface != nil {
274-
response.Result.IPs[i].Interface = current.Int(*ifaceIPConfig.Interface + numOfInitialIfaces)
271+
for i := numOfInitialIPs; i < len(result.IPs); i++ {
272+
ifaceIPConfig := result.IPs[i].Copy()
273+
if result.IPs[i].Interface != nil {
274+
result.IPs[i].Interface = current.Int(*ifaceIPConfig.Interface + numOfInitialIfaces)
275275
}
276276
}
277277
return nil

‎go-controller/pkg/cni/cnishim.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ func (p *Plugin) CmdAdd(args *skel.CmdArgs) error {
266266
primaryUDNPodRequest := response.PrimaryUDNPodReq
267267
primaryUDNPodRequest.ctx, primaryUDNPodRequest.cancel = context.WithCancel(pr.ctx)
268268
defer primaryUDNPodRequest.cancel()
269-
err = primaryUDNCmdAddGetCNIResultFunc(response, getCNIResult, primaryUDNPodRequest, clientset, response.PrimaryUDNPodInfo)
269+
err = primaryUDNCmdAddGetCNIResultFunc(result, getCNIResult, primaryUDNPodRequest, clientset, response.PrimaryUDNPodInfo)
270270
if err != nil {
271271
klog.Error(err.Error())
272272
return err

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,11 @@ func getPodResourceInfo(pod *corev1.Pod, resourceName string) (*types.ResourceIn
2020
if err != nil {
2121
return nil, fmt.Errorf("failed to get resources allocated for pod %s from ResourceClient: %v", podDesc, err)
2222
}
23-
klog.V(5).Infof("ResourceMap for pod %s: %+v", pod, resourceMap)
24-
2523
entry, ok := resourceMap[resourceName]
2624
if !ok {
2725
return nil, fmt.Errorf("failed to get resources allocated for pod %s: no resources for resource %s", podDesc, resourceName)
2826
}
27+
klog.V(5).Infof("ResourceMap for pod %s resource %s: %+v", podDesc, resourceName, entry)
2928
return entry, nil
3029
}
3130

‎go-controller/pkg/ovn/master_test.go‎

Lines changed: 53 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,45 +1566,62 @@ var _ = ginkgo.Describe("Default network controller operations", func() {
15661566
gomega.Expect(err).NotTo(gomega.HaveOccurred())
15671567
})
15681568

1569-
ginkgo.It("doesn't retry deleting a node that doesn't have a node-subnet annotation", func() {
1570-
app.Action = func(ctx *cli.Context) error {
1571-
_, err := config.InitConfig(ctx, nil, nil)
1569+
ginkgo.DescribeTable("doesn't retry deleting a node that is missing annotation",
1570+
func(node *corev1.Node) {
1571+
app.Action = func(ctx *cli.Context) error {
1572+
_, err := config.InitConfig(ctx, nil, nil)
1573+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
1574+
startFakeController(oc, wg)
1575+
ginkgo.By("create new node with no annotation defined and ensure there's a retry entry")
1576+
_, err = kubeFakeClient.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{})
1577+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
1578+
retry.CheckRetryObjectMultipleFieldsEventually(
1579+
node.Name,
1580+
oc.retryNodes,
1581+
gomega.BeNil(), // oldObj should be nil
1582+
gomega.Not(gomega.BeNil()), // newObj should not be nil
1583+
)
1584+
keyExists := true
1585+
retry.CheckRetryObjectEventually(node.Name, keyExists, oc.retryNodes)
1586+
ginkgo.By("delete node and check that there are no retries for the deleted node")
1587+
err = kubeFakeClient.CoreV1().Nodes().Delete(context.TODO(), node.Name, metav1.DeleteOptions{})
1588+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
1589+
retry.CheckRetryObjectEventually(node.Name, !keyExists, oc.retryNodes)
1590+
ginkgo.By("ensure failures for sync host network address or adding nodes are cleared")
1591+
gomega.Eventually(func() bool {
1592+
_, foundSyncHostNetAddrSetFailed := oc.syncHostNetAddrSetFailed.Load(node.Name)
1593+
_, foundAddNodeFailed := oc.addNodeFailed.Load(node.Name)
1594+
return foundSyncHostNetAddrSetFailed || foundAddNodeFailed
1595+
}).Should(gomega.BeFalse())
1596+
return nil
1597+
}
1598+
err := app.Run([]string{
1599+
app.Name,
1600+
})
15721601
gomega.Expect(err).NotTo(gomega.HaveOccurred())
1573-
startFakeController(oc, wg)
1574-
newNode := &corev1.Node{
1602+
1603+
},
1604+
ginkgo.Entry("k8s.ovn.org/node-subnets",
1605+
&corev1.Node{
15751606
ObjectMeta: metav1.ObjectMeta{
1576-
Name: "newNode",
1577-
Annotations: map[string]string{},
1607+
Name: "newNode",
1608+
Annotations: map[string]string{
1609+
"k8s.ovn.org/node-id": "2",
1610+
},
15781611
},
1579-
}
1580-
ginkgo.By("create new node with no host-subnet annotation defined and ensure theres a retry entry")
1581-
_, err = kubeFakeClient.CoreV1().Nodes().Create(context.TODO(), newNode, metav1.CreateOptions{})
1582-
gomega.Expect(err).NotTo(gomega.HaveOccurred())
1583-
retry.CheckRetryObjectMultipleFieldsEventually(
1584-
newNode.Name,
1585-
oc.retryNodes,
1586-
gomega.BeNil(), // oldObj should be nil
1587-
gomega.Not(gomega.BeNil()), // newObj should not be nil
1588-
)
1589-
keyExists := true
1590-
retry.CheckRetryObjectEventually(newNode.Name, keyExists, oc.retryNodes)
1591-
ginkgo.By("delete node and check that there are no retries for the deleted node")
1592-
err = kubeFakeClient.CoreV1().Nodes().Delete(context.TODO(), newNode.Name, metav1.DeleteOptions{})
1593-
gomega.Expect(err).NotTo(gomega.HaveOccurred())
1594-
retry.CheckRetryObjectEventually(newNode.Name, !keyExists, oc.retryNodes)
1595-
ginkgo.By("ensure failures for sync host network address or adding nodes are cleared")
1596-
gomega.Eventually(func() bool {
1597-
_, foundSyncHostNetAddrSetFailed := oc.syncHostNetAddrSetFailed.Load(newNode.Name)
1598-
_, foundAddNodeFailed := oc.addNodeFailed.Load(newNode.Name)
1599-
return foundSyncHostNetAddrSetFailed || foundAddNodeFailed
1600-
}).Should(gomega.BeFalse())
1601-
return nil
1602-
}
1603-
err := app.Run([]string{
1604-
app.Name,
1605-
})
1606-
gomega.Expect(err).NotTo(gomega.HaveOccurred())
1607-
})
1612+
},
1613+
),
1614+
ginkgo.Entry("k8s.ovn.org/node-id",
1615+
&corev1.Node{
1616+
ObjectMeta: metav1.ObjectMeta{
1617+
Name: "newNode",
1618+
Annotations: map[string]string{
1619+
"k8s.ovn.org/node-subnets": "{\"default\": [\"10.130.0.0/23\", \"fd01:0:0:2::/64\"]}",
1620+
},
1621+
},
1622+
},
1623+
),
1624+
)
16081625

16091626
ginkgo.It("delete a partially constructed node", func() {
16101627
app.Action = func(ctx *cli.Context) error {

‎go-controller/pkg/ovn/namespace.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -376,16 +376,17 @@ func (oc *DefaultNetworkController) getHostNamespaceAddressesForNode(node *corev
376376
}
377377
// for shared gateway mode we will use LRP IPs to SNAT host network traffic
378378
// so add these to the address set.
379-
lrpIPs, err := udn.GetGWRouterIPs(node, oc.GetNetInfo())
380-
if err != nil {
381-
if util.IsAnnotationNotSetError(err) {
382-
// FIXME(tssurya): This is present for backwards compatibility
383-
// Remove me a few months from now
384-
var err1 error
385-
lrpIPs, err1 = util.ParseNodeGatewayRouterLRPAddrs(node)
386-
if err1 != nil {
387-
return nil, fmt.Errorf("failed to get join switch port IP address for node %s: %v/%v", node.Name, err, err1)
388-
}
379+
lrpIPs, gwIPsErr := udn.GetGWRouterIPs(node, oc.GetNetInfo())
380+
if gwIPsErr != nil {
381+
if !util.IsAnnotationNotSetError(gwIPsErr) {
382+
return nil, gwIPsErr
383+
}
384+
// FIXME(tssurya): This is present for backwards compatibility
385+
// Remove me a few months from now
386+
var lrpAddrsErr error
387+
lrpIPs, lrpAddrsErr = util.ParseNodeGatewayRouterLRPAddrs(node)
388+
if lrpAddrsErr != nil {
389+
return nil, fmt.Errorf("failed to fallback to annotations after error %q: %w", gwIPsErr, lrpAddrsErr)
389390
}
390391
}
391392

0 commit comments

Comments
 (0)