Skip to content

Commit 09fa2d6

Browse files
adamtrizuljak-sapadamtrizuljak-sap
authored
Implement Configured condition for ISIS controller (#632)
Implement Configured condition for ISIS Extract ISIS controller + test changes from branch feat/configured-condition Find error paths in controller and handle them by setting conditions.FromError(err) Fix optional ISIS.InterfaceRefs field Signed-off-by: Adam Trizuljak <adam.trizuljak@sap.com>
1 parent eeadeb6 commit 09fa2d6

7 files changed

Lines changed: 144 additions & 38 deletions

File tree

‎api/core/v1alpha1/isis_types.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ type ISISStatus struct {
112112
// +kubebuilder:printcolumn:name="NET",type=string,JSONPath=`.spec.networkEntityTitle`
113113
// +kubebuilder:printcolumn:name="Level",type=string,JSONPath=`.spec.type`,priority=1
114114
// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`
115+
// +kubebuilder:printcolumn:name="Configured",type=string,JSONPath=`.status.conditions[?(@.type=="Configured")].status`,priority=1
115116
// +kubebuilder:printcolumn:name="Paused",type=string,JSONPath=`.status.conditions[?(@.type=="Paused")].status`,priority=1
116117
// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
117118

‎charts/network-operator/templates/crd/isis.networking.metal.ironcore.dev.yaml‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/networking.metal.ironcore.dev_isis.yaml‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/samples/v1alpha1_isis.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ spec:
1616
addressFamilies:
1717
- IPv4Unicast
1818
- IPv6Unicast
19-
interfaces:
19+
interfaceRefs:
2020
- name: eth1-1
2121
- name: eth1-2
2222
- name: lo0

‎internal/controller/core/isis_controller.go‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func (r *ISISReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctr
171171
return ctrl.Result{}, nil
172172
}
173173

174-
if conditions.InitializeConditions(obj, v1alpha1.ReadyCondition) {
174+
if conditions.InitializeConditions(obj, v1alpha1.ReadyCondition, v1alpha1.ConfiguredCondition) {
175175
log.V(1).Info("Initializing status conditions")
176176
return ctrl.Result{}, r.Status().Update(ctx, obj)
177177
}
@@ -286,17 +286,17 @@ func (r *ISISReconciler) reconcile(ctx context.Context, s *isisScope) (reterr er
286286

287287
s.ISIS.Labels[v1alpha1.DeviceLabel] = s.Device.Name
288288

289+
defer func() {
290+
conditions.RecomputeReady(s.ISIS)
291+
}()
292+
289293
// Ensure the ISIS is owned by the Device.
290294
if !controllerutil.HasControllerReference(s.ISIS) {
291295
if err := controllerutil.SetOwnerReference(s.Device, s.ISIS, r.Scheme, controllerutil.WithBlockOwnerDeletion(true)); err != nil {
292296
return err
293297
}
294298
}
295299

296-
defer func() {
297-
conditions.RecomputeReady(s.ISIS)
298-
}()
299-
300300
var interfaces []*v1alpha1.Interface
301301
for _, iface := range s.ISIS.Spec.InterfaceRefs {
302302
intf := new(v1alpha1.Interface)
@@ -315,7 +315,7 @@ func (r *ISISReconciler) reconcile(ctx context.Context, s *isisScope) (reterr er
315315

316316
if !conditions.IsConfigured(intf) {
317317
conditions.Set(s.ISIS, metav1.Condition{
318-
Type: v1alpha1.ReadyCondition,
318+
Type: v1alpha1.ConfiguredCondition,
319319
Status: metav1.ConditionFalse,
320320
Reason: v1alpha1.WaitingForDependenciesReason,
321321
Message: "Waiting for referenced interfaces to become configured",
@@ -343,8 +343,6 @@ func (r *ISISReconciler) reconcile(ctx context.Context, s *isisScope) (reterr er
343343
})
344344

345345
cond := conditions.FromError(err)
346-
// As this resource is configuration only, we use the Configured condition as top-level Ready condition.
347-
cond.Type = v1alpha1.ReadyCondition
348346
conditions.Set(s.ISIS, cond)
349347

350348
return err

‎internal/controller/core/isis_controller_test.go‎

Lines changed: 100 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
package core
44

55
import (
6+
"net/netip"
7+
68
. "github.com/onsi/ginkgo/v2"
79
. "github.com/onsi/gomega"
810
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -109,11 +111,13 @@ var _ = Describe("ISIS Controller", func() {
109111
Eventually(func(g Gomega) {
110112
resource := &v1alpha1.ISIS{}
111113
g.Expect(k8sClient.Get(ctx, key, resource)).To(Succeed())
112-
g.Expect(resource.Status.Conditions).To(HaveLen(2))
114+
g.Expect(resource.Status.Conditions).To(HaveLen(3))
113115
g.Expect(resource.Status.Conditions[0].Type).To(Equal(v1alpha1.ReadyCondition))
114116
g.Expect(resource.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue))
115-
g.Expect(resource.Status.Conditions[1].Type).To(Equal(v1alpha1.PausedCondition))
116-
g.Expect(resource.Status.Conditions[1].Status).To(Equal(metav1.ConditionFalse))
117+
g.Expect(resource.Status.Conditions[1].Type).To(Equal(v1alpha1.ConfiguredCondition))
118+
g.Expect(resource.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue))
119+
g.Expect(resource.Status.Conditions[2].Type).To(Equal(v1alpha1.PausedCondition))
120+
g.Expect(resource.Status.Conditions[2].Status).To(Equal(metav1.ConditionFalse))
117121
}).Should(Succeed())
118122

119123
By("Ensuring the resource is created in the provider")
@@ -202,4 +206,97 @@ var _ = Describe("ISIS Controller", func() {
202206
}).Should(Succeed())
203207
})
204208
})
209+
210+
Context("When a referenced Interface is not configured", func() {
211+
var deviceName string
212+
213+
BeforeEach(func() {
214+
By("Creating the Device resource")
215+
device := &v1alpha1.Device{
216+
GenerateName: "test-isis-intfnr-",
217+
Namespace: metav1.NamespaceDefault,
218+
Spec: v1alpha1.DeviceSpec{
219+
Endpoint: v1alpha1.Endpoint{Address: "192.168.10.2:9339"},
220+
Provider: "test-provider",
221+
},
222+
}
223+
Expect(k8sClient.Create(ctx, device)).To(Succeed())
224+
deviceName = device.Name
225+
DeferCleanup(func() {
226+
Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed())
227+
})
228+
})
229+
230+
It("Should set ConfiguredCondition to False with WaitingForDependenciesReason", func() {
231+
By("Creating an Interface referencing a non-existent VRF so it stays not configured")
232+
intf := &v1alpha1.Interface{
233+
GenerateName: "test-isis-intfnr-intf-",
234+
Namespace: metav1.NamespaceDefault,
235+
Spec: v1alpha1.InterfaceSpec{
236+
DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName},
237+
Name: "Ethernet1/1",
238+
AdminState: v1alpha1.AdminStateUp,
239+
Type: v1alpha1.InterfaceTypePhysical,
240+
VrfRef: &v1alpha1.LocalObjectReference{Name: "test-isis-intfnr-nonexistent-vrf"},
241+
IPv4: &v1alpha1.InterfaceIPv4{Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.5.1/24")}}},
242+
},
243+
}
244+
Expect(k8sClient.Create(ctx, intf)).To(Succeed())
245+
intfKey := client.ObjectKeyFromObject(intf)
246+
DeferCleanup(func() {
247+
Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, intf))).To(Succeed())
248+
Eventually(func(g Gomega) {
249+
g.Expect(apierrors.IsNotFound(k8sClient.Get(ctx, intfKey, &v1alpha1.Interface{}))).To(BeTrue())
250+
}).Should(Succeed())
251+
})
252+
253+
By("Verifying the Interface is not configured")
254+
Eventually(func(g Gomega) {
255+
g.Expect(k8sClient.Get(ctx, intfKey, intf)).To(Succeed())
256+
cond := meta.FindStatusCondition(intf.Status.Conditions, v1alpha1.ConfiguredCondition)
257+
g.Expect(cond).ToNot(BeNil())
258+
g.Expect(cond.Status).To(Equal(metav1.ConditionFalse))
259+
}).Should(Succeed())
260+
261+
By("Creating an ISIS referencing the non-configured Interface")
262+
isis := &v1alpha1.ISIS{
263+
GenerateName: "test-isis-intfnr-",
264+
Namespace: metav1.NamespaceDefault,
265+
Spec: v1alpha1.ISISSpec{
266+
DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName},
267+
Instance: "UNDERLAY",
268+
NetworkEntityTitle: "49.0001.0000.0000.0001.00",
269+
Type: v1alpha1.ISISLevel1,
270+
AddressFamilies: []v1alpha1.AddressFamily{v1alpha1.AddressFamilyIPv4Unicast},
271+
InterfaceRefs: []v1alpha1.LocalObjectReference{{Name: intf.Name}},
272+
},
273+
}
274+
Expect(k8sClient.Create(ctx, isis)).To(Succeed())
275+
isisKey := client.ObjectKeyFromObject(isis)
276+
DeferCleanup(func() {
277+
Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, isis))).To(Succeed())
278+
Eventually(func(g Gomega) {
279+
g.Expect(apierrors.IsNotFound(k8sClient.Get(ctx, isisKey, &v1alpha1.ISIS{}))).To(BeTrue())
280+
}).Should(Succeed())
281+
})
282+
283+
By("Verifying the controller sets ConfiguredCondition to False with WaitingForDependenciesReason")
284+
Eventually(func(g Gomega) {
285+
g.Expect(k8sClient.Get(ctx, isisKey, isis)).To(Succeed())
286+
cond := meta.FindStatusCondition(isis.Status.Conditions, v1alpha1.ConfiguredCondition)
287+
g.Expect(cond).ToNot(BeNil())
288+
g.Expect(cond.Status).To(Equal(metav1.ConditionFalse))
289+
g.Expect(cond.Reason).To(Equal(v1alpha1.WaitingForDependenciesReason))
290+
291+
ready := meta.FindStatusCondition(isis.Status.Conditions, v1alpha1.ReadyCondition)
292+
g.Expect(ready).ToNot(BeNil())
293+
g.Expect(ready.Status).To(Equal(metav1.ConditionFalse))
294+
}).Should(Succeed())
295+
296+
By("Verifying the ISIS is not configured in the provider")
297+
Consistently(func(g Gomega) {
298+
g.Expect(testDevices.StateFor(deviceName).ISIS.Has("UNDERLAY")).To(BeFalse())
299+
}).Should(Succeed())
300+
})
301+
})
205302
})

‎internal/provider/cisco/nxos/provider.go‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2272,37 +2272,39 @@ func (p *Provider) EnsureISIS(ctx context.Context, req *provider.EnsureISISReque
22722272
dom.AfItems.DomAfList.Set(item)
22732273
}
22742274

2275-
interfaceNames, err := p.EnsureInterfacesExist(ctx, req.Interfaces)
2276-
if err != nil {
2277-
return err
2278-
}
2275+
if len(req.Interfaces) > 0 {
2276+
interfaceNames, err := p.EnsureInterfacesExist(ctx, req.Interfaces)
2277+
if err != nil {
2278+
return err
2279+
}
22792280

2280-
// prevent bounds check in for the range loop below
2281-
// [Bounds Check Elimination]: https://go101.org/optimizations/5-bce.html
2282-
_ = req.Interfaces[len(interfaceNames)-1]
2281+
// prevent bounds check in for the range loop below
2282+
// [Bounds Check Elimination]: https://go101.org/optimizations/5-bce.html
2283+
_ = req.Interfaces[len(interfaceNames)-1]
22832284

2284-
for i, iface := range req.Interfaces {
2285-
intf := new(ISISInterface)
2286-
intf.ID = interfaceNames[i]
2287-
intf.NetworkTypeP2P = AdminStOff
2288-
if iface.Spec.Type == v1alpha1.InterfaceTypePhysical {
2289-
intf.NetworkTypeP2P = AdminStOn
2290-
}
2291-
if ipv4 {
2292-
intf.V4Enable = true
2293-
intf.V4Bfd = "inheritVrf"
2294-
if iface.Spec.BFD != nil && iface.Spec.BFD.Enabled {
2295-
intf.V4Bfd = string(PassiveControlEnabled)
2285+
for i, iface := range req.Interfaces {
2286+
intf := new(ISISInterface)
2287+
intf.ID = interfaceNames[i]
2288+
intf.NetworkTypeP2P = AdminStOff
2289+
if iface.Spec.Type == v1alpha1.InterfaceTypePhysical {
2290+
intf.NetworkTypeP2P = AdminStOn
22962291
}
2297-
}
2298-
if ipv6 {
2299-
intf.V6Enable = true
2300-
intf.V6Bfd = "inheritVrf"
2301-
if iface.Spec.BFD != nil && iface.Spec.BFD.Enabled {
2302-
intf.V6Bfd = string(PassiveControlEnabled)
2292+
if ipv4 {
2293+
intf.V4Enable = true
2294+
intf.V4Bfd = "inheritVrf"
2295+
if iface.Spec.BFD != nil && iface.Spec.BFD.Enabled {
2296+
intf.V4Bfd = string(PassiveControlEnabled)
2297+
}
2298+
}
2299+
if ipv6 {
2300+
intf.V6Enable = true
2301+
intf.V6Bfd = "inheritVrf"
2302+
if iface.Spec.BFD != nil && iface.Spec.BFD.Enabled {
2303+
intf.V6Bfd = string(PassiveControlEnabled)
2304+
}
23032305
}
2306+
dom.IfItems.IfList.Set(intf)
23042307
}
2305-
dom.IfItems.IfList.Set(intf)
23062308
}
23072309
sb.Update(i)
23082310

0 commit comments

Comments
 (0)