Skip to content

Commit e1e0b96

Browse files
sven-rosenzweigsven-rosenzweig
committed
feat(controller): add support for L3 subinterfaces
Implement basic CRUD operations for SubInterfaces: * Validate that the parent interface exists and is in a configured state before applying subinterface; halt reconciliation otherwise * Ensure parent interface is Layer 3 (no switchport configuration) * Watch parent interface for create/update events to trigger reconciliation when it becomes available * Cascade deletion: removing the parent interface also deletes associated subinterfaces To support the cascading deletion of interfaces, we set owner reference on subinterface to parent interface. Omitting device owner reference on subinterface as this blocks deletion until device gets deleted.
1 parent 900cf07 commit e1e0b96

3 files changed

Lines changed: 290 additions & 2 deletions

File tree

‎api/core/v1alpha1/groupversion_info.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,15 @@ const (
208208

209209
// VRFNotFoundReason indicates that a referenced VRF was not found.
210210
VRFNotFoundReason = "VRFNotFound"
211+
212+
// ParentInterfaceNotFoundReason indicates that a referenced parent interface for a subinterface was not found.
213+
ParentInterfaceNotFoundReason = "ParentInterfaceNotFound"
214+
215+
// ParentInterfaceNotConfiguredReason indicates that the parent interface of a subinterface is not configured.
216+
ParentInterfaceNotConfiguredReason = "ParentInterfaceNotConfigured"
217+
218+
// InvalidParentInterfaceTypeReason indicates that a referenced parent interface type is not supported.
219+
InvalidParentInterfaceTypeReason = "InvalidParentInterfaceType"
211220
)
212221

213222
// Reasons that are specific to [Device] objects.

‎internal/controller/core/interface_controller.go‎

Lines changed: 144 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ const (
215215
interfaceUnnumberedRefKey = ".spec.ipv4.unnumbered.interfaceRef.name"
216216
interfaceVlanRefKey = ".spec.vlanRef.name"
217217
interfaceVrfRefKey = ".spec.vrfRef.name"
218+
interfaceParentRefKey = ".spec.parentInterfaceRef.name"
218219
)
219220

220221
// SetupWithManager sets up the controller with the Manager.
@@ -277,6 +278,16 @@ func (r *InterfaceReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man
277278
return err
278279
}
279280

281+
if err := mgr.GetFieldIndexer().IndexField(ctx, &v1alpha1.Interface{}, interfaceParentRefKey, func(obj client.Object) []string {
282+
intf := obj.(*v1alpha1.Interface)
283+
if intf.Spec.ParentInterfaceRef == nil {
284+
return nil
285+
}
286+
return []string{intf.Spec.ParentInterfaceRef.Name}
287+
}); err != nil {
288+
return err
289+
}
290+
280291
bldr := ctrl.NewControllerManagedBy(mgr).
281292
For(&v1alpha1.Interface{}).
282293
Named("interface").
@@ -308,6 +319,19 @@ func (r *InterfaceReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man
308319
},
309320
}),
310321
).
322+
// Watches enqueues subinterfaces when their parent interface changes.
323+
Watches(
324+
&v1alpha1.Interface{},
325+
handler.EnqueueRequestsFromMapFunc(r.parentToSubinterfaces),
326+
builder.WithPredicates(predicate.Funcs{
327+
UpdateFunc: func(e event.UpdateEvent) bool {
328+
return false
329+
},
330+
GenericFunc: func(e event.GenericEvent) bool {
331+
return false
332+
},
333+
}),
334+
).
311335
// Watches enqueues Aggregate Interfaces for updates in referenced member resources.
312336
Watches(
313337
&v1alpha1.Interface{},
@@ -409,8 +433,9 @@ func (r *InterfaceReconciler) reconcile(ctx context.Context, s *scope) (reterr e
409433

410434
s.Interface.Labels[v1alpha1.DeviceLabel] = s.Device.Name
411435

412-
// Ensure the Interface is owned by the Device.
413-
if !controllerutil.HasControllerReference(s.Interface) {
436+
// Ensure the Interface (except subinterfaces) is owned by the Device.
437+
// Subinterfaces have their parent interface as owner, and the parent interface is owned by the Device.
438+
if !controllerutil.HasControllerReference(s.Interface) && s.Interface.Spec.Type != v1alpha1.InterfaceTypeSubinterface {
414439
if err := controllerutil.SetOwnerReference(s.Device, s.Interface, r.Scheme, controllerutil.WithBlockOwnerDeletion(true)); err != nil {
415440
return err
416441
}
@@ -441,6 +466,13 @@ func (r *InterfaceReconciler) reconcile(ctx context.Context, s *scope) (reterr e
441466
}
442467
}
443468

469+
if s.Interface.Spec.Type == v1alpha1.InterfaceTypeSubinterface {
470+
err := r.reconcileSubinterfaces(ctx, s)
471+
if err != nil {
472+
return err
473+
}
474+
}
475+
444476
var multiChassisID *int16
445477
if s.Interface.Spec.Aggregation != nil && s.Interface.Spec.Aggregation.MultiChassis != nil {
446478
multiChassisID = &s.Interface.Spec.Aggregation.MultiChassis.ID
@@ -763,6 +795,79 @@ func (r *InterfaceReconciler) reconcileMemberInterfaces(ctx context.Context, s *
763795
return members, nil
764796
}
765797

798+
// reconcileSubinterfaces ensures that the parent interfaces exist and belong to the same device as the subinterface.
799+
// It also updates the subinterfaces owner reference to the parent interface
800+
func (r *InterfaceReconciler) reconcileSubinterfaces(ctx context.Context, s *scope) error {
801+
parentIntf := new(v1alpha1.Interface)
802+
803+
key := client.ObjectKey{Name: s.Interface.Spec.ParentInterfaceRef.Name, Namespace: s.Interface.Namespace}
804+
if err := r.Get(ctx, key, parentIntf); err != nil {
805+
if apierrors.IsNotFound(err) {
806+
conditions.Set(s.Interface, metav1.Condition{
807+
Type: v1alpha1.ConfiguredCondition,
808+
Status: metav1.ConditionFalse,
809+
Reason: v1alpha1.ParentInterfaceNotFoundReason,
810+
Message: fmt.Sprintf("referenced parent interface %q for not found", key),
811+
})
812+
return reconcile.TerminalError(fmt.Errorf("failed to get parent interface %q: %w", s.Interface.Spec.ParentInterfaceRef.Name, err))
813+
}
814+
return err
815+
}
816+
817+
// Check matching device reference
818+
if parentIntf.Spec.DeviceRef.Name != s.Device.Name {
819+
conditions.Set(s.Interface, metav1.Condition{
820+
Type: v1alpha1.ConfiguredCondition,
821+
Status: metav1.ConditionFalse,
822+
Reason: v1alpha1.CrossDeviceReferenceReason,
823+
Message: fmt.Sprintf("parent interface %q belongs to a different device", key),
824+
})
825+
return reconcile.TerminalError(fmt.Errorf("parent interface %q belongs to device %q in not ready state", s.Interface.Spec.ParentInterfaceRef.Name, parentIntf.Spec.DeviceRef.Name))
826+
}
827+
828+
// Check if parent interface is an aggregate or physical interface
829+
if parentIntf.Spec.Type != v1alpha1.InterfaceTypePhysical && parentIntf.Spec.Type != v1alpha1.InterfaceTypeAggregate {
830+
conditions.Set(s.Interface, metav1.Condition{
831+
Type: v1alpha1.ConfiguredCondition,
832+
Status: metav1.ConditionFalse,
833+
Reason: v1alpha1.InvalidParentInterfaceTypeReason,
834+
Message: fmt.Sprintf("parent interface %q is not of type Physical or Aggregate, got %q", key, parentIntf.Spec.Type),
835+
})
836+
return reconcile.TerminalError(fmt.Errorf("parent interface %q is not of type Physical or Aggregate, got %q", s.Interface.Spec.ParentInterfaceRef.Name, parentIntf.Spec.Type))
837+
}
838+
839+
// L2 interfaces do not support subinterfaces config
840+
if parentIntf.Spec.Switchport != nil {
841+
conditions.Set(s.Interface, metav1.Condition{
842+
Type: v1alpha1.ConfiguredCondition,
843+
Status: metav1.ConditionFalse,
844+
Reason: v1alpha1.InvalidInterfaceTypeReason,
845+
Message: fmt.Sprintf("parent interface %q is an L2 interface", key),
846+
})
847+
return reconcile.TerminalError(fmt.Errorf("parent interface %q is an L2 interface", s.Interface.Spec.ParentInterfaceRef.Name))
848+
}
849+
850+
// Ensure the Subinterface is owned by the parent interface.
851+
if !controllerutil.HasControllerReference(s.Interface) {
852+
if err := controllerutil.SetOwnerReference(parentIntf, s.Interface, r.Scheme, controllerutil.WithBlockOwnerDeletion(true)); err != nil {
853+
return reconcile.TerminalError(err)
854+
}
855+
}
856+
857+
// Parent interface must be configured
858+
if !conditions.IsConfigured(parentIntf) {
859+
conditions.Set(s.Interface, metav1.Condition{
860+
Type: v1alpha1.ConfiguredCondition,
861+
Status: metav1.ConditionFalse,
862+
Reason: v1alpha1.ParentInterfaceNotConfiguredReason,
863+
Message: fmt.Sprintf("parent interface %q not ready", key),
864+
})
865+
return reconcile.TerminalError(fmt.Errorf("parent interface %q in not ready state", s.Interface.Spec.ParentInterfaceRef.Name))
866+
}
867+
868+
return nil
869+
}
870+
766871
func (r *InterfaceReconciler) finalize(ctx context.Context, s *scope) (reterr error) {
767872
if s.Interface.Spec.Aggregation != nil {
768873
if err := r.finalizeMemberInterfaces(ctx, s); err != nil {
@@ -943,6 +1048,43 @@ func (r *InterfaceReconciler) aggregateToMembers(ctx context.Context, obj client
9431048
return requests
9441049
}
9451050

1051+
// parentToSubinterfaces is a [handler.MapFunc] to be used to enqueue requests for reconciliation
1052+
// for Subinterfaces based on their parent Interface.
1053+
func (r *InterfaceReconciler) parentToSubinterfaces(ctx context.Context, obj client.Object) []ctrl.Request {
1054+
intf, ok := obj.(*v1alpha1.Interface)
1055+
if !ok {
1056+
panic(fmt.Sprintf("Expected a Interface but got a %T", obj))
1057+
}
1058+
1059+
if intf.Spec.Type != v1alpha1.InterfaceTypePhysical && intf.Spec.Type != v1alpha1.InterfaceTypeAggregate {
1060+
return nil
1061+
}
1062+
1063+
log := ctrl.LoggerFrom(ctx, "Interface", klog.KObj(intf))
1064+
interfaces := new(v1alpha1.InterfaceList)
1065+
1066+
// List all interfaces in the same namespace with a parent interface reference to the physical interface.
1067+
if err := r.List(ctx, interfaces, client.InNamespace(intf.Namespace), client.MatchingFields{interfaceParentRefKey: intf.Name}); err != nil {
1068+
log.Error(err, "Failed to list Interfaces")
1069+
return nil
1070+
}
1071+
1072+
requests := []ctrl.Request{}
1073+
for _, i := range interfaces.Items {
1074+
if i.Spec.ParentInterfaceRef != nil && i.Spec.ParentInterfaceRef.Name == intf.Name {
1075+
log.V(2).Info("Enqueuing SubInterface for reconciliation", "SubInterface", klog.KObj(&i))
1076+
requests = append(requests, ctrl.Request{
1077+
NamespacedName: client.ObjectKey{
1078+
Name: i.Name,
1079+
Namespace: i.Namespace,
1080+
},
1081+
})
1082+
}
1083+
}
1084+
1085+
return requests
1086+
}
1087+
9461088
// vlanToRoutedVLAN is a [handler.MapFunc] to be used to enqueue requests for reconciliation
9471089
// for a RoutedVLAN Interface when its referenced VLAN changes.
9481090
func (r *InterfaceReconciler) vlanToRoutedVLAN(ctx context.Context, obj client.Object) []ctrl.Request {

‎internal/controller/core/interface_controller_test.go‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,143 @@ var _ = Describe("Interface Controller", func() {
754754
}).Should(Succeed())
755755
})
756756

757+
It("Should fail reconcile when parent interface does not exist for subinterface", func() {
758+
By("Creating a Subinterface referencing a non-existent parent")
759+
subintf := &v1alpha1.Interface{
760+
ObjectMeta: metav1.ObjectMeta{
761+
Name: name,
762+
Namespace: metav1.NamespaceDefault,
763+
},
764+
Spec: v1alpha1.InterfaceSpec{
765+
DeviceRef: v1alpha1.LocalObjectReference{Name: name},
766+
Name: name + ".100",
767+
AdminState: v1alpha1.AdminStateUp,
768+
Description: "Subinterface without parent",
769+
Type: v1alpha1.InterfaceTypeSubinterface,
770+
ParentInterfaceRef: &v1alpha1.LocalObjectReference{
771+
Name: "non-existent-parent",
772+
},
773+
Encapsulation: &v1alpha1.Encapsulation{
774+
Type: v1alpha1.EncapsulationTypeDot1Q,
775+
Tag: 100,
776+
},
777+
IPv4: &v1alpha1.InterfaceIPv4{
778+
Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.0.100/24")}},
779+
},
780+
},
781+
}
782+
Expect(k8sClient.Create(ctx, subintf)).To(Succeed())
783+
784+
By("Verifying the controller sets parent interface not ready status")
785+
Eventually(func(g Gomega) {
786+
resource := &v1alpha1.Interface{}
787+
g.Expect(k8sClient.Get(ctx, key, resource)).To(Succeed())
788+
g.Expect(resource.Status.Conditions).To(HaveLen(4))
789+
g.Expect(resource.Status.Conditions[0].Type).To(Equal(v1alpha1.ReadyCondition))
790+
g.Expect(resource.Status.Conditions[0].Status).To(Equal(metav1.ConditionFalse))
791+
g.Expect(resource.Status.Conditions[1].Type).To(Equal(v1alpha1.ConfiguredCondition))
792+
g.Expect(resource.Status.Conditions[1].Status).To(Equal(metav1.ConditionFalse))
793+
g.Expect(resource.Status.Conditions[1].Reason).To(Equal(v1alpha1.ParentInterfaceNotFoundReason))
794+
g.Expect(resource.Status.Conditions[2].Type).To(Equal(v1alpha1.OperationalCondition))
795+
g.Expect(resource.Status.Conditions[2].Status).To(Equal(metav1.ConditionUnknown))
796+
g.Expect(resource.Status.Conditions[3].Type).To(Equal(v1alpha1.PausedCondition))
797+
g.Expect(resource.Status.Conditions[3].Status).To(Equal(metav1.ConditionFalse))
798+
}).Should(Succeed())
799+
})
800+
801+
It("Should successfully reconcile parent Physical interface and Subinterface", func() {
802+
const parentName = "test-parent-eth"
803+
const subinterfaceName = "test-subintf"
804+
805+
By("Creating a Physical parent interface")
806+
parentIntf := &v1alpha1.Interface{
807+
ObjectMeta: metav1.ObjectMeta{
808+
Name: parentName,
809+
Namespace: metav1.NamespaceDefault,
810+
},
811+
Spec: v1alpha1.InterfaceSpec{
812+
DeviceRef: v1alpha1.LocalObjectReference{Name: name},
813+
Name: parentName,
814+
AdminState: v1alpha1.AdminStateUp,
815+
Description: "Parent Physical Interface",
816+
MTU: 9000,
817+
Type: v1alpha1.InterfaceTypePhysical,
818+
},
819+
}
820+
Expect(k8sClient.Create(ctx, parentIntf)).To(Succeed())
821+
822+
By("Verifying the parent Physical interface is ready")
823+
Eventually(func(g Gomega) {
824+
resource := &v1alpha1.Interface{}
825+
g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: parentName, Namespace: metav1.NamespaceDefault}, resource)).To(Succeed())
826+
g.Expect(resource.Status.Conditions).To(HaveLen(4))
827+
g.Expect(resource.Status.Conditions[0].Type).To(Equal(v1alpha1.ReadyCondition))
828+
g.Expect(resource.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue))
829+
g.Expect(resource.Status.Conditions[1].Type).To(Equal(v1alpha1.ConfiguredCondition))
830+
g.Expect(resource.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue))
831+
}).Should(Succeed())
832+
833+
By("Creating a Subinterface referencing the parent")
834+
subintf := &v1alpha1.Interface{
835+
ObjectMeta: metav1.ObjectMeta{
836+
Name: subinterfaceName,
837+
Namespace: metav1.NamespaceDefault,
838+
},
839+
Spec: v1alpha1.InterfaceSpec{
840+
DeviceRef: v1alpha1.LocalObjectReference{Name: name},
841+
Name: parentName + ".100",
842+
AdminState: v1alpha1.AdminStateUp,
843+
Description: "Subinterface with 802.1q encapsulation",
844+
Type: v1alpha1.InterfaceTypeSubinterface,
845+
ParentInterfaceRef: &v1alpha1.LocalObjectReference{
846+
Name: parentName,
847+
},
848+
Encapsulation: &v1alpha1.Encapsulation{
849+
Type: v1alpha1.EncapsulationTypeDot1Q,
850+
Tag: 100,
851+
},
852+
IPv4: &v1alpha1.InterfaceIPv4{
853+
Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.100.1/24")}},
854+
},
855+
},
856+
}
857+
Expect(k8sClient.Create(ctx, subintf)).To(Succeed())
858+
859+
By("Verifying the controller sets the parent interface as owner reference")
860+
Eventually(func(g Gomega) {
861+
resource := &v1alpha1.Interface{}
862+
g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: subinterfaceName, Namespace: metav1.NamespaceDefault}, resource)).To(Succeed())
863+
g.Expect(resource.OwnerReferences).To(HaveLen(1))
864+
g.Expect(resource.OwnerReferences[0].Kind).To(Equal("Interface"))
865+
g.Expect(resource.OwnerReferences[0].Name).To(Equal(parentName))
866+
}).Should(Succeed())
867+
868+
By("Verifying the subinterface status is ready")
869+
Eventually(func(g Gomega) {
870+
resource := &v1alpha1.Interface{}
871+
g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: subinterfaceName, Namespace: metav1.NamespaceDefault}, resource)).To(Succeed())
872+
g.Expect(resource.Status.Conditions).To(HaveLen(4))
873+
g.Expect(resource.Status.Conditions[0].Type).To(Equal(v1alpha1.ReadyCondition))
874+
g.Expect(resource.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue))
875+
g.Expect(resource.Status.Conditions[1].Type).To(Equal(v1alpha1.ConfiguredCondition))
876+
g.Expect(resource.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue))
877+
g.Expect(resource.Status.Conditions[2].Type).To(Equal(v1alpha1.OperationalCondition))
878+
g.Expect(resource.Status.Conditions[2].Status).To(Equal(metav1.ConditionTrue))
879+
g.Expect(resource.Status.Conditions[3].Type).To(Equal(v1alpha1.PausedCondition))
880+
g.Expect(resource.Status.Conditions[3].Status).To(Equal(metav1.ConditionFalse))
881+
}).Should(Succeed())
882+
883+
By("Verifying the Subinterface is configured in the provider")
884+
Eventually(func(g Gomega) {
885+
g.Expect(testProvider.Ports.Has(parentName+".100")).To(BeTrue(), "Provider should have Subinterface configured")
886+
}).Should(Succeed())
887+
888+
By("Verifying the parent Physical interface is configured in the provider")
889+
Eventually(func(g Gomega) {
890+
g.Expect(testProvider.Ports.Has(parentName)).To(BeTrue(), "Provider should have parent Physical Interface configured")
891+
}).Should(Succeed())
892+
})
893+
757894
It("Should handle unnumbered reference to non-Loopback Interface", func() {
758895
By("Creating a Physical Interface to be referenced")
759896
phys := &v1alpha1.Interface{

0 commit comments

Comments
 (0)