diff --git a/Tiltfile b/Tiltfile index 2b88b139..1fc8e2ab 100644 --- a/Tiltfile +++ b/Tiltfile @@ -48,6 +48,8 @@ k8s_resource(new_name='eth1-1', objects=['eth1-1:interface'], trigger_mode=TRIGG k8s_resource(new_name='eth1-2', objects=['eth1-2:interface'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False) k8s_resource(new_name='eth1-10', objects=['eth1-10:interface'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False) k8s_resource(new_name='po10', objects=['po-10:interface'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False) +k8s_resource(new_name='eth1-3', objects=['eth1-3:interface'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False) +k8s_resource(new_name='po20', objects=['po-20:interface'], resource_deps=['eth1-3'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False) k8s_resource(new_name='svi-10', objects=['svi-10:interface'], resource_deps=['vlan-10'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False) k8s_yaml('./config/samples/v1alpha1_banner.yaml') diff --git a/api/core/v1alpha1/interface_types.go b/api/core/v1alpha1/interface_types.go index f5c0727f..15dd035b 100644 --- a/api/core/v1alpha1/interface_types.go +++ b/api/core/v1alpha1/interface_types.go @@ -16,15 +16,15 @@ import ( // +kubebuilder:validation:XValidation:rule="self.type == 'Physical' || !has(self.ipv4) || !has(self.ipv4.unnumbered)", message="unnumbered ipv4 configuration can only be used for interfaces of type Physical" // +kubebuilder:validation:XValidation:rule="self.type != 'Aggregate' || has(self.aggregation)", message="aggregation must be specified for interfaces of type Aggregate" // +kubebuilder:validation:XValidation:rule="self.type == 'Aggregate' || !has(self.aggregation)", message="aggregation must only be specified on interfaces of type Aggregate" -// +kubebuilder:validation:XValidation:rule="self.type != 'Aggregate' || !has(self.ipv4)", message="ipv4 must not be specified for interfaces of type Aggregate" +// +kubebuilder:validation:XValidation:rule="self.type != 'Aggregate' || !has(self.switchport) || !has(self.ipv4)", message="ipv4 must not be specified for Aggregate interfaces with switchport configuration" // +kubebuilder:validation:XValidation:rule="self.type != 'RoutedVLAN' || has(self.vlanRef)", message="vlanRef must be specified for interfaces of type RoutedVLAN" // +kubebuilder:validation:XValidation:rule="self.type == 'RoutedVLAN' || !has(self.vlanRef)", message="vlanRef must only be specified on interfaces of type RoutedVLAN" // +kubebuilder:validation:XValidation:rule="self.type != 'RoutedVLAN' || !has(self.switchport)", message="switchport must not be specified for interfaces of type RoutedVLAN" // +kubebuilder:validation:XValidation:rule="self.type != 'RoutedVLAN' || !has(self.aggregation)", message="aggregation must not be specified for interfaces of type RoutedVLAN" // +kubebuilder:validation:XValidation:rule="self.type == 'RoutedVLAN' || !has(self.ipv4) || !self.ipv4.anycastGateway", message="anycastGateway can only be enabled for interfaces of type RoutedVLAN" -// +kubebuilder:validation:XValidation:rule="self.type != 'Aggregate' || !has(self.vrfRef)", message="vrfRef must not be specified for interfaces of type Aggregate" +// +kubebuilder:validation:XValidation:rule="self.type != 'Aggregate' || !has(self.switchport) || !has(self.vrfRef)", message="vrfRef must not be specified for Aggregate interfaces with switchport configuration" // +kubebuilder:validation:XValidation:rule="self.type != 'Physical' || !has(self.switchport) || !has(self.vrfRef)", message="vrfRef must not be specified for Physical interfaces with switchport configuration" -// +kubebuilder:validation:XValidation:rule="self.type != 'Aggregate' || !has(self.bfd)", message="bfd must not be specified for interfaces of type Aggregate" +// +kubebuilder:validation:XValidation:rule="self.type != 'Aggregate' || !has(self.switchport) || !has(self.bfd)", message="bfd must not be specified for Aggregate interfaces with switchport configuration" // +kubebuilder:validation:XValidation:rule="!has(self.bfd) || !has(self.switchport)", message="bfd must not be specified for interfaces with switchport configuration" // +kubebuilder:validation:XValidation:rule="self.type == 'Physical' || !has(self.ethernet)", message="ethernet configuration must only be specified on interfaces of type Physical" type InterfaceSpec struct { @@ -95,7 +95,7 @@ type InterfaceSpec struct { VrfRef *LocalObjectReference `json:"vrfRef,omitempty"` // BFD defines the Bidirectional Forwarding Detection configuration for the interface. - // BFD is only applicable for Layer 3 interfaces (Physical, Loopback, RoutedVLAN). + // BFD is only applicable for Layer 3 interfaces. // +optional BFD *BFD `json:"bfd,omitempty"` diff --git a/api/core/v1alpha1/prefix_types.go b/api/core/v1alpha1/prefix_types.go index 7d470ce5..4a1671d0 100644 --- a/api/core/v1alpha1/prefix_types.go +++ b/api/core/v1alpha1/prefix_types.go @@ -5,6 +5,8 @@ package v1alpha1 import ( "encoding/json" "net/netip" + + "k8s.io/apimachinery/pkg/api/equality" ) // IPPrefix represents an IP prefix in CIDR notation. @@ -37,6 +39,12 @@ func (p IPPrefix) IsZero() bool { return !p.IsValid() } +// Equal reports whether p and q are the same prefix. +// This method exists as a convenience for callers that need a direct comparison. +func (p IPPrefix) Equal(q IPPrefix) bool { + return p.Prefix == q.Prefix +} + // MarshalJSON implements [json.Marshaler]. func (p IPPrefix) MarshalJSON() ([]byte, error) { if !p.IsValid() { @@ -63,6 +71,19 @@ func (p *IPPrefix) UnmarshalJSON(data []byte) error { return nil } +// IsPointToPoint reports whether the prefix indicates a point-to-point link. +// For IPv4, this means a /31 subnet mask as defined in [RFC 3021]. +// For IPv6, this means a /127 subnet mask as defined in [RFC 6164]. +// +// [RFC 3021]: https://datatracker.ietf.org/doc/html/rfc3021 +// [RFC 6164]: https://datatracker.ietf.org/doc/html/rfc6164 +func (p IPPrefix) IsPointToPoint() bool { + if p.Addr().Is4() { + return p.Bits() == 31 + } + return p.Bits() == 127 +} + // DeepCopyInto copies all properties of this object into another object of the same type func (in *IPPrefix) DeepCopyInto(out *IPPrefix) { *out = *in @@ -77,3 +98,15 @@ func (in *IPPrefix) DeepCopy() *IPPrefix { in.DeepCopyInto(out) return out } + +func init() { + // IPPrefix embeds [netip.Prefix] which contains unexported fields. + // [equality.Semantic.DeepEqual] panics on unexported fields, so an + // explicit equality function is registered in this package's init to + // make any type containing IPPrefix safe to compare. + if err := equality.Semantic.AddFunc(func(a, b IPPrefix) bool { + return a.Equal(b) + }); err != nil { + panic(err) + } +} diff --git a/api/core/v1alpha1/prefix_types_test.go b/api/core/v1alpha1/prefix_types_test.go new file mode 100644 index 00000000..40149449 --- /dev/null +++ b/api/core/v1alpha1/prefix_types_test.go @@ -0,0 +1,31 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import "testing" + +func TestIPPrefix_IsPointToPoint(t *testing.T) { + tests := []struct { + name string + prefix string + want bool + }{ + {name: "IPv4 /31 is p2p", prefix: "10.0.0.0/31", want: true}, + {name: "IPv4 /32 is not p2p", prefix: "10.0.0.1/32", want: false}, + {name: "IPv4 /30 is not p2p", prefix: "10.0.0.0/30", want: false}, + {name: "IPv4 /24 is not p2p", prefix: "192.168.1.0/24", want: false}, + {name: "IPv6 /127 is p2p", prefix: "2001:db8::/127", want: true}, + {name: "IPv6 /128 is not p2p", prefix: "2001:db8::1/128", want: false}, + {name: "IPv6 /126 is not p2p", prefix: "2001:db8::/126", want: false}, + {name: "IPv6 /64 is not p2p", prefix: "2001:db8::/64", want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := MustParsePrefix(tt.prefix) + if got := p.IsPointToPoint(); got != tt.want { + t.Errorf("IPPrefix(%q).IsPointToPoint() = %v, want %v", tt.prefix, got, tt.want) + } + }) + } +} diff --git a/charts/network-operator/templates/crd/interfaces.networking.metal.ironcore.dev.yaml b/charts/network-operator/templates/crd/interfaces.networking.metal.ironcore.dev.yaml index d329d1c1..d3ab3128 100644 --- a/charts/network-operator/templates/crd/interfaces.networking.metal.ironcore.dev.yaml +++ b/charts/network-operator/templates/crd/interfaces.networking.metal.ironcore.dev.yaml @@ -156,7 +156,7 @@ spec: bfd: description: |- BFD defines the Bidirectional Forwarding Detection configuration for the interface. - BFD is only applicable for Layer 3 interfaces (Physical, Loopback, RoutedVLAN). + BFD is only applicable for Layer 3 interfaces. properties: desiredMinimumTxInterval: description: |- @@ -440,8 +440,9 @@ spec: rule: self.type != 'Aggregate' || has(self.aggregation) - message: aggregation must only be specified on interfaces of type Aggregate rule: self.type == 'Aggregate' || !has(self.aggregation) - - message: ipv4 must not be specified for interfaces of type Aggregate - rule: self.type != 'Aggregate' || !has(self.ipv4) + - message: ipv4 must not be specified for Aggregate interfaces with switchport + configuration + rule: self.type != 'Aggregate' || !has(self.switchport) || !has(self.ipv4) - message: vlanRef must be specified for interfaces of type RoutedVLAN rule: self.type != 'RoutedVLAN' || has(self.vlanRef) - message: vlanRef must only be specified on interfaces of type RoutedVLAN @@ -452,13 +453,15 @@ spec: rule: self.type != 'RoutedVLAN' || !has(self.aggregation) - message: anycastGateway can only be enabled for interfaces of type RoutedVLAN rule: self.type == 'RoutedVLAN' || !has(self.ipv4) || !self.ipv4.anycastGateway - - message: vrfRef must not be specified for interfaces of type Aggregate - rule: self.type != 'Aggregate' || !has(self.vrfRef) + - message: vrfRef must not be specified for Aggregate interfaces with + switchport configuration + rule: self.type != 'Aggregate' || !has(self.switchport) || !has(self.vrfRef) - message: vrfRef must not be specified for Physical interfaces with switchport configuration rule: self.type != 'Physical' || !has(self.switchport) || !has(self.vrfRef) - - message: bfd must not be specified for interfaces of type Aggregate - rule: self.type != 'Aggregate' || !has(self.bfd) + - message: bfd must not be specified for Aggregate interfaces with switchport + configuration + rule: self.type != 'Aggregate' || !has(self.switchport) || !has(self.bfd) - message: bfd must not be specified for interfaces with switchport configuration rule: '!has(self.bfd) || !has(self.switchport)' - message: ethernet configuration must only be specified on interfaces diff --git a/config/crd/bases/networking.metal.ironcore.dev_interfaces.yaml b/config/crd/bases/networking.metal.ironcore.dev_interfaces.yaml index dc496169..08918da4 100644 --- a/config/crd/bases/networking.metal.ironcore.dev_interfaces.yaml +++ b/config/crd/bases/networking.metal.ironcore.dev_interfaces.yaml @@ -153,7 +153,7 @@ spec: bfd: description: |- BFD defines the Bidirectional Forwarding Detection configuration for the interface. - BFD is only applicable for Layer 3 interfaces (Physical, Loopback, RoutedVLAN). + BFD is only applicable for Layer 3 interfaces. properties: desiredMinimumTxInterval: description: |- @@ -437,8 +437,9 @@ spec: rule: self.type != 'Aggregate' || has(self.aggregation) - message: aggregation must only be specified on interfaces of type Aggregate rule: self.type == 'Aggregate' || !has(self.aggregation) - - message: ipv4 must not be specified for interfaces of type Aggregate - rule: self.type != 'Aggregate' || !has(self.ipv4) + - message: ipv4 must not be specified for Aggregate interfaces with switchport + configuration + rule: self.type != 'Aggregate' || !has(self.switchport) || !has(self.ipv4) - message: vlanRef must be specified for interfaces of type RoutedVLAN rule: self.type != 'RoutedVLAN' || has(self.vlanRef) - message: vlanRef must only be specified on interfaces of type RoutedVLAN @@ -449,13 +450,15 @@ spec: rule: self.type != 'RoutedVLAN' || !has(self.aggregation) - message: anycastGateway can only be enabled for interfaces of type RoutedVLAN rule: self.type == 'RoutedVLAN' || !has(self.ipv4) || !self.ipv4.anycastGateway - - message: vrfRef must not be specified for interfaces of type Aggregate - rule: self.type != 'Aggregate' || !has(self.vrfRef) + - message: vrfRef must not be specified for Aggregate interfaces with + switchport configuration + rule: self.type != 'Aggregate' || !has(self.switchport) || !has(self.vrfRef) - message: vrfRef must not be specified for Physical interfaces with switchport configuration rule: self.type != 'Physical' || !has(self.switchport) || !has(self.vrfRef) - - message: bfd must not be specified for interfaces of type Aggregate - rule: self.type != 'Aggregate' || !has(self.bfd) + - message: bfd must not be specified for Aggregate interfaces with switchport + configuration + rule: self.type != 'Aggregate' || !has(self.switchport) || !has(self.bfd) - message: bfd must not be specified for interfaces with switchport configuration rule: '!has(self.bfd) || !has(self.switchport)' - message: ethernet configuration must only be specified on interfaces diff --git a/config/samples/v1alpha1_interface.yaml b/config/samples/v1alpha1_interface.yaml index 29eef0a1..092d9b76 100644 --- a/config/samples/v1alpha1_interface.yaml +++ b/config/samples/v1alpha1_interface.yaml @@ -147,6 +147,53 @@ spec: --- apiVersion: networking.metal.ironcore.dev/v1alpha1 kind: Interface +metadata: + labels: + app.kubernetes.io/name: network-operator + app.kubernetes.io/managed-by: kustomize + networking.metal.ironcore.dev/device-name: leaf1 + name: eth1-3 +spec: + deviceRef: + name: leaf1 + name: eth1/3 + description: L3 Port-Channel Member + adminState: Up + type: Physical + mtu: 9216 +--- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: Interface +metadata: + labels: + app.kubernetes.io/name: network-operator + app.kubernetes.io/managed-by: kustomize + networking.metal.ironcore.dev/device-name: leaf1 + name: po-20 +spec: + deviceRef: + name: leaf1 + name: po20 + description: L3 Port-Channel + adminState: Up + type: Aggregate + mtu: 9216 + ipv4: + addresses: + - 10.0.100.0/31 + bfd: + enabled: true + desiredMinimumTxInterval: 300ms + requiredMinimumReceive: 300ms + detectionMultiplier: 3 + aggregation: + controlProtocol: + mode: Active + memberInterfaceRefs: + - name: eth1-3 +--- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: Interface metadata: labels: app.kubernetes.io/name: network-operator diff --git a/docs/api-reference/index.md b/docs/api-reference/index.md index 06e61d0d..aabe2211 100644 --- a/docs/api-reference/index.md +++ b/docs/api-reference/index.md @@ -1349,7 +1349,7 @@ _Appears in:_ | `aggregation` _[Aggregation](#aggregation)_ | Aggregation defines the aggregation (bundle) configuration for the interface.
This is only applicable for interfaces of type Aggregate. | | Optional: \{\}
| | `vlanRef` _[LocalObjectReference](#localobjectreference)_ | VlanRef is a reference to the VLAN resource that this interface provides routing for.
This is only applicable for interfaces of type RoutedVLAN.
The referenced VLAN must exist in the same namespace. | | Optional: \{\}
| | `vrfRef` _[LocalObjectReference](#localobjectreference)_ | VrfRef is a reference to the VRF resource that this interface belongs to.
If not specified, the interface will be part of the default VRF.
This is only applicable for Layer 3 interfaces.
The referenced VRF must exist in the same namespace. | | Optional: \{\}
| -| `bfd` _[BFD](#bfd)_ | BFD defines the Bidirectional Forwarding Detection configuration for the interface.
BFD is only applicable for Layer 3 interfaces (Physical, Loopback, RoutedVLAN). | | Optional: \{\}
| +| `bfd` _[BFD](#bfd)_ | BFD defines the Bidirectional Forwarding Detection configuration for the interface.
BFD is only applicable for Layer 3 interfaces. | | Optional: \{\}
| | `ethernet` _[Ethernet](#ethernet)_ | Ethernet defines the ethernet-specific configuration for physical interfaces.
This configuration is only applicable to Physical interfaces.
When omitted, ethernet parameters use their default values (e.g., FEC mode defaults to auto). | | Optional: \{\}
| diff --git a/internal/controller/core/interface_controller.go b/internal/controller/core/interface_controller.go index 9a68893e..43063e3e 100644 --- a/internal/controller/core/interface_controller.go +++ b/internal/controller/core/interface_controller.go @@ -316,6 +316,33 @@ func (r *InterfaceReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man }, }), ). + // Watches enqueues member Physical Interfaces when their parent Aggregate changes. + // Only triggers when Aggregate Spec fields change that affect member reconciliation. + Watches( + &v1alpha1.Interface{}, + handler.EnqueueRequestsFromMapFunc(r.aggregateToMembers), + builder.WithPredicates(predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return false + }, + UpdateFunc: func(e event.UpdateEvent) bool { + oldIntf := e.ObjectOld.(*v1alpha1.Interface) + newIntf := e.ObjectNew.(*v1alpha1.Interface) + // Only trigger when fields that affect member Physical interface + // reconciliation change (e.g. layer, VRF membership, MTU). + return !equality.Semantic.DeepEqual(oldIntf.Spec.IPv4, newIntf.Spec.IPv4) || + !equality.Semantic.DeepEqual(oldIntf.Spec.Switchport, newIntf.Spec.Switchport) || + !equality.Semantic.DeepEqual(oldIntf.Spec.VrfRef, newIntf.Spec.VrfRef) || + oldIntf.Spec.MTU != newIntf.Spec.MTU + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return false + }, + GenericFunc: func(e event.GenericEvent) bool { + return false + }, + }), + ). // Watches enqueues RoutedVLAN Interfaces for updates in referenced VLAN resources. // Only triggers on create and delete events since VLAN IDs are immutable. Watches( @@ -400,6 +427,18 @@ func (r *InterfaceReconciler) reconcile(ctx context.Context, s *scope) (_ ctrl.R } } + var aggregateParent *v1alpha1.Interface + if s.Interface.Spec.Type == v1alpha1.InterfaceTypePhysical && s.Interface.Status.MemberOf != nil { + aggregateParent = new(v1alpha1.Interface) + key := client.ObjectKey{Name: s.Interface.Status.MemberOf.Name, Namespace: s.Interface.Namespace} + if err := r.Get(ctx, key, aggregateParent); err != nil { + if !apierrors.IsNotFound(err) { + return ctrl.Result{}, fmt.Errorf("failed to get aggregate parent %q: %w", s.Interface.Status.MemberOf.Name, err) + } + aggregateParent = nil + } + } + var multiChassisID *int16 if s.Interface.Spec.Aggregation != nil && s.Interface.Spec.Aggregation.MultiChassis != nil { multiChassisID = &s.Interface.Spec.Aggregation.MultiChassis.ID @@ -443,13 +482,14 @@ func (r *InterfaceReconciler) reconcile(ctx context.Context, s *scope) (_ ctrl.R // Ensure the Interface is realized on the provider. err := s.Provider.EnsureInterface(ctx, &provider.EnsureInterfaceRequest{ - Interface: s.Interface, - ProviderConfig: s.ProviderConfig, - IPv4: ip, - Members: members, - MultiChassisID: multiChassisID, - VLAN: vlan, - VRF: vrf, + Interface: s.Interface, + ProviderConfig: s.ProviderConfig, + IPv4: ip, + Members: members, + MultiChassisID: multiChassisID, + AggregateParent: aggregateParent, + VLAN: vlan, + VRF: vrf, }) cond := conditions.FromError(err) @@ -874,6 +914,34 @@ func (r *InterfaceReconciler) interfaceToAggregate(ctx context.Context, obj clie return requests } +// aggregateToMembers is a [handler.MapFunc] to be used to enqueue requests for reconciliation +// for member Physical Interfaces when their parent Aggregate Interface gets updated. +func (r *InterfaceReconciler) aggregateToMembers(ctx context.Context, obj client.Object) []ctrl.Request { + intf, ok := obj.(*v1alpha1.Interface) + if !ok { + panic(fmt.Sprintf("Expected a Interface but got a %T", obj)) + } + + if intf.Spec.Type != v1alpha1.InterfaceTypeAggregate { + return nil + } + + log := ctrl.LoggerFrom(ctx, "Aggregate", klog.KObj(intf)) + + requests := make([]ctrl.Request, 0, len(intf.Spec.Aggregation.MemberInterfaceRefs)) + for _, ref := range intf.Spec.Aggregation.MemberInterfaceRefs { + log.Info("Enqueuing member Interface for reconciliation", "Member", ref.Name) + requests = append(requests, ctrl.Request{ + NamespacedName: client.ObjectKey{ + Name: ref.Name, + Namespace: intf.Namespace, + }, + }) + } + + return requests +} + // vlanToRoutedVLAN is a [handler.MapFunc] to be used to enqueue requests for reconciliation // for a RoutedVLAN Interface when its referenced VLAN changes. func (r *InterfaceReconciler) vlanToRoutedVLAN(ctx context.Context, obj client.Object) []ctrl.Request { diff --git a/internal/controller/core/interface_controller_test.go b/internal/controller/core/interface_controller_test.go index 915b852c..08088084 100644 --- a/internal/controller/core/interface_controller_test.go +++ b/internal/controller/core/interface_controller_test.go @@ -577,6 +577,163 @@ var _ = Describe("Interface Controller", func() { }).Should(Succeed()) }) + It("Should successfully reconcile an Aggregate Interface with IPv4 addresses and VRF", func() { + By("Creating a VRF resource") + vrf := &v1alpha1.VRF{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.VRFSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: name}, + Name: "PROD", + VNI: 1000, + }, + } + Expect(k8sClient.Create(ctx, vrf)).To(Succeed()) + + By("Creating a Physical member interface") + member := &v1alpha1.Interface{ + ObjectMeta: metav1.ObjectMeta{ + Name: memberName1, + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.InterfaceSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: name}, + Name: memberName1, + AdminState: v1alpha1.AdminStateUp, + Type: v1alpha1.InterfaceTypePhysical, + }, + } + Expect(k8sClient.Create(ctx, member)).To(Succeed()) + + By("Creating an L3 Aggregate Interface with IPv4 and VRF") + aggregate := &v1alpha1.Interface{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.InterfaceSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: name}, + Name: name, + AdminState: v1alpha1.AdminStateUp, + Description: "Test L3 Aggregate Interface", + MTU: 9000, + Type: v1alpha1.InterfaceTypeAggregate, + VrfRef: &v1alpha1.LocalObjectReference{Name: vrf.Name}, + IPv4: &v1alpha1.InterfaceIPv4{ + Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.0.1/31")}}, + }, + Aggregation: &v1alpha1.Aggregation{ + MemberInterfaceRefs: []v1alpha1.LocalObjectReference{ + {Name: memberName1}, + }, + ControlProtocol: v1alpha1.ControlProtocol{ + Mode: v1alpha1.LACPModeActive, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, aggregate)).To(Succeed()) + + By("Verifying the controller sets successful status conditions") + Eventually(func(g Gomega) { + resource := &v1alpha1.Interface{} + g.Expect(k8sClient.Get(ctx, key, resource)).To(Succeed()) + g.Expect(resource.Status.Conditions).To(HaveLen(3)) + g.Expect(resource.Status.Conditions[0].Type).To(Equal(v1alpha1.ReadyCondition)) + g.Expect(resource.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) + g.Expect(resource.Status.Conditions[1].Type).To(Equal(v1alpha1.ConfiguredCondition)) + g.Expect(resource.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) + g.Expect(resource.Status.Conditions[2].Type).To(Equal(v1alpha1.OperationalCondition)) + g.Expect(resource.Status.Conditions[2].Status).To(Equal(metav1.ConditionTrue)) + }).Should(Succeed()) + + By("Verifying the Interface has the VRF label") + Eventually(func(g Gomega) { + resource := &v1alpha1.Interface{} + g.Expect(k8sClient.Get(ctx, key, resource)).To(Succeed()) + g.Expect(resource.Labels).To(HaveKeyWithValue(v1alpha1.VRFLabel, vrf.Name)) + }).Should(Succeed()) + + By("Verifying member interface is properly linked") + Eventually(func(g Gomega) { + memberIntf := &v1alpha1.Interface{} + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: memberName1, Namespace: metav1.NamespaceDefault}, memberIntf)).To(Succeed()) + g.Expect(memberIntf.Status.MemberOf).ToNot(BeNil()) + g.Expect(memberIntf.Status.MemberOf.Name).To(Equal(name)) + g.Expect(memberIntf.Labels).To(HaveKeyWithValue(v1alpha1.AggregateLabel, name)) + }).Should(Succeed()) + + By("Verifying the Aggregate Interface is configured in the provider") + Eventually(func(g Gomega) { + g.Expect(testProvider.Ports.Has(name)).To(BeTrue(), "Provider should have L3 Aggregate Interface configured") + }).Should(Succeed()) + }) + + It("Should reconcile a Physical interface that is a member of an L3 Aggregate", func() { + By("Creating an L3 Aggregate interface") + aggregate := &v1alpha1.Interface{ + ObjectMeta: metav1.ObjectMeta{ + Name: "po100", + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.InterfaceSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: name}, + Name: "po100", + AdminState: v1alpha1.AdminStateUp, + Type: v1alpha1.InterfaceTypeAggregate, + IPv4: &v1alpha1.InterfaceIPv4{ + Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.100.0/31")}}, + }, + Aggregation: &v1alpha1.Aggregation{ + MemberInterfaceRefs: []v1alpha1.LocalObjectReference{ + {Name: "eth1-100"}, + }, + ControlProtocol: v1alpha1.ControlProtocol{ + Mode: v1alpha1.LACPModeActive, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, aggregate)).To(Succeed()) + + By("Creating the member Physical interface") + member := &v1alpha1.Interface{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eth1-100", + Namespace: metav1.NamespaceDefault, + }, + Spec: v1alpha1.InterfaceSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: name}, + Name: "eth1-100", + AdminState: v1alpha1.AdminStateUp, + Type: v1alpha1.InterfaceTypePhysical, + }, + } + Expect(k8sClient.Create(ctx, member)).To(Succeed()) + + By("Verifying the member eventually gets MemberOf set by the Aggregate reconciliation") + Eventually(func(g Gomega) { + m := &v1alpha1.Interface{} + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: "eth1-100", Namespace: metav1.NamespaceDefault}, m)).To(Succeed()) + g.Expect(m.Status.MemberOf).NotTo(BeNil()) + g.Expect(m.Status.MemberOf.Name).To(Equal("po100")) + }).Should(Succeed()) + + By("Verifying the member Physical interface reconciles without error") + Eventually(func(g Gomega) { + m := &v1alpha1.Interface{} + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: "eth1-100", Namespace: metav1.NamespaceDefault}, m)).To(Succeed()) + g.Expect(m.Labels).To(HaveKeyWithValue(v1alpha1.AggregateLabel, "po100")) + }).Should(Succeed()) + + By("Verifying the member Physical interface is configured in the provider") + Eventually(func(g Gomega) { + g.Expect(testProvider.Ports.Has("eth1-100")).To(BeTrue(), "Provider should have member Physical Interface configured") + }).Should(Succeed()) + }) + It("Should handle unnumbered reference to non-Loopback Interface", func() { By("Creating a Physical Interface to be referenced") phys := &v1alpha1.Interface{ diff --git a/internal/provider/cisco/nxos/intf.go b/internal/provider/cisco/nxos/intf.go index 67363138..a5732cda 100644 --- a/internal/provider/cisco/nxos/intf.go +++ b/internal/provider/cisco/nxos/intf.go @@ -8,7 +8,6 @@ import ( "encoding/json" "errors" "fmt" - "maps" "slices" "strconv" "strings" @@ -99,12 +98,12 @@ func (p *PhysIf) Validate() error { } func (p *PhysIf) Default() { - p.AccessVlan = DefaultVLAN p.FecMode = FecModeAuto p.Layer = Layer2 p.MTU = DefaultMTU p.Medium = MediumBroadcast p.Mode = SwitchportModeAccess + p.AccessVlan = DefaultVLAN p.NativeVlan = DefaultVLAN p.TrunkVlans = DefaultVLANRange p.PhysExtdItems.BufferBoost = AdminStEnable @@ -129,6 +128,9 @@ func (v *VrfMember) XPath() string { if loopbackRe.MatchString(v.IfName) { return "System/intf-items/lb-items/LbRtdIf-list[id=" + v.IfName + "]/rtvrfMbr-items" } + if portchannelRe.MatchString(v.IfName) { + return "System/intf-items/aggr-items/AggrIf-list[id=" + v.IfName + "]/rtvrfMbr-items" + } return "System/intf-items/phys-items/PhysIf-list[id=" + v.IfName + "]/rtvrfMbr-items" } @@ -225,11 +227,13 @@ type PortChannel struct { ID string `json:"id"` Layer Layer `json:"layer"` MTU int32 `json:"mtu,omitempty"` + Medium Medium `json:"medium"` Mode SwitchportMode `json:"mode"` PcMode PortChannelMode `json:"pcMode"` NativeVlan string `json:"nativeVlan"` TrunkVlans string `json:"trunkVlans"` UserCfgdFlags UserFlags `json:"userCfgdFlags"` + RtvrfMbrItems *VrfMember `json:"rtvrfMbr-items,omitempty"` RsmbrIfsItems struct { RsMbrIfsList gnmiext.List[string, *PortChannelMember] `json:"RsMbrIfs-list,omitzero"` } `json:"rsmbrIfs-items,omitzero"` @@ -367,25 +371,6 @@ func (a *AddrItem) XPath() string { return "System/ipv4-items/inst-items/dom-items/Dom-list[name=" + a.Vrf + "]/if-items/If-list[id=" + a.ID + "]" } -// IsPointToPoint checks if the address item represents a point-to-point interface. -// It returns true if the interface is unnumbered or if its address has a /31 (IPv4) [RFC3021] -// or /127 (IPv6) [RFC6164] subnet mask, indicating a point-to-point link. -// -// [RFC3021]: https://datatracker.ietf.org/doc/html/rfc3021 -// [RFC6164]: https://datatracker.ietf.org/doc/html/rfc6164 -func (a *AddrItem) IsPointToPoint() bool { - if a != nil { - if a.Unnumbered != "" { - return true - } - if a.AddrItems.AddrList.Len() == 1 { - addr := slices.Collect(maps.Values(a.AddrItems.AddrList))[0] - return strings.HasSuffix(addr.Addr, "/31") || strings.HasSuffix(addr.Addr, "/127") - } - } - return false -} - type IntfAddr struct { Addr string `json:"addr"` Pref int `json:"pref"` diff --git a/internal/provider/cisco/nxos/intf_test.go b/internal/provider/cisco/nxos/intf_test.go index dd1822af..5103b789 100644 --- a/internal/provider/cisco/nxos/intf_test.go +++ b/internal/provider/cisco/nxos/intf_test.go @@ -55,6 +55,7 @@ func init() { Descr: "vPC Leaf1 to Host1", ID: "po10", Layer: Layer2, + Medium: MediumBroadcast, Mode: SwitchportModeTrunk, PcMode: PortChannelModeActive, NativeVlan: DefaultVLAN, @@ -64,6 +65,25 @@ func init() { pc.RsmbrIfsItems.RsMbrIfsList.Set(NewPortChannelMember("eth1/10")) Register("pc", pc) + Register("pc_rtd", &PortChannel{ + AccessVlan: "unknown", + AdminSt: AdminStUp, + Descr: "L3 Port-Channel to Spine1", + ID: "po20", + Layer: Layer3, + MTU: 9216, + Medium: MediumPointToPoint, + Mode: SwitchportModeAccess, + NativeVlan: "unknown", + PcMode: PortChannelModeActive, + TrunkVlans: DefaultVLANRange, + UserCfgdFlags: UserFlagAdminState | UserFlagAdminLayer | UserFlagAdminMTU, + RtvrfMbrItems: NewVrfMember("po20", "default"), + AggrExtdItems: struct { + BufferBoost AdminSt4 `json:"bufferBoost,omitempty"` + }{BufferBoost: AdminStEnable}, + }) + svi := &SwitchVirtualInterface{ AdminSt: AdminStUp, Descr: "Foo", diff --git a/internal/provider/cisco/nxos/provider.go b/internal/provider/cisco/nxos/provider.go index c12fe772..8920db11 100644 --- a/internal/provider/cisco/nxos/provider.go +++ b/internal/provider/cisco/nxos/provider.go @@ -622,6 +622,22 @@ func (p *Provider) DeleteEVPNInstance(ctx context.Context, req *provider.EVPNIns return p.client.Delete(ctx, conf...) } +// isPointToPoint reports whether the given IPv4 configuration represents a +// point-to-point link. It returns true if the interface is unnumbered or if it +// has a single address whose prefix indicates a point-to-point link. +func isPointToPoint(ipv4 *v1alpha1.InterfaceIPv4) bool { + if ipv4 == nil { + return false + } + if ipv4.Unnumbered != nil { + return true + } + if len(ipv4.Addresses) == 1 { + return ipv4.Addresses[0].IsPointToPoint() + } + return false +} + func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInterfaceRequest) error { name, err := ShortName(req.Interface.Spec.Name) if err != nil { @@ -668,21 +684,19 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } } - if req.Interface.Spec.Type != v1alpha1.InterfaceTypeAggregate { - del := make([]gnmiext.Configurable, 0, 2) - addrs := new(AddrList) - if err := p.client.GetConfig(ctx, addrs); err != nil && !errors.Is(err, gnmiext.ErrNil) { - return err - } - for _, a := range addrs.GetAddrItemsByInterface(name) { - if addr == nil || a.Vrf != vrf { - del = append(del, a) - } - } - if err := p.client.Delete(ctx, del...); err != nil { - return err + del := make([]gnmiext.Configurable, 0, 2) + addrs := new(AddrList) + if err := p.client.GetConfig(ctx, addrs); err != nil && !errors.Is(err, gnmiext.ErrNil) { + return err + } + for _, a := range addrs.GetAddrItemsByInterface(name) { + if addr == nil || a.Vrf != vrf { + del = append(del, a) } } + if err := p.client.Delete(ctx, del...); err != nil { + return err + } conf := make([]gnmiext.Configurable, 0, 4) switch req.Interface.Spec.Type { @@ -705,7 +719,6 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } if req.Interface.Spec.Ethernet != nil && req.Interface.Spec.Ethernet.FECMode != "" { - p.FecMode = FecModeAuto switch req.Interface.Spec.Ethernet.FECMode { case v1alpha1.FECModeFC: p.FecMode = FecModeCL74 @@ -718,20 +731,20 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } } - if req.IPv4 != nil { + // If this Physical interface is a member of an L3 Aggregate (port-channel), + // it must be Layer3 on NX-OS even though it has no IP address of its own. + if req.IPv4 != nil || (req.AggregateParent != nil && req.AggregateParent.Spec.IPv4 != nil) { p.Layer = Layer3 + p.RtvrfMbrItems = NewVrfMember(name, vrf) + p.AccessVlan = "unknown" + p.NativeVlan = "unknown" } - if addr.IsPointToPoint() { + + if isPointToPoint(req.Interface.Spec.IPv4) || (req.AggregateParent != nil && isPointToPoint(req.AggregateParent.Spec.IPv4)) { p.Medium = MediumPointToPoint } - p.AccessVlan = "unknown" - p.NativeVlan = "unknown" - p.RtvrfMbrItems = NewVrfMember(name, vrf) if req.Interface.Spec.Switchport != nil { - p.RtvrfMbrItems = nil - p.AccessVlan = DefaultVLAN - p.NativeVlan = DefaultVLAN switch req.Interface.Spec.Switchport.Mode { case v1alpha1.SwitchportModeAccess: p.Mode = SwitchportModeAccess @@ -749,11 +762,8 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } } - if cfg.Spec.BufferBoost != nil { + if cfg.Spec.BufferBoost != nil && !cfg.Spec.BufferBoost.Enabled { p.PhysExtdItems.BufferBoost = AdminStDisable - if cfg.Spec.BufferBoost.Enabled { - p.PhysExtdItems.BufferBoost = AdminStEnable - } } if err := p.Validate(); err != nil { @@ -794,9 +804,9 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte if req.Interface.Spec.AdminState == v1alpha1.AdminStateUp { pc.AdminSt = AdminStUp } - // Note: Layer 3 port-channel interfaces are not yet supported pc.Layer = Layer2 pc.Mode = SwitchportModeAccess + pc.Medium = MediumBroadcast pc.AccessVlan = DefaultVLAN pc.NativeVlan = DefaultVLAN pc.TrunkVlans = DefaultVLANRange @@ -809,6 +819,17 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte pc.UserCfgdFlags |= UserFlagAdminMTU } + if req.IPv4 != nil { + pc.Layer = Layer3 + pc.RtvrfMbrItems = NewVrfMember(name, vrf) + pc.AccessVlan = "unknown" + pc.NativeVlan = "unknown" + } + + if isPointToPoint(req.Interface.Spec.IPv4) { + pc.Medium = MediumPointToPoint + } + pc.PcMode = PortChannelModeActive switch m := req.Interface.Spec.Aggregation.ControlProtocol.Mode; m { case v1alpha1.LACPModeActive: @@ -859,11 +880,8 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } } - if cfg.Spec.BufferBoost != nil { + if cfg.Spec.BufferBoost != nil && !cfg.Spec.BufferBoost.Enabled { pc.AggrExtdItems.BufferBoost = AdminStDisable - if cfg.Spec.BufferBoost.Enabled { - pc.AggrExtdItems.BufferBoost = AdminStEnable - } } conf = append(conf, pc) @@ -923,7 +941,7 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte return fmt.Errorf("unsupported interface type: %s", req.Interface.Spec.Type) } - if (req.Interface.Spec.Type == v1alpha1.InterfaceTypePhysical && req.IPv4 == nil) || req.Interface.Spec.Type == v1alpha1.InterfaceTypeAggregate { + if (req.Interface.Spec.Type == v1alpha1.InterfaceTypePhysical || req.Interface.Spec.Type == v1alpha1.InterfaceTypeAggregate) && req.IPv4 == nil && (req.AggregateParent == nil || req.AggregateParent.Spec.IPv4 == nil) { stp := new(SpanningTree) stp.IfName = name stp.Mode = SpanningTreeModeDefault @@ -957,8 +975,6 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte conf = append(conf, addr) } - bfd := new(BFD) - bfd.ID = name if req.Interface.Spec.BFD != nil { f := new(Feature) f.Name = "bfd" @@ -970,6 +986,8 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte icmp.Ctrl = "port-unreachable" conf = append(conf, icmp) + bfd := new(BFD) + bfd.ID = name bfd.AdminSt = AdminStDisabled if req.Interface.Spec.BFD.Enabled { bfd.AdminSt = AdminStEnabled @@ -994,7 +1012,7 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte icmp := new(ICMPIf) icmp.ID = name switch req.Interface.Spec.Type { - case v1alpha1.InterfaceTypePhysical: + case v1alpha1.InterfaceTypePhysical, v1alpha1.InterfaceTypeAggregate: if err := p.client.Delete(ctx, icmp); err != nil { return err } @@ -1017,14 +1035,12 @@ func (p *Provider) DeleteInterface(ctx context.Context, req *provider.InterfaceR } conf := make([]gnmiext.Configurable, 0, 3) - if req.Interface.Spec.Type != v1alpha1.InterfaceTypeAggregate { - addrs := new(AddrList) - if err := p.client.GetConfig(ctx, addrs); err != nil && !errors.Is(err, gnmiext.ErrNil) { - return err - } - for _, addr := range addrs.GetAddrItemsByInterface(name) { - conf = append(conf, addr) - } + addrs := new(AddrList) + if err := p.client.GetConfig(ctx, addrs); err != nil && !errors.Is(err, gnmiext.ErrNil) { + return err + } + for _, addr := range addrs.GetAddrItemsByInterface(name) { + conf = append(conf, addr) } bfd := new(BFD) @@ -1037,7 +1053,6 @@ func (p *Provider) DeleteInterface(ctx context.Context, req *provider.InterfaceR i.ID = name conf = append(conf, i) - // Delete any spanning tree config associated with the interface. stp := new(SpanningTree) stp.IfName = name if err = p.client.GetConfig(ctx, stp); err == nil { diff --git a/internal/provider/cisco/nxos/testdata/pc.json b/internal/provider/cisco/nxos/testdata/pc.json index e08bfde9..55ba0320 100644 --- a/internal/provider/cisco/nxos/testdata/pc.json +++ b/internal/provider/cisco/nxos/testdata/pc.json @@ -8,6 +8,7 @@ "descr": "vPC Leaf1 to Host1", "id": "po10", "layer": "Layer2", + "medium": "broadcast", "mode": "trunk", "pcMode": "active", "nativeVlan": "vlan-1", diff --git a/internal/provider/cisco/nxos/testdata/pc_rtd.json b/internal/provider/cisco/nxos/testdata/pc_rtd.json new file mode 100644 index 00000000..9895c4ad --- /dev/null +++ b/internal/provider/cisco/nxos/testdata/pc_rtd.json @@ -0,0 +1,28 @@ +{ + "intf-items": { + "aggr-items": { + "AggrIf-list": [ + { + "accessVlan": "unknown", + "adminSt": "up", + "descr": "L3 Port-Channel to Spine1", + "id": "po20", + "layer": "Layer3", + "mtu": 9216, + "medium": "p2p", + "mode": "access", + "pcMode": "active", + "nativeVlan": "unknown", + "trunkVlans": "1-4094", + "userCfgdFlags": "admin_layer,admin_mtu,admin_state", + "rtvrfMbr-items": { + "tDn": "/System/inst-items/Inst-list[name='default']" + }, + "aggrExtd-items": { + "bufferBoost": "enable" + } + } + ] + } + } +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index eb0a5191..6fad4007 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -95,6 +95,10 @@ type EnsureInterfaceRequest struct { Members []*v1alpha1.Interface // MultiChassisID is the multi-chassis identifier for multi-chassis link aggregation. MultiChassisID *int16 + // AggregateParent is the parent Aggregate interface when this interface is + // a member of an aggregated interface. + // This field is only applicable if the interface type is Physical. + AggregateParent *v1alpha1.Interface // VLAN is the referenced VLAN for routed VLAN interfaces (SVI). // This field is only applicable if the interface type is RoutedVLAN. VLAN *v1alpha1.VLAN