Skip to content

Commit 0768e7b

Browse files
girishmggirishmg
authored
Merge pull request #5103 from crnithya/ic_encap_fix
 OVN IC mode should handle user provided encapIPs
2 parents 76bf584 + 508747e commit 0768e7b

File tree

9 files changed

+317
-41
lines changed

9 files changed

+317
-41
lines changed

‎go-controller/pkg/libovsdb/libovsdb.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,6 @@ func NewSBClientWithConfig(cfg config.OvnAuthConfig, promRegistry prometheus.Reg
140140
enableMetricsOption := client.WithMetricsRegistryNamespaceSubsystem(promRegistry,
141141
"ovnkube", "master_libovsdb")
142142

143-
dbModel.SetIndexes(map[string][]model.ClientIndex{
144-
sbdb.EncapTable: {{Columns: []model.ColumnKey{{Column: "chassis_name"}}}},
145-
})
146-
147143
c, err := newClient(cfg, dbModel, stopCh, enableMetricsOption)
148144
if err != nil {
149145
return nil, err

‎go-controller/pkg/libovsdb/ops/chassis.go‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -138,27 +138,33 @@ func DeleteChassisWithPredicate(sbClient libovsdbclient.Client, p chassisPredica
138138
}
139139

140140
// CreateOrUpdateChassis creates or updates the chassis record along with the encap record
141-
func CreateOrUpdateChassis(sbClient libovsdbclient.Client, chassis *sbdb.Chassis, encap *sbdb.Encap) error {
141+
func CreateOrUpdateChassis(sbClient libovsdbclient.Client, chassis *sbdb.Chassis, encaps ...*sbdb.Encap) error {
142142
m := newModelClient(sbClient)
143-
opModels := []operationModel{
144-
{
143+
opModels := make([]operationModel, 0, len(encaps)+1)
144+
for i := range encaps {
145+
encap := encaps[i]
146+
opModel := operationModel{
145147
Model: encap,
146148
DoAfter: func() {
147-
chassis.Encaps = []string{encap.UUID}
149+
encapsList := append(chassis.Encaps, encap.UUID)
150+
chassis.Encaps = sets.New(encapsList...).UnsortedList()
148151
},
149-
OnModelUpdates: onModelUpdatesAllNonDefault(),
152+
OnModelUpdates: onModelUpdatesNone(),
150153
ErrNotFound: false,
151154
BulkOp: false,
152-
},
153-
{
154-
Model: chassis,
155-
OnModelMutations: []interface{}{&chassis.OtherConfig},
156-
OnModelUpdates: []interface{}{&chassis.Encaps},
157-
ErrNotFound: false,
158-
BulkOp: false,
159-
},
155+
}
156+
opModels = append(opModels, opModel)
157+
}
158+
159+
opModel := operationModel{
160+
Model: chassis,
161+
OnModelMutations: []interface{}{&chassis.OtherConfig},
162+
OnModelUpdates: []interface{}{&chassis.Encaps},
163+
ErrNotFound: false,
164+
BulkOp: false,
160165
}
161166

167+
opModels = append(opModels, opModel)
162168
if _, err := m.CreateOrUpdate(opModels...); err != nil {
163169
return err
164170
}

‎go-controller/pkg/libovsdb/ops/chassis_test.go‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,3 +181,97 @@ func TestDeleteChassis(t *testing.T) {
181181
})
182182
}
183183
}
184+
185+
func TestCreateOrUpdateChassis(t *testing.T) {
186+
uuid1 := "b9998337-2498-4d1e-86e6-fc0417abb2f0"
187+
uuid2 := "b9998337-2498-4d1e-86e6-fc0417abb2f1"
188+
uuid3 := "b9998337-2498-4d1e-86e6-fc0417abb2f2"
189+
tests := []struct {
190+
desc string
191+
chassis *sbdb.Chassis
192+
encaps []*sbdb.Encap
193+
initialDB []libovsdbtest.TestData
194+
expectedDB []libovsdbtest.TestData
195+
}{
196+
{
197+
desc: "create new chassis with encap records",
198+
chassis: &sbdb.Chassis{Name: "test1"},
199+
encaps: []*sbdb.Encap{{ChassisName: "test1", IP: "10.0.0.10", Type: "geneve"},
200+
{ChassisName: "test1", IP: "10.0.0.11", Type: "geneve"}},
201+
initialDB: []libovsdbtest.TestData{},
202+
expectedDB: []libovsdbtest.TestData{
203+
&sbdb.Chassis{UUID: uuid1, Name: "test1", Encaps: []string{uuid2, uuid3}},
204+
&sbdb.Encap{UUID: uuid2, ChassisName: "test1", IP: "10.0.0.10", Type: "geneve"},
205+
&sbdb.Encap{UUID: uuid3, ChassisName: "test1", IP: "10.0.0.11", Type: "geneve"},
206+
},
207+
},
208+
{
209+
desc: "update chassis by inserting new encap record",
210+
chassis: &sbdb.Chassis{Name: "test2"},
211+
encaps: []*sbdb.Encap{{ChassisName: "test2", IP: "10.0.0.10", Type: "geneve"},
212+
{ChassisName: "test2", IP: "10.0.0.11", Type: "geneve"}},
213+
initialDB: []libovsdbtest.TestData{
214+
&sbdb.Chassis{UUID: uuid1, Name: "test2", Encaps: []string{uuid2}},
215+
&sbdb.Encap{UUID: uuid2, ChassisName: "test2", IP: "10.0.0.10", Type: "geneve"},
216+
},
217+
expectedDB: []libovsdbtest.TestData{
218+
&sbdb.Chassis{UUID: uuid1, Name: "test2", Encaps: []string{uuid2, uuid3}},
219+
&sbdb.Encap{UUID: uuid2, ChassisName: "test2", IP: "10.0.0.10", Type: "geneve"},
220+
&sbdb.Encap{UUID: uuid3, ChassisName: "test2", IP: "10.0.0.11", Type: "geneve"},
221+
},
222+
},
223+
{
224+
desc: "update chassis by removing obsolete encap record",
225+
chassis: &sbdb.Chassis{Name: "test3"},
226+
encaps: []*sbdb.Encap{{ChassisName: "test3", IP: "10.0.0.11", Type: "geneve"}},
227+
initialDB: []libovsdbtest.TestData{
228+
&sbdb.Chassis{UUID: uuid1, Name: "test3", Encaps: []string{uuid2, uuid3}},
229+
&sbdb.Encap{UUID: uuid2, ChassisName: "test3", IP: "10.0.0.10", Type: "geneve"},
230+
&sbdb.Encap{UUID: uuid3, ChassisName: "test3", IP: "10.0.0.11", Type: "geneve"},
231+
},
232+
expectedDB: []libovsdbtest.TestData{
233+
&sbdb.Chassis{UUID: uuid1, Name: "test3", Encaps: []string{uuid3}},
234+
&sbdb.Encap{UUID: uuid3, ChassisName: "test3", IP: "10.0.0.11", Type: "geneve"},
235+
},
236+
},
237+
{
238+
desc: "update chassis by adding new encap record and deleting the old one",
239+
chassis: &sbdb.Chassis{Name: "test4"},
240+
encaps: []*sbdb.Encap{{ChassisName: "test4", IP: "10.0.0.11", Type: "geneve"}},
241+
initialDB: []libovsdbtest.TestData{
242+
&sbdb.Chassis{UUID: uuid1, Name: "test4", Encaps: []string{uuid2}},
243+
&sbdb.Encap{UUID: uuid2, ChassisName: "test4", IP: "10.0.0.10", Type: "geneve"},
244+
},
245+
expectedDB: []libovsdbtest.TestData{
246+
&sbdb.Chassis{UUID: uuid1, Name: "test4", Encaps: []string{uuid3}},
247+
&sbdb.Encap{UUID: uuid3, ChassisName: "test4", IP: "10.0.0.11", Type: "geneve"},
248+
},
249+
},
250+
}
251+
for _, tt := range tests {
252+
t.Run(tt.desc, func(t *testing.T) {
253+
dbSetup := libovsdbtest.TestSetup{
254+
SBData: tt.initialDB,
255+
}
256+
sbClient, cleanup, err := libovsdbtest.NewSBTestHarness(dbSetup, nil)
257+
if err != nil {
258+
t.Fatalf("%s: failed to set up test harness: %v", tt.desc, err)
259+
}
260+
t.Cleanup(cleanup.Cleanup)
261+
262+
err = CreateOrUpdateChassis(sbClient, tt.chassis, tt.encaps...)
263+
if err != nil {
264+
t.Fatal(fmt.Errorf("%s: got unexpected error: %v", tt.desc, err))
265+
}
266+
267+
matcher := libovsdbtest.HaveDataIgnoringUUIDs(tt.expectedDB)
268+
match, err := matcher.Match(sbClient)
269+
if err != nil {
270+
t.Fatalf("%s: matcher error: %v", tt.desc, err)
271+
}
272+
if !match {
273+
t.Fatalf("%s: DB state did not match: %s", tt.desc, matcher.FailureMessage(sbClient))
274+
}
275+
})
276+
}
277+
}

‎go-controller/pkg/node/default_node_network_controller.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
apierrors "k8s.io/apimachinery/pkg/api/errors"
2121
"k8s.io/apimachinery/pkg/labels"
2222
"k8s.io/apimachinery/pkg/selection"
23+
"k8s.io/apimachinery/pkg/util/sets"
2324
"k8s.io/apimachinery/pkg/util/wait"
2425
clientset "k8s.io/client-go/kubernetes"
2526
"k8s.io/client-go/tools/record"
@@ -941,6 +942,13 @@ func (nc *DefaultNodeNetworkController) Init(ctx context.Context) error {
941942
if err := util.SetNodeZone(nodeAnnotator, sbZone); err != nil {
942943
return fmt.Errorf("failed to set node zone annotation for node %s: %w", nc.name, err)
943944
}
945+
946+
encapIPList := sets.New[string]()
947+
encapIPList.Insert(strings.Split(config.Default.EffectiveEncapIP, ",")...)
948+
if err := util.SetNodeEncapIPs(nodeAnnotator, encapIPList); err != nil {
949+
return fmt.Errorf("failed to set node-encap-ips annotation for node %s: %w", nc.name, err)
950+
}
951+
944952
if err := nodeAnnotator.Run(); err != nil {
945953
return fmt.Errorf("failed to set node %s annotations: %w", nc.name, err)
946954
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -956,6 +956,8 @@ func (h *defaultNetworkControllerEventHandler) UpdateResource(oldObj, newObj int
956956
newNodeIsLocalZoneNode := h.oc.isLocalZoneNode(newNode)
957957
zoneClusterChanged := h.oc.nodeZoneClusterChanged(oldNode, newNode, newNodeIsLocalZoneNode, types.DefaultNetworkName)
958958
nodeSubnetChange := nodeSubnetChanged(oldNode, newNode, types.DefaultNetworkName)
959+
nodeEncapIPsChanged := util.NodeEncapIPsChanged(oldNode, newNode)
960+
959961
var aggregatedErrors []error
960962
if newNodeIsLocalZoneNode {
961963
var nodeSyncsParam *nodeSyncs
@@ -1002,7 +1004,8 @@ func (h *defaultNetworkControllerEventHandler) UpdateResource(oldObj, newObj int
10021004
// Check if the node moved from local zone to remote zone and if so syncZoneIC should be set to true.
10031005
// Also check if node subnet changed, so static routes are properly set
10041006
// Also check if the node is used to be a hybrid overlay node
1005-
syncZoneIC = syncZoneIC || h.oc.isLocalZoneNode(oldNode) || nodeSubnetChange || zoneClusterChanged || primaryAddrChanged(oldNode, newNode) || switchToOvnNode
1007+
syncZoneIC = syncZoneIC || h.oc.isLocalZoneNode(oldNode) || nodeSubnetChange || zoneClusterChanged ||
1008+
switchToOvnNode || nodeEncapIPsChanged
10061009
if syncZoneIC {
10071010
klog.Infof("Node %s in remote zone %s needs interconnect zone sync up. Zone cluster changed: %v",
10081011
newNode.Name, util.GetNodeZone(newNode), zoneClusterChanged)

‎go-controller/pkg/ovn/zone_interconnect/chassis_handler.go‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,29 @@ func (zch *ZoneChassisHandler) createOrUpdateNodeChassis(node *corev1.Node, isRe
134134
node.Name, parsedErr)
135135
}
136136

137-
nodePrimaryIp, err := util.GetNodePrimaryIP(node)
137+
// Get the encap IPs.
138+
encapIPs, err := util.ParseNodeEncapIPsAnnotation(node)
138139
if err != nil {
139-
return fmt.Errorf("failed to parse node %s primary IP %w", node.Name, err)
140+
return fmt.Errorf("failed to parse node-encap-ips for node - %s, error: %w",
141+
node.Name, err)
142+
}
143+
144+
encaps := make([]*sbdb.Encap, 0, len(encapIPs))
145+
encapOptions := map[string]string{}
146+
encapOptions["csum"] = "true"
147+
// set the geneve port if using something else than default
148+
if config.Default.EncapPort != config.DefaultEncapPort {
149+
encapOptions["dst_port"] = strconv.FormatUint(uint64(config.Default.EncapPort), 10)
150+
}
151+
152+
for _, ovnEncapIP := range encapIPs {
153+
encap := sbdb.Encap{
154+
ChassisName: chassisID,
155+
IP: strings.TrimSpace(ovnEncapIP),
156+
Type: "geneve",
157+
Options: encapOptions,
158+
}
159+
encaps = append(encaps, &encap)
140160
}
141161

142162
chassis := sbdb.Chassis{
@@ -147,17 +167,5 @@ func (zch *ZoneChassisHandler) createOrUpdateNodeChassis(node *corev1.Node, isRe
147167
},
148168
}
149169

150-
encap := sbdb.Encap{
151-
ChassisName: chassisID,
152-
IP: nodePrimaryIp,
153-
Type: "geneve",
154-
Options: map[string]string{"csum": "true"},
155-
}
156-
157-
// set the geneve port if using something else than default
158-
if config.Default.EncapPort != config.DefaultEncapPort {
159-
encap.Options["dst_port"] = strconv.FormatUint(uint64(config.Default.EncapPort), 10)
160-
}
161-
162-
return libovsdbops.CreateOrUpdateChassis(zch.sbClient, &chassis, &encap)
170+
return libovsdbops.CreateOrUpdateChassis(zch.sbClient, &chassis, encaps...)
163171
}

0 commit comments

Comments
 (0)